-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Align benchmark::State to a cacheline. #1230
Conversation
This can avoid interference with neighboring objects and stabilize benchmark results.
@dominichamon Another reason to have absl dependency :) Would make this PR so much simpler. |
do you have the results from a run with and without this change? |
@dominichamon I only have an internal microbenchmark, which demonstrated higher variability across runs (especially when combined with a change to TCMalloc) without the alignment attribute. |
This seems plausible (although hard to tell without a test), but is this only about the |
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.
Can you use std::hardware_destructive_interference_size
from C++17 if available?
we could try to run some tests, but i don't think it should block this PR (which i totally missed). or just align all the things. |
not in the public header. soon. |
Co-authored-by: Roman Lebedev <[email protected]>
yeah your way is better because it separates the definition of the cacheline size from the macro that uses it. |
@@ -290,11 +290,50 @@ BENCHMARK(BM_test)->Unit(benchmark::kMillisecond); | |||
#define BENCHMARK_OVERRIDE | |||
#endif | |||
|
|||
#if defined(__GNUC__) |
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.
Not really sure if this outermost guard is needed, but then i don't know what set of per-cpu defines MSVC provides.
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.
Yay, thank you!
This can avoid interference with neighboring objects and stabilize
benchmark results.