Make similar faster and safer #18107
Conversation
By converting the eltype argument into a ::Type{T} parameter, we avoid runtime method lookup.
Perhaps more importantly, the introduction of NeedsShaping makes `similar` safer, by preventing any possibility of an infinite recursion.
|
@nanosoldier |
|
Something went wrong when running your job: |
|
Since nanosoldier failed, it's worth saying that locally this resulted in modest improvements (15-40%) for about a dozen benchmarks, and no (reproducible) regressions. |
| similar{T}(a::AbstractArray{T}, dims::Tuple) = similar(a, T, to_shape(dims)) | ||
| similar{T}(a::AbstractArray{T}, dims::DimOrInd...) = similar(a, T, to_shape(dims)) | ||
| similar{T}(a::AbstractArray, ::Type{T}, dims::DimOrInd...) = similar(a, T, to_shape(dims)) | ||
| similar{T}(a::AbstractArray, ::Type{T}, dims::NeedsShaping) = similar(a, T, to_shape(dims)) |
There was a problem hiding this comment.
is this now a MethodError or handled elsewhere for dims not in this new Union?
There was a problem hiding this comment.
Anything that's not ::Dims or ::NeedsShaping definitely has to be handled elsewhere. That's been the design all along (see CustomUnitRanges.jl), but it turns out that if you mess up and fail to "catch" the similar method then it resulted in a StackOverflowError. This can't do that anymore.
There was a problem hiding this comment.
should this add a test_throws MethodError for such a case?
|
Hmm, somehow this got closed by @jrevels fixing nanosoldier. |
|
I'm canceling the CI; everything passed the first time. |
|
Oops, sorry about that. Gotta be more careful with my use of |
* Make similar faster and safer
By converting the eltype argument into a ::Type{T} parameter, we avoid runtime method lookup.
Perhaps more importantly, the introduction of NeedsShaping makes `similar` safer, by preventing any possibility of an infinite recursion.
* Test that similar throws a MethodError for unsupported dims types
(cherry picked from commit 66bacec)
* Make similar faster and safer
By converting the eltype argument into a ::Type{T} parameter, we avoid runtime method lookup.
Perhaps more importantly, the introduction of NeedsShaping makes `similar` safer, by preventing any possibility of an infinite recursion.
* Test that similar throws a MethodError for unsupported dims types
By converting the eltype argument into a ::Type{T} parameter, we avoid runtime method lookup.
Perhaps more importantly, the introduction of NeedsShaping makes
similarsafer, by preventing any possibility of an infinite recursion.