[mono] Switch simdhash to prime bucket counts; increase load factor #113274
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kg
changed the title
3 tasks
This comment was marked as outdated.
This comment was marked as outdated.
kg
force-pushed
the
simdhash-size-policy
branch
from
0e8782e to
9860d11
Compare
kg
requested review from
BrzVlad,
kotlarmilos,
steveisok,
vitek-karas and
thaystg
as code owners
There was a problem hiding this comment.
Copilot wasn't able to review any files in this pull request.
Files not reviewed (18)
- src/mono/mono/metadata/bundled-resources.c: Language not supported
- src/mono/mono/metadata/class.c: Language not supported
- src/mono/mono/metadata/metadata.c: Language not supported
- src/mono/mono/mini/interp/transform.c: Language not supported
- src/mono/mono/mini/seq-points.c: Language not supported
- src/native/containers/dn-simdhash-ght-compatible.c: Language not supported
- src/native/containers/dn-simdhash-ght-compatible.h: Language not supported
- src/native/containers/dn-simdhash-ptr-ptr.c: Language not supported
- src/native/containers/dn-simdhash-ptrpair-ptr.c: Language not supported
- src/native/containers/dn-simdhash-specialization.h: Language not supported
- src/native/containers/dn-simdhash-string-ptr.c: Language not supported
- src/native/containers/dn-simdhash-u32-ptr.c: Language not supported
- src/native/containers/dn-simdhash-utils.h: Language not supported
- src/native/containers/dn-simdhash.c: Language not supported
- src/native/containers/dn-simdhash.h: Language not supported
- src/native/containers/simdhash-benchmark/Makefile: Language not supported
- src/native/containers/simdhash-benchmark/all-measurements.h: Language not supported
- src/native/containers/simdhash-benchmark/benchmark.c: Language not supported
|
I think we should probably resume using GHashTable for the one table in question (#113287) since its performance is still slightly better, but we want to land these fixes/improvements to benefit the rest of the code that still uses simdhash. I did a quick audit of the rest of our tables and I think only the problem one happens to have a bad hash. |
…ms like sometimes they don't get inlined Add some measurements for simdhash ght Improve makefile Introduce scan data that's fetched once per bucket Targeted simdhash optimizations via new dn_simdhash_ght_get_value_or_default Fix indentation to satisfy CI build warnings Fix build warnings Switch to spaced prime bucket counts instead of power of two bucket counts for better collision resistance Fix cache alignment of ght compatible and ptr ptr simdhash variants on 64-bit Higher simdhash load factor (closer to ghashtable's; improves performance for bad hashes) Improve benchmark overhead calculation Add sequential keys measurements Fix build on CI Revert ght bucket size Revert "Revert ght bucket size" This reverts commit 2a225ad. Bug fixes Add murmurhash32 variant for use in improved mono hash Increase load factor to 30%
This was referenced Apr 8, 2025
|
The build-time perf results measured with the test app from #110406 (comment) on a macOS‑arm64 host targeting iOS‑arm64 in Release configuration are below. The build‑time performance are unchanged with this PR compared to both .NET 9 and .NET 8. AOT-compiling all assemblies for a non-interpreted build (.NET 8) AOT-compiling all assemblies for a non-interpreted build (.NET 9) AOT-compiling all assemblies for a non-interpreted build (.NET 10) - this PR |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Supercedes (contains) #113241.
The current version of dn_simdhash_ght_compatible has especially bad performance on arm64 vs x64, so I took some steps to redesign it and address some codegen deficiencies. This has the added benefit of making it a bit faster on x64 as well.
There weren't benchmarks for ght_compatible in the benchmark suite originally, so I added a couple. (I was benchmarking end-to-end originally, which makes it harder to compare across platforms.)
The F14 research and my initial research into vectorized hashing both showed that power-of-two bucket counts delivered optimal performance, but later on when doing the Dictionary vectorization work I determined that power-of-two counts have vastly inferior collision resistance compared to prime bucket counts. GHashTable and SCG.Dictionary both use spaced primes for bucket counts, so this PR attempts to do the same for dn_simdhash.
The best-case performance (for an optimal hash) gets a little worse but when I benchmarked power-of-two counts with a synthetic bad hash function the perf regression was over 100x, versus ~2x for prime bucket counts.
I also improved the search suffix selection to match the better one I came up with from the vectorized dictionary research, and added some benchmarks for the bad hash function scenario.
Selected benchmark measurements from x64:
I did measurements on ampere arm64 as well and verified that it's okay there.