-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Run boolean and bitwise kernels for longer to reduce benchmark noise #8872
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
Conversation
d6c560f to
53e05b2
Compare
53e05b2 to
dcd00cf
Compare
| fn bitwise_array_benchmark(c: &mut Criterion) { | ||
| let size = 64 * 1024_usize; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We often run these kernels on 8k elements, not 64k elements, so adjust the benchmark to reflect that
|
🤖 |
|
🤖 |
|
run benchmark bitwise_kernel |
|
🤖: Benchmark completed Details
|
|
🤖 |
|
🤖: Benchmark completed Details
|
|
Roughly speaking this shows what I expect which is the benchmarks taking longer |
|
I am not convinced this is the right approach. I think a better approach would be to run the kernels on multiple different sizes / allocations. I suspect alignment is causing benchmark artifacts |
Note to self:
Which issue does this PR close?
We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax.
Rationale for this change
The boolean and bitwise kernels are currently very noisy and sometimes get nonsensical results (e.g. #8854 (comment)) I think this is because they are so fast. For example, Here is an output from a recent run (note the benchmark each takes only nanoseconds
What changes are included in this PR?
Are these changes tested?
I will benchmark then
Are there any user-facing changes?
No these are benchmarks