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 <bitset> benchmark correctness #4817

Merged
merged 5 commits into from
Jul 11, 2024
Merged

Conversation

AlexGuteniev
Copy link
Contributor

@AlexGuteniev AlexGuteniev commented Jul 7, 2024

I see more things to do with bitset regarding performance, but first let's fix the current benchmark.

Fortunately, the discovered issues didn't have impact on results, and on performance decisions.

Add DoNotOptimize to input

Should use this to prevent compiler making the benchmarked function or its part constant.
Currently the compiler doesn't do this, but it might improve in the future

Make 512 case as large

Small cases were initialized with uint64_t which was not enough to initialize 512 bit bitset.

The 512 rows in the benchmark improved, but it is gooseberries vs watermelons: small cases have inner loop whereas large doesn't.

(The loop in small cases is to defeat branch prediction for the cases of branchy implementation activation. It is not needed for larger bitsets, as these would be vectorized, and if they wouldn't, the amount of bits should be enough to overflow the branch prediction)

@AlexGuteniev AlexGuteniev requested a review from a team as a code owner July 7, 2024 09:44
@AlexGuteniev AlexGuteniev changed the title Fix <bitset> binchmark correctness Fix <bitset> benchmark correctness Jul 7, 2024
@StephanTLavavej StephanTLavavej added the test Related to test code label Jul 8, 2024
@StephanTLavavej StephanTLavavej self-assigned this Jul 8, 2024
benchmarks/src/bitset_to_string.cpp Outdated Show resolved Hide resolved
benchmarks/src/bitset_to_string.cpp Outdated Show resolved Hide resolved
benchmarks/src/bitset_to_string.cpp Outdated Show resolved Hide resolved
@StephanTLavavej StephanTLavavej removed their assignment Jul 8, 2024
@StephanTLavavej StephanTLavavej self-assigned this Jul 10, 2024
@StephanTLavavej
Copy link
Member

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

@StephanTLavavej StephanTLavavej merged commit afb1fc1 into microsoft:main Jul 11, 2024
39 checks passed
@StephanTLavavej
Copy link
Member

Thanks for improving how we benchmark all of these bits! 🚀 💻 ⏱️

@AlexGuteniev AlexGuteniev deleted the bits branch July 12, 2024 04:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test Related to test code
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants