Skip to content

Refactor and optimize two #521

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 10 commits into from
Closed

Refactor and optimize two #521

wants to merge 10 commits into from

Conversation

@sjakobi
Copy link
Member

@sjakobi sjakobi commented Oct 16, 2025 •
edited
Loading

TODO:

  • Create a type synonym ShiftedHash with utilities like shiftSH, maskSH etc
  • Check the Core

@sjakobi
two: Optimize computation of idx2 further
89c7baa 
This reduces the core size for the inner loop of two by 10 terms.
@sjakobi
Use two with Collisions too
aa19655 
This results in a ~10% reduction in Code size for each of the insert
variants.

Closes #447.
@sjakobi
Refactor two
324497b 
* Use the "shifted hash" approach.
* Create a variant `two'` that can be used to address #468.
@sjakobi
Update docs and remove outdated note
f1a4d3f 
Closes #335.
sjakobi
sjakobi commented Oct 16, 2025
View reviewed changes
@sjakobi sjakobi mentioned this pull request Oct 16, 2025
Draft
1 task
@sjakobi
Copy link
Member Author

sjakobi commented Oct 19, 2025 •
edited
Loading

So far, I'm seeing speedups of 2–4% on benchmarks for fromList and union with 100–10.000 elements.

@sjakobi sjakobi marked this pull request as ready for review October 19, 2025 17:12
@sjakobi sjakobi marked this pull request as draft October 19, 2025 17:53
sjakobi
sjakobi commented Oct 19, 2025
View reviewed changes
Data/HashMap/Internal.hs
Comment on lines +964 to +967
-- | This function lives at the top-level so 'two' and `two'` can be inlined
-- without inlining this loop.
two_go :: ShiftedHash -> HashMap k v -> ShiftedHash -> HashMap k v -> ST s (HashMap k v)
two_go !sh1 t1 !sh2 t2
Copy link
Member Author

Choose a reason for hiding this comment

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

Having this loop at the top-level nicely reduces the bloat, but functions like union seem to get slower by up to 4% :/

@sjakobi sjakobi mentioned this pull request Oct 19, 2025
Draft
@sjakobi
Copy link
Member Author

sjakobi commented Oct 19, 2025

I've extracted the (in my mind) unquestionable changes to #524.

I'll have another go at goDifferentHash etc once that is merged.

@sjakobi sjakobi closed this Oct 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@sjakobi