-
-
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
Fast paths for allunique
#43375
Fast paths for allunique
#43375
Conversation
Can we get another review on this? |
Pre-1.8 bump? |
false | ||
``` | ||
""" | ||
function allunique(C) | ||
if haslength(C) | ||
length(C) < 2 && return true |
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.
this check is redundant.
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.
I think this will catch length-1 objects which aren't StridedArrays, without first collecting them. I suppose transpose([1])
is an example.
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.
makes sense.
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.
Looks good to go, other than the one change I requested.
Bump? Test failures today seem to be Distributed |
rebasing to hope for clearer CI. |
This adds fast paths for
allunique(::Tuple)
andallunique(::AbstractArray)
, which just search linearly instead of making a dictionary. Both are used only forlength(x) < 32
.For arrays, the crossover point for the
Dict
being faster seems to usually be about there, I tried a few old & new computers, timingrand(n)
which should be the worst case. If there are many repeats, then linear search is much faster, even on longer arrays.These times aren't quite the latest version, sadly.
Vectors on an older computer:
Edit:
These are wrong for special floating point valuesnow fixed, and tested.