meggart / DiskArrays.jl

Other
72 stars 13 forks source link

Fix type stability issue with SubDiskArray #139

Closed wkearn closed 7 months ago

wkearn commented 7 months ago

I ran into a performance issue that I traced to a type instability in size(a::SubDiskArray).

In the existing definition of SubDiskArray

struct SubDiskArray{T,N} <: AbstractDiskArray{T,N}
    v::SubArray{T,N}
end

the child view is only parameterized by two of the five type parameters of SubArray, so it is a field with an abstract type, which leads to the type instability.

There are a few ways to fix this. The way proposed in this pull request is to parameterize by all of the type parameters of SubArray:

struct SubDiskArray{T,N,P,I,L} <: AbstractDiskArray{T,N}
    v::SubArray{T,N,P,I,L}
end

which gives the child array a concrete type and fixes the type inference problem and my performance issue. One could also do something like

struct SubDiskArray{T,N, A <: SubArray{T,N}} <: AbstractDiskArray{T,N}
    v::A
end

or try to put some constraints on P, I, and L from the types of the parent array. I don't think I understand the whole DiskArrays codebase enough to work out the correct form of that last option, though.

I've also added a test that throws an error with the current type instability and does not error with the new type definition.