-
Notifications
You must be signed in to change notification settings - Fork 13
chore: remove unnecessary black_box in criterion #98
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
CodSpeed Instrumentation Performance ReportMerging #98 will degrade performances by 24.19%Comparing Summary
Benchmarks breakdown
|
CodSpeed WallTime Performance ReportMerging #98 will degrade performances by 3.01%Comparing Summary
Benchmarks breakdown
|
af3989f
to
fc76872
Compare
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.
Good catch! We definitely should follow what criterion is doing (see their code). Since we're already wrapping it with black_box
in the drop we don't have to do it before as well!
LGTM!
The original code only uses black_box when dropping, not inside of the measurement.
Apply the same optimization from PR 98 to both divan and bencher compat layers: - Remove black_box around routine calls during measurement - Only apply black_box to outputs after measurement ends - Reduces stack usage and eliminates unnecessary memory operations - Improves benchmark performance by avoiding artificial overhead 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
a7c4bcd
to
b71c93b
Compare
The original code only uses black_box when dropping, not inside of the measurement.
This actually matters as black_box usually spills the value to the stack, so if the benchmark routine returns a large value, the benchmark will record also an unnecessary memcpy.