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

Remove WormMap and WormSet. #235

Merged
merged 2 commits into from
May 19, 2024

Conversation

bruno-roustant
Copy link
Collaborator

Since the latest improvements to HashMap, in practice WormMap is not useful compared to HashMap. For example, in Lucene, WormMap and HashMap were compared and HashMap was chosen.
If I had some improvements to bring, I would work on HashMap.
So, maybe we can remove WormMap and WormSet to reduce the HPPC lib?

@dweiss
Copy link
Member

dweiss commented May 19, 2024

This is entirely up to you, @bruno-roustant . If you think there is no clear advantage to using WormMap then I think it makes sense to get rid of them to decrease library size. I did a quick lookup of how many times people may be using it [1] but I don't think it's used that frequently, so perhaps we should go ahead and remove.

[1] https://github.com/search?q=WormMap%20path%3A*.java&type=code

@dweiss
Copy link
Member

dweiss commented May 19, 2024

If you decide to go ahead, then perhaps add an entry to changes.txt?

@bruno-roustant
Copy link
Collaborator Author

Thanks for searching usages. I agree it is not used much except in benchmarks.
I added an entry in CHANGES, in the 0.10.0 section since it is an API change I think.

@dweiss
Copy link
Member

dweiss commented May 19, 2024

Yep, looks great.

@bruno-roustant bruno-roustant merged commit cf83546 into carrotsearch:master May 19, 2024
2 checks passed
@bruno-roustant bruno-roustant deleted the remove_worm branch May 19, 2024 15:15
@dweiss dweiss added this to the 0.10.0 milestone May 21, 2024
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