Make unique(f, itr) and unique!(f, itr) faster

[andyferris]

Avoid creation of a Set{Any} to make the function-providing forms faster, and address regression of unique(itr) perfermance from #30141.

cc @raghav9-97 @oxinabox @fredrikekre

Setup:

julia> x = rand(1:100_000, 1_000_000);

Before #30141:

julia> @btime unique(x);
  13.898 ms (49 allocations: 5.00 MiB)

After #30141:

julia> @btime unique(identity, x);
  40.192 ms (644863 allocations: 14.84 MiB)

After this PR:

julia> @btime unique(identity, x);
  14.092 ms (50 allocations: 5.00 MiB)

[oxinabox]

That is really clever.

[oxinabox]

How much does the manual specialisation on AbstractArray help performance?

[andyferris]

How much does the manual specialisation on AbstractArray help performance?

Where's this?

[chethega]

Can we do

else
    TT = promote_typejoin(eltype(seen), typeof(y))
    if eltype(seen) === TT
        push!(seen, y)
        else
             seen2 = ...
        end
end

Idea is that we don't grow the stack if f is e.g. type-unstable between Float32 and Float64.

[chethega]

Eh, maybe disregard. I misunderstoof promote_typejoin, which apparently doesn't do that. Point remains, do we want collect-style widening here (integer and float merge to Float64) or do we want exact widening as you did?

[nalimilan]

Wait, is this calling _unique! recursively each time a new value is encountered?! @chethega is right that this is bad, a call should only happen when encountering a new type.

[andyferris]

No - it simply pushes if the value is of type eltype of seen. If not, and only if not, then it constructs seen2 and makes a new call.

[oxinabox]

How much does the manual specialisation on AbstractArray help performance?

Where's this?

Nevermind, I misread the code.
I had thought unique(f, iter) and unique(f, arr::AbstractArray) were both defined.

[JeffBezanson]

Adding to 1.1 since this fixes a perf regression introduced on master.

[andyferris]

I’m happy for this to merge now, unless someone else wants to review?

[oxinabox]

Can an test be added:

the make sure the behavour of the following has not changed.

v = Vector{Any}(); push!(v, 10_000); push!(v, 10_000.0);  push!(v, 10_000f0)
@test unique(v) == Any[10_000]

It is not the behavour I expect, but it is the behavour of 1.0

[andyferris]

What didn’t you expect? The eltype, or the length 1 result?

[oxinabox]

The length 1 result.
I was not hugely surprised, but I do think it is the kinda behavour that might be easily changed by mistake.
I take it that behavour is maintained in this PR?

(On further thought, it would actually be hard to not maintain that behavour given all such numbers both hash the same and are isequal)

[andyferris]

The checks are always done via isequal (which is obviously different from == and ===, the first being bad for NaN and the latter bad for things like big numbers), and that part should be exactly the same.

[andyferris]

The hidden fact here is that in for Set uses isequal.

[andyferris]

Ok let’s merge - no point holding up 1.1.