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

perf: skip debug work when not in debug mode #60

Merged
merged 3 commits into from
Apr 13, 2023

Conversation

mark-rushakoff
Copy link
Member

MergedFileDescriptors was unconditionally checking import paths and file descriptor differences, then conditionally returning the differences as an error. There is no need to check the differences if we will never return them.

This made a measurable difference in running simd q:

$ hyperfine --warmup 4 -r 50 '/tmp/simd-dodebugwork q' '/tmp/simd-skipdebugwork q' Benchmark 1: /tmp/simd-dodebugwork q
  Time (mean ± σ):     121.6 ms ±   6.5 ms    [User: 173.6 ms, System: 23.7 ms]
  Range (min … max):   117.0 ms … 162.7 ms    50 runs

  Warning: Statistical outliers were detected. ...

Benchmark 2: /tmp/simd-skipdebugwork q
  Time (mean ± σ):     113.6 ms ±   6.5 ms    [User: 158.5 ms, System: 23.1 ms]
  Range (min … max):   109.3 ms … 139.7 ms    50 runs

  Warning: Statistical outliers were detected. ...

Summary
  '/tmp/simd-skipdebugwork q' ran
    1.07 ± 0.08 times faster than '/tmp/simd-dodebugwork q'

And on two single runs of TestAppStateDeterminism, this optimization reduced local runtime from 20.9 seconds to 19.7, an anecdotal ~6% speedup.

MergedFileDescriptors was unconditionally checking import paths and file
descriptor differences, then conditionally returning the differences as
an error. There is no need to check the differences if we will never
return them.

This made a measurable difference in running simd q:

$ hyperfine --warmup 4 -r 50 '/tmp/simd-dodebugwork q' '/tmp/simd-skipdebugwork q'
Benchmark 1: /tmp/simd-dodebugwork q
  Time (mean ± σ):     121.6 ms ±   6.5 ms    [User: 173.6 ms, System: 23.7 ms]
  Range (min … max):   117.0 ms … 162.7 ms    50 runs

  Warning: Statistical outliers were detected. ...

Benchmark 2: /tmp/simd-skipdebugwork q
  Time (mean ± σ):     113.6 ms ±   6.5 ms    [User: 158.5 ms, System: 23.1 ms]
  Range (min … max):   109.3 ms … 139.7 ms    50 runs

  Warning: Statistical outliers were detected. ...

Summary
  '/tmp/simd-skipdebugwork q' ran
    1.07 ± 0.08 times faster than '/tmp/simd-dodebugwork q'

And on two single runs of TestAppStateDeterminism, this optimization
reduced local runtime from 20.9 seconds to 19.7, an anecdotal ~6%
speedup.
@mark-rushakoff mark-rushakoff requested a review from a team as a code owner April 13, 2023 13:44
Copy link
Member

@julienrbrt julienrbrt left a comment

Choose a reason for hiding this comment

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

lgtm!

The outdated versions seem to be causing CI failures.
@mark-rushakoff mark-rushakoff merged commit d932639 into main Apr 13, 2023
@mark-rushakoff mark-rushakoff deleted the mr/perf-skip-debug-work branch April 13, 2023 15:26
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.

4 participants