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

Add mutable hash map #93

Merged
merged 12 commits into from
Mar 25, 2024
Merged

Add mutable hash map #93

merged 12 commits into from
Mar 25, 2024

Conversation

yj-qin
Copy link
Collaborator

@yj-qin yj-qin commented Mar 21, 2024

This PR adds a mutable hash map implements with Robin Hood Hashing. It based on an open-addressing hash table that uses linear probing to resolve hash collisions. When removing key-value pairs, it uses the shift-back method for reordering.

@yj-qin yj-qin force-pushed the feat-hashmap branch 2 times, most recently from 80f4efe to 5799d31 Compare March 22, 2024 00:29
@bobzhang bobzhang requested a review from peter-jerry-ye March 22, 2024 01:25
@yj-qin yj-qin force-pushed the feat-hashmap branch 2 times, most recently from d12fc41 to 84168f9 Compare March 22, 2024 04:17
hashmap/hashmap.mbt Outdated Show resolved Hide resolved
hashmap/hashmap.mbt Outdated Show resolved Hide resolved
@waterlens
Copy link

I feel the implementation of the hash table should be considered to reference modern implementations in Rust and Cpp. It's preferable to use integers rather than float points arithmetics to estimate the load factor. And it's not common to use a large load factor for Robin Hood hash table, considering that it will quickly be worse than a sorted array by hash values. Also for #90.

@yj-qin
Copy link
Collaborator Author

yj-qin commented Mar 22, 2024

I feel the implementation of the hash table should be considered to reference modern implementations in Rust and Cpp. It's preferable to use integers rather than float points arithmetics to estimate the load factor. And it's not common to use a large load factor for Robin Hood hash table, considering that it will quickly be worse than a sorted array by hash values. Also for #90.

Integer arithmetics will be better. Maybe we can do a benchmark to find a better load factor?

@waterlens
Copy link

waterlens commented Mar 22, 2024

I feel the implementation of the hash table should be considered to reference modern implementations in Rust and Cpp. It's preferable to use integers rather than float points arithmetics to estimate the load factor. And it's not common to use a large load factor for Robin Hood hash table, considering that it will quickly be worse than a sorted array by hash values. Also for #90.

Integer arithmetics will be better. Maybe we can do a benchmark to find a better load factor?

I think it should be fine between 0.5 and 0.75. But doing a benchmark should be a better approach for this.

@yj-qin yj-qin marked this pull request as draft March 22, 2024 17:17
@peter-jerry-ye
Copy link
Collaborator

@yj-qin What else do you expect to change apart from the load factor?
@bobzhang Do you think we need a benchmark to find the load factor or we can merge for now to provide the functionality first?

@yj-qin
Copy link
Collaborator Author

yj-qin commented Mar 24, 2024

@yj-qin What else do you expect to change apart from the load factor? @bobzhang Do you think we need a benchmark to find the load factor or we can merge for now to provide the functionality first?

I think It's ready to merge if current load factor (13/16 about 81.25%) is acceptable. We can optimize the load factor or the implementation of the hash table later.

@yj-qin yj-qin marked this pull request as ready for review March 24, 2024 02:51
@peter-jerry-ye
Copy link
Collaborator

@yj-qin If you are ready, we are merging this

@peter-jerry-ye peter-jerry-ye merged commit cdd9c7e into moonbitlang:main Mar 25, 2024
4 checks passed
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.

3 participants