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

[WIP] Use unboxed sums for change / no-change tracking #461

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

sjakobi
Copy link
Member

@sjakobi sjakobi commented May 12, 2022

…instead of pervasive ptrEq.

then NoChange
else Changed (Leaf h (L k x))
else Changed $ collision h l (L k x)
| otherwise = Changed $ runST (two s h k x hy t)
Copy link
Member Author

Choose a reason for hiding this comment

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

I suspect that we're less lazy in the result of two now, and that this is the actual source of the minor performance improvements.

Copy link
Member Author

Choose a reason for hiding this comment

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

I should try to make two itself more strict: Force the result of the recursive application of go, force the returned BitmapIndexed nodes.

@sjakobi
Copy link
Member Author

sjakobi commented May 12, 2022

With the benchmarks I'm getting a lot of noise and a rather reliable 2-3% speedup in HashMap.insert-dup.ByteString.

@treeowl
Copy link
Collaborator

treeowl commented May 12, 2022

The last time I experimented with this idea, the performance was worse, but maybe things have changed. Or maybe there are differences between HashMap and Map that explain the difference.

@treeowl
Copy link
Collaborator

treeowl commented May 12, 2022

FWIW, I think those old experiments used unboxed tuples with Int# tags rather than the (then bleeding edge) unboxed sums, but I don't know why that would make a difference, considering the way GHC implements unboxed sums.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants