-
Notifications
You must be signed in to change notification settings - Fork 116
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 robin_map/set::erase_fast()
method (fixes #75)
#76
Conversation
This commit introduces a new method ``void erase_it(iterator)`` to both set and map classes resembling the existing ``iterator erase(iterator)``. The main difference is that it does _not_ return an iterator, which is useful to avoid a performance pitfall explained in issue #75.
FWIW There are two locations that call |
I expanded the README.md paragraph a bit to add a broader discussion of performance pitfalls. People seem to run into the case #1 regularly because of bad hash functions, so I think it might be worth pointing out more specifically what can go wrong. |
By the way: alternatives to
Just a few ideas. |
Thank you for this! Minor comments:
But being more explicit with a dedicated section may be good. |
I renamed the function to Regarding the other point: looking back through the issue tracker history, a number of people seem to have run into performance pitfalls, so I think it is useful to spell out in more detail what can go wrong in a worst-case scenario. |
If these changes are acceptable to you, it would be great to merge them and declare a new minor version (1.3.0). |
robin_map/set::erase_it()
method (fixes #75)robin_map/set::erase_fast()
method (fixes #75)
Looks good, thanks! I merged the changes and will check to create a release later today. |
Awesome, thank you! |
See Tessil/robin-map#76 and #507 for context. (This is still a pre-release version of robin-map, I will update the referenced commit once a proper release has been made)
Change included in the latest release: https://github.com/Tessil/robin-map/releases/tag/v1.3.0 |
Hooray! Thank you for including this change, it makes my life much easier :). |
This commit introduces a new method
void erase_it(iterator)
to both set and map classes resembling the existingiterator erase(iterator)
.The main difference is that it does not return an iterator, which is useful to avoid a performance pitfall explained in issue #75.