Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix-up behavior for empty PerfGroup #44

Merged
merged 1 commit into from
Oct 3, 2024

Conversation

topolarity
Copy link
Member

For machines that don't have perf properly configured, it's common to end up with an empty PerfGroup that has an invalid leader_fd. This commit adds the appropriate guards to make sure that doesn't error excessively.

On my WSL2 setup, this takes the test suite from:

Test Summary:      | Pass  Error  Total  Time
LinuxPerf          |   31      4     35  4.6s
  simple benchmark |           2      2  2.6s
  @measure         |           1      1  0.1s
  Parser           |   24            24  0.3s
  @pstats          |    1      1      2  1.5s
  _addcommas       |    6             6  0.0s
ERROR: LoadError: Some tests did not pass: 31 passed, 0 failed, 4 errored, 0 broken.

to:

Test Summary: | Pass  Total  Time
LinuxPerf     |   37     37  3.1s

For machines that don't have `perf` properly configured (e.g. WSL),
it's common to end up with an empty PerfGroup that has an invalid
`leader_fd`. This commit adds the appropriate guards to make sure
that doesn't error excessively.
@Zentrik
Copy link
Collaborator

Zentrik commented Oct 1, 2024

I thought perf didn't work at all on wsl2?

@topolarity
Copy link
Member Author

topolarity commented Oct 1, 2024

WSL2 has better support than WSL1 (depending on your HW/kernel configuration), but it's missing many of the timers you might expect.

That's what this is fixing - when none of the timers are supported the EventGroup can end up empty and then this behaves poorly.

Copy link
Collaborator

@Zentrik Zentrik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we should warn after constructing an EventGroup if its leader_fd is -1 or if any events within it are disabled. Looks like we already do.

@Zentrik Zentrik merged commit d4e0a9a into JuliaPerf:master Oct 3, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants