I would actually have expected it to be slightly faster than rm2!, as you skip one in-like search inside of pop! when e \in S, but apparently not. Perhaps because the ternary pop! returns a Union-type?
If you modify the pop! source code (making use of internal methods) to return a Bool, something which should be faster than any approach wrapping pop!
function rm4!(s::Set, x)
index = Base.ht_keyindex(s.dict, x)
index < 1 && return false
Base._delete!(s.dict, index)
return true
end
I still get the same timing as rm2!, at least for the test above (with the same S0 and es)
I think the ternary pop! is convenient for the user, and thus somewhat rendered my this topic purposeless. (in practice, if we are sure that the returned element won’t be some certain value e.g. 0, then it can be in the place of nothing). So we don’t need to wrap it within a user-defined function like rm3!.
Another advantage of the ternary pop! is that it is usable for Dict either
julia> a = Dict(i => 10i for i = 1:2)
Dict{Int64, Int64} with 2 entries:
2 => 20
1 => 10
julia> pop!(a, 2, 0) # if we are sure `0` ∉ keys(a)
20
julia> pop!(a, 2, 0) # if we are sure `0` ∉ keys(a)
0
julia> a
Dict{Int64, Int64} with 1 entry:
1 => 10
If you’re interested in making sets (and dicts) more efficient, then have a look at the proposed “token interfaces”. The idea is that you can query the set for a key, and then you get a token back. This token then contain the information about the presence of they key.
So, the interface would be something like:
s = Set([1,2,3,4]) # make a set
token = gettoken(s, 3)
if ispresent(token)
delete!(s, token)
true
else
false
end
Interesting that’s the case. Cursory look at @code_llvm suggests rm2! doesn’t inline pop! (even if I contribute @inline to the call), so I wouldn’t expect it to be able to remove redundant code. In any case, I’d prefer eliminating redundancies on the semantic level, maybe with the new API above.
I’d never be comfortable with a requirement to never put an element into a set. In some cases the type system can separate the signal value’s type from the set element type, but not always e.g. Set{Any}. If you really want to go this direction, I’d be more comfortable if you made an entirely new and internal type documented to never be put into a collection. I definitely prefer the alternatives that don’t contribute another return type to the call; even nothing does that.