Skip to content

[mono] dn_simdhash_ght_compatible optimizations for ARM64#113241

Closed
kg wants to merge 4 commits intodotnet:mainfrom
kg:simdhash-inline-ght-equals
Closed

[mono] dn_simdhash_ght_compatible optimizations for ARM64#113241
kg wants to merge 4 commits intodotnet:mainfrom
kg:simdhash-inline-ght-equals

Conversation

@kg
Copy link
Member

@kg kg commented Mar 7, 2025

Depends on #113095.

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 other variants of simdhash should be unaffected, this was mostly a design defect in the ghashtable compatibility variant.

For the find_random_keys benchmark:
Improved from 109.7% to 76.84% on x64.
Improved from 121.17% to 82.22% on ARM64.

Before (x64):

ght_find_random_keys: Warmed 18 time(s). Running 8192 iterations... 14 step(s): avg 17.507ns min 12.824ns max 21.249ns
dnght_find_random_keys: Warmed 21 time(s). Running 8192 iterations... 13 step(s): avg 19.211ns min 18.790ns max 19.541ns
dn_find_random_keys: Warmed 31 time(s). Running 8192 iterations... 19 step(s): avg 13.353ns min 12.913ns max 13.980ns

After (x64):

ght_find_random_keys: Warmed 16 time(s). Running 4096 iterations... 23 step(s): avg 21.244ns min 13.822ns max 23.902ns
dnght_find_random_keys: Warmed 24 time(s). Running 8192 iterations... 15 step(s): avg 16.325ns min 15.494ns max 16.779ns
dn_find_random_keys: Warmed 30 time(s). Running 8192 iterations... 18 step(s): avg 13.687ns min 13.139ns max 13.834ns

Before (arm64):

ght_find_random_keys: Warmed 6 time(s). Running 2048 iterations... 15 step(s): avg 69.279ns min 67.222ns max 70.292ns
dnght_find_random_keys: Warmed 5 time(s). Running 2048 iterations... 12 step(s): avg 83.949ns min 83.271ns max 84.813ns
dn_find_random_keys: Warmed 6 time(s). Running 2048 iterations... 15 step(s): avg 69.026ns min 68.155ns max 69.770ns

After (arm64):

ght_find_random_keys: Warmed 7 time(s). Running 2048 iterations... 15 step(s): avg 65.820ns min 64.640ns max 69.091ns
dnght_find_random_keys: Warmed 8 time(s). Running 2048 iterations... 19 step(s): avg 54.118ns min 53.717ns max 54.888ns
dn_find_random_keys: Warmed 10 time(s). Running 4096 iterations... 13 step(s): avg 40.598ns min 40.352ns max 40.794ns

@ghost ghost added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Mar 7, 2025
@kg kg changed the title Simdhash inline ght equals [mono] dn_simdhash_ght_compatible optimizations for ARM64 Mar 7, 2025
kg added 4 commits March 6, 2025 20:28
@kg
Use a macro for simdhash ght key comparer and key hasher since it see…
c30d631 
…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
@kg
Targeted simdhash optimizations via new dn_simdhash_ght_get_value_or_…
9296496 
…default
@kg
Fix indentation to satisfy CI build warnings
04a66a1
@kg
Fix build warnings
934cdcc
@kg kg force-pushed the simdhash-inline-ght-equals branch from 273d648 to 934cdcc Compare March 7, 2025 04:28
This was referenced Mar 7, 2025
Open
Closed
Open
@kg kg marked this pull request as ready for review March 7, 2025 09:52
Copilot AI review requested due to automatic review settings March 7, 2025 09:52
Copilot AI reviewed Mar 7, 2025
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 14 out of 14 changed files in this pull request and generated no comments.

@kg kg added area-VM-meta-mono and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Mar 7, 2025
@kotlarmilos kotlarmilos mentioned this pull request Mar 7, 2025
Closed
@kg kg mentioned this pull request Mar 7, 2025
Merged
@kg
Copy link
Member Author

kg commented Mar 11, 2025

Superceded by #113274

@kg kg closed this Mar 11, 2025
@github-actions github-actions bot locked and limited conversation to collaborators Apr 11, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

Copilot code review Copilot Copilot left review comments

@BrzVlad BrzVlad Awaiting requested review from BrzVlad BrzVlad is a code owner

@kotlarmilos kotlarmilos Awaiting requested review from kotlarmilos kotlarmilos is a code owner

@steveisok steveisok Awaiting requested review from steveisok steveisok is a code owner

@vitek-karas vitek-karas Awaiting requested review from vitek-karas vitek-karas is a code owner

@thaystg thaystg Awaiting requested review from thaystg thaystg is a code owner

Assignees

@kg kg

Labels

area-VM-meta-mono

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@kg