Optimize pop!(s::Set, x, default)

[petvana]

Minor optimization to compute index in Dict only once.

This PR should not be merged before #52017.

Master

  126.417 μs (1 allocation: 16 bytes)
  147.812 μs (1 allocation: 16 bytes)

PR

  86.494 μs (1 allocation: 16 bytes)
  156.912 μs (1 allocation: 16 bytes)

Testing code

using BenchmarkTools

function PR_pop!(s::Set, x, default)
    dict = s.dict
    index = Base.ht_keyindex(dict, x)
    if index > 0
        @inbounds key = dict.keys[index]
        Base._delete!(dict, index)
        return key
    else
        return default
    end
end

N = 10000
x = collect(1:N)
x_negative = collect(-N:-1)

function pop_all(s, x)
    for v in x
        pop!(s, v, -1)
    end
end

function pop_all_PR(s, x)
    for v in x
        PR_pop!(s, v, -1)
    end
end

# Master
@btime pop_all(s, x) setup=(s=Set(x))
@btime pop_all(s, x_negative) setup=(s=Set(x))

# PR
@btime pop_all_PR(s, x) setup=(s=Set(x))
@btime pop_all_PR(s, x_negative) setup=(s=Set(x))

[jakobnissen]

I'm curious why it is slower for x_negative?
Anyway, your benchmark is the best possible case for the implementation on master - if hashing is more expensive, this PRs implementation will pull ahead.

[petvana]

Yes, the speedup is about 30% here, but should be close to 50% for expensive hashing function.

The slowdown for x_negative (returning default value) is almost negligible and I guess it depends on specific clang optimizations.

[petvana]

Once PR passes CI, it seems ready.