Empirical check that top_set_bit(abs(x)) == Base.ndigits0z(x, 2) is valid:

for T in [UInt8, Int8] 
    for i in typemin(T):typemax(T)
        for j in (i, big(i), Int(i), unsigned(Int(i)))
            Base.top_set_bit(abs(j)) == Base.ndigits0z(j, 2) || error()
        end
    end
end

While we're at it, is there a reason we're doing hash(Int64(num) << Int(pow), h) rather than hash(Int64(x))? (and similarly hash(Float64(x)) instead of hash(Int64(num) << Int(pow), h)

I suppose the existing version could be better for types that are slow to decompose and to convert to integers or floating-point values. I benchmarked no difference on any of

@btime hash(x) setup=(x=rand(Int)//1)
@btime hash(x) setup=(x=rand(Int)//1024)
@btime hash(x) setup=(x=rand(Int)//rand(Int))
@btime hash(x) setup=(x=rand(Int128))

for switching from num << pow to x in those three places, so the case to make that switch is pretty weak.

The first three won't hit it since there is a fast path for hash(x::Rational{<:BitInteger64}, h::UInt). The last will almost never hit it since random Int128s will almost always be bigger than typemax(Int64). You would want to test something like rational{BigInt} (where the pieces were rational) or something like that. We also should possibly be using unsafe_trunc here since we have verified that the arguments are in bounds.

I tested

@btime hash(x) setup=(x=big(rand(Int))//big(1))
@btime hash(x) setup=(x=big(rand(Int128))//big(1))

And got poor results

The reason for the regression for this PR is likely that top_set_bit(abs(x::BigInt)) is slow because the compiler can't infer nothrow. I have a fix for that.

@btime Base.top_set_bit(abs(x)) setup=(x=big(rand(Int128))); gets 1.7x faster with the second commit of this PR (though the compiler still can't infer nothrow), which changes the performance figures due to the first commit and the proposed change from bitshifting to just using the original x.

These performance data do not indicate that we should switch from bitshift to direct conversion. It's possible that that could be good in some cases, but I'll leave that for a different PR.

I reran all the benchmarks that have come up in this and #49998 and reported before => after all benchmarks are allocation free.

@btime Base.hash(x, UInt(1234)) setup=(x=Int128(rand(Int)));
  # 12.262 ns => 3.958 ns
@btime Base.hash(x, UInt(1234)) setup=(x=Int128(rand(Int128)));
  # 24.975 ns => 16.367 ns
@btime Base.top_set_bit(abs(x)) setup=(x=big(rand(Int)));
  # 11.469 ns => 2.541 ns
@btime Base.top_set_bit(abs(x)) setup=(x=big(rand(Int128)));
  # 11.512 ns => 2.583 ns
@btime hash(x) setup=(x=big(rand(Int))//big(1));
  # 33.442 ns => 24.807 ns
@btime hash(x) setup=(x=big(rand(Int128))//big(1));
  # 41.625 ns => 35.247 ns
@btime hash(x) setup=(x=rand(Int)//1);
  # 3.708 ns  => 3.750 ns
@btime hash(x) setup=(x=rand(Int)//1024);
  # 3.837 ns  => 3.833 ns
@btime hash(x) setup=(x=rand(Int)//rand(Int));
  # 14.780 ns => 14.780 ns
@btime hash(x) setup=(x=rand(Int128));
  # 23.845 ns => 16.784 ns

To be clear: are those benchmarks including #49998?

Those results are just this PR.

interesting. That somehow seems to be faster than #49998 which is rather surprising to me. I guess our compiler is doing a really good job cleaning this up for Int128.

Yes. It took a fair bit of fiddling before it matched the performance of #49998, but once it did I knew it was pretty good :). I expect the reason this appears more impactful than #49998 is that the impact is more pronounced on my machine than yours.

Test failures seem real.

Oops, I hadn't noticed :)

I think this is good to go, but I do kind of want a review from @vtjnash here.

Closing since #49996 gets the same performance.

(oops, closed the wrong one)