typekey_hash: length-independent run time for hashing long Vararg
#57795
Conversation
I'm not familiar with the C code, and I'm not sure this change is
correct, but playing with it in the REPL doesn't reveal any obvious
bugs.
Prior to this change, the run time of an operation like
`Tuple{Vararg{T, 1000000}} where {T}` scales linearly with the field
count (1000000 in this example). The goal of this change is to make
the run time be independent from the field count.
The idea is to simply return the same hash for all field counts greater
than or equal to a cut off.
Follows up on PR JuliaLang#50932, which introduced the loop linear in the field
count.
nsajko
added
performance
| @@ -1805,6 +1805,9 @@ static unsigned typekey_hash(jl_typename_t *tn, jl_value_t **key, size_t n, int | |||
| unsigned hashp = type_hash(p, &failed); | |||
| if (failed && !nofail) | |||
There was a problem hiding this comment.
No idea if I'm supposed handle failed or nofail somehow for this PR.
|
An alternative change I've been thinking about is to avoid the loop altogether when the Vararg length is greater than three. In that case we could simply bitmix the hashes for both the |
Types are not normalized, so no, this PR is not safe. Just do not make Tuples this large and you'll be fine. |
|
FTR this isn't about large tuples, that is, I never have tuple values that large, I just use types like |
|
Int values are already the natural numbers as a type, and do not have these issues |
I'm not familiar with the C code, and I'm not sure this change is correct, but playing with it in the REPL doesn't reveal any obvious bugs.
Prior to this change, the run time of an operation like
Tuple{Vararg{T, 1000000}} where {T}scales linearly with the field count (1000000 in this example). The goal of this change is to make the run time be independent from the field count.The idea is to simply return the same hash for all field counts greater than or equal to a cut off.
Follows up on PR #50932, which introduced the loop linear in the field count.