Skip to content
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

More hash optimization #50041

Merged
merged 3 commits into from
Jun 7, 2023

Conversation

oscardssmith
Copy link
Member

This is two separate optimizations.

The first is speeding up hash(::Union{Float16, Float32}) using the fact that they always are within the range of Int64 so you don't need to check that (or check whether they are within the UInt64 range.

The second is that AbstractFloats and Integers will never have a denominator that isn't 1, so we can change the rules of hashing to not hash the denominator if it is 1.

…d hash of arbitrary real types by not hashing the denominatior when it is one
@sjkelly
Copy link
Contributor

sjkelly commented Jun 2, 2023

Do you have benchmarks?

@oscardssmith
Copy link
Member Author

oscardssmith commented Jun 2, 2023

#Before
julia> @btime hash(x, UInt(1)) setup=x=rand(Int128)
  16.148 ns (0 allocations: 0 bytes)

julia> @btime hash(x, UInt(1)) setup=x=big(rand(Int128))
  18.352 ns (0 allocations: 0 bytes)

#After
julia> @btime hash(x, UInt(1)) setup=x=rand(Int128)
  11.877 ns (0 allocations: 0 bytes)

julia> @btime hash(x, UInt(1)) setup=x=big(rand(Int128))
  18.248 ns (0 allocations: 0 bytes)

@oscardssmith
Copy link
Member Author

Now that we've successfully unbroken hashing on master, I think this is ready to go.

@oscardssmith oscardssmith merged commit 36c5953 into JuliaLang:master Jun 7, 2023
6 checks passed
@oscardssmith oscardssmith deleted the more-hash-optimization branch June 7, 2023 13:26
@LilithHafner
Copy link
Member

For optimization 1, the OP is out of date, only Float16s are changed, not Float32s as well? I also don't see the speedup:

julia> @btime sum(hash, reinterpret(Float16, typemin(UInt16):typemax(UInt16)));
  224.000 μs (0 allocations: 0 bytes)

shell> git checkout more-hash-optimization -- base/float.jl base/gmp.jl base/rational.jl

julia> @btime sum(hash, reinterpret(Float16, typemin(UInt16):typemax(UInt16)));
  223.333 μs (0 allocations: 0 bytes)

@oscardssmith
Copy link
Member Author

Updated benchmarks. I agree that I'm not seeing much improvement for Float16. Not fully sure why.

@LilithHafner
Copy link
Member

If this doesn't provide measurable performance benefits, we should probably remove it:

julia/base/float.jl

Lines 658 to 669 in 0a2d6fc

function hash(x::Float16, h::UInt)
# see comments on trunc and hash(Real, UInt)
if isfinite(x) # all finite Float16 fit in Int64
xi = fptosi(Int64, x)
if isequal(xi, x)
return hash(xi, h)
end
elseif isnan(x)
return hx_NaN h # NaN does not have a stable bit pattern
end
return hash_uint64(bitcast(UInt64, Float64(x))) - 3h
end

oscardssmith added a commit that referenced this pull request Jul 26, 2023
This fixes 2 bugs introduced by #49996 and #50041.
Closes #50628.
KristofferC pushed a commit that referenced this pull request Aug 10, 2023
This fixes 2 bugs introduced by #49996 and #50041.
Closes #50628.

(cherry picked from commit c777c71)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants