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

Replace BoltMap internals with ahash/fxmap #51

Open
knutwalker opened this issue Feb 15, 2023 · 4 comments
Open

Replace BoltMap internals with ahash/fxmap #51

knutwalker opened this issue Feb 15, 2023 · 4 comments
Labels

Comments

@knutwalker
Copy link
Collaborator

No description provided.

@thelonelyvulpes
Copy link

@knutwalker if you want to flesh out the considerations I am happy to pick this up.
I have a basic replacement done with aHash (thelonelyvulpes#1), but I wanted to explore boltmap's traits to improve ergonomics.

@knutwalker
Copy link
Collaborator Author

@thelonelyvulpes thanks for the input. The other thing right now we would want to consider is #52, where we might want to use a Vec<(BoltString, BoltType)> instead of a map. For smaller map sizes, the linear lookup can be faster than going through hashing and finding a free slot, but we want to properly benchmark this (there is some precedent in this article).
Using an ahashmap internally is a good first step regardless and we would want to benchmark against that, so I think we can go forward with this issue. We'll be happy to accept your contributions and if you want you can also split it into updating the internals end exploring the traits.

@thelonelyvulpes
Copy link

I have been learning a bit about Criterion should have some benchmarks to look at soon, been busy working some other pet projects so will try and tackle wrapping up this relatively small change, do we need to worry breaking changes?

@knutwalker
Copy link
Collaborator Author

We've a bunch of other things planned that would be breaking, at the very least an update to support 5.x and drop 4.y where y < 4, so I think we don't need to worry about it too much right now.

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

No branches or pull requests

2 participants