-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Introduce in!(x, s::Set) to improve performance of unique() #45156
Conversation
Regarding |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The patch itself LGTM
Initially, I was inspired by Lines 149 to 151 in 709daeb
|
This also ties into the long-standing issue of providing token-based API for dictionaries (#24454), which would also expose an interface to solve this problem. |
Token-based API seems best in the long term. Here, I have two minor comments. It would be nice to store Meanwhile, the PR seems ready to be merged. |
Functional API ( |
I think `in!` is a useful general function for users, and would be good to have as official API. Its semantics is clear and unambiguous, while providing a clear performance advantage over the naive implementation. For more evidence that this functionality is useful, consider: * Rust's `HashSet::insert` works just like this implementation of `in!` * This function was already used in the implementation of `Base.unique`, precisely for the performance over the naive approach Comes from #45156 with some initial discussion.
I think `in!` is a useful general function for users, and would be good to have as official API. Its semantics is clear and unambiguous, while providing a clear performance advantage over the naive implementation. For more evidence that this functionality is useful, consider: * Rust's `HashSet::insert` works just like this implementation of `in!` * This function was already used in the implementation of `Base.unique`, precisely for the performance over the naive approach Comes from JuliaLang#45156 with some initial discussion.
This PR introduces
in!
function not to computeht_keyindex
twice. It is not exported yet, but the fallback would be:Master
PR
(
allunique
has been already optimized, but withDict
.)