Avoid double-counting in Base.summarysize #54555
Conversation
| x::Union{Float64, Tuple{Float64, Float64}} | ||
| y::Union{Float64, Tuple{Float64, Float64}} | ||
| end | ||
| let s = S53061[S53061(rand(), (rand(),rand())) for _ in 1:10^5] |
There was a problem hiding this comment.
not a big deal, but does the array have to be this big to reproduce this?
There was a problem hiding this comment.
checked, the answer is yes, at least in the current form of the struct, more you reduce the size more you need attempts to find them unequal
There was a problem hiding this comment.
e.g. this needs 10 attempts more or less with 10^4: allequal(Base.summarysize(s) for i in 1:10) # usually false.
There was a problem hiding this comment.
why is this nondeterministic? That makes very little sense to me.
There was a problem hiding this comment.
to me too actually, but I'm not qualified enough to answer :D
There was a problem hiding this comment.
It's because the old code has to allocate a box for the value, which it should not have to do, and so the later uniquing by address becomes dependent on GC/allocator behavior. The correct way to fix this is to iterate over the pointers inside a struct instead of over its fields (some fields might be inlined but contain embedded pointers).
|
I believe I have a complete fix in #54606. Thanks for the tests. |
Ref #53061