Skip to content

Commit

Permalink
Ensure hash(::Platform) is stable (#37734)
Browse files Browse the repository at this point in the history
This allows us to key dictionaries with `Platform` objects more reliably
  • Loading branch information
staticfloat committed Sep 27, 2020
1 parent 4165fdd commit 91d7e18
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 8 deletions.
17 changes: 14 additions & 3 deletions base/binaryplatforms.jl
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,20 @@ function Base.setindex!(p::AbstractPlatform, v::String, k::String)
return p
end

# Hash definitino to ensure that it's stable
function Base.hash(p::Platform, h::UInt)
h += 0x506c6174666f726d % UInt
h = hash(p.tags, h)
h = hash(p.compare_strategies, h)
return h
end

# Simple equality definition; for compatibility testing, use `platforms_match()`
function Base.:(==)(a::Platform, b::Platform)
return a.tags == b.tags && a.compare_strategies == b.compare_strategies
end


# Allow us to easily serialize Platform objects
function Base.repr(p::Platform; context=nothing)
str = string(
Expand All @@ -166,9 +180,6 @@ function Base.show(io::IO, p::Platform)
print(io, str)
end

# Simple equality definition; for compatibility testing, use `platforms_match()`
Base.:(==)(a::AbstractPlatform, b::AbstractPlatform) = tags(a) == tags(b)

function validate_tags(tags::Dict)
throw_invalid_key(k) = throw(ArgumentError("Key \"$(k)\" cannot have value \"$(tags[k])\""))
# Validate `arch`
Expand Down
19 changes: 14 additions & 5 deletions test/binaryplatforms.jl
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,16 @@ end

# Test that trying to set illegal tags fails
@test_throws ArgumentError p["os"] = "a+b"

# Test that our `hash()` is stable
@test hash(HostPlatform()) == hash(HostPlatform())

# Test that round-tripping through `triplet` for a does not
# maintain equality, as we end up losing the `compare_strategies`:
p = Platform("x86_64", "linux"; cuda = v"11")
Base.BinaryPlatforms.set_compare_strategy!(p, "cuda", Base.BinaryPlatforms.compare_version_cap)
q = parse(Platform, triplet(p))
@test q != p
end

@testset "Triplet parsing" begin
Expand Down Expand Up @@ -186,13 +196,12 @@ end
# Round-trip our little homie through `triplet()`, with some bending
# of the rules for MacOS and FreeBSD, who have incomplete `os_version`
# numbers embedded within their triplets.
p = HostPlatform()
if !Sys.isbsd(p)
@test parse(Platform, triplet(p)) == p
end
p = Platform("x86_64", "linux")
@test parse(Platform, triplet(p)) == p

# Also test round-tripping through `repr()`:
@test eval(Meta.parse(repr(HostPlatform()))) == HostPlatform()
p = Platform("aarch64", "macos"; os_version=v"20", march="armv8_4_crypto_sve")
@test eval(Meta.parse(repr(p))) == p
end

@testset "platforms_match()" begin
Expand Down

2 comments on commit 91d7e18

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Executing the daily benchmark build, I will reply here when finished:

@nanosoldier runbenchmarks(ALL, isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @ararslan

Please sign in to comment.