Skip to content

add a hash value to Typeofwrapper objects #49725

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

Merged
merged 1 commit into from
May 16, 2023
Merged

Conversation

vtjnash
Copy link
Member

@vtjnash vtjnash commented May 10, 2023

We probably should not do this, in full correctness, but the performance gain is too great to ignore. This is a very rare API mistake, but it occurred on a very popular constructor, making this very costly to construct a Broadcasted.

$ git grep 'struct.*<:Union{' base
base/broadcast.jl:struct Broadcasted{Style<:Union{Nothing,BroadcastStyle}, Axes, F, Args<:Tuple} <: Base.AbstractBroadcasted
base/compiler/ssair/ir.jl:struct InsertBefore{T<:Union{IRCode, IncrementalCompact}} <: Inserter
base/sort.jl:struct ScratchQuickSort{L<:Union{Integer,Missing}, H<:Union{Integer,Missing}, T<:Algorithm} <: Algorithm

We probably should not do this in full correctness, but the performance
gain is too great to ignore.
@KristofferC
Copy link
Member

but the performance gain is too great to ignore

A natural follow up question is what the actual numbers are.

@LilithHafner
Copy link
Member

ScratchQuickSort is internal so we could make it struct ScratchQuickSort{L, H, T<:Algorithm} <: Algorithm if that would help with compile times.

@vtjnash
Copy link
Member Author

vtjnash commented May 13, 2023

I think it is okay. We should try to make this work okay and correctly.

@vtjnash vtjnash merged commit c55000a into master May 16, 2023
@vtjnash vtjnash deleted the jn/hash-Typeofwrapper branch May 16, 2023 18:09
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.

3 participants