make reshaping SizedArrays not turn them into SArrays

[oxinabox]

I had an array, and I wanted to carry size information about it around in its type..
At first I thought, I can use a SArray, but then I remember my matrix might endup being something like 10^3 x 10^6 elements; and creating a SArray would be really slow.

I looked in the docs and I spotted SizedArray and I am likle Pefect.
So I am happily using that.
Then at some point I needed to call vec on my SizedArray,
and after I did that, it had turned into a SArray.
Which I really did not want, reasons outlined above.

This fixed that.
The code was basically already there, I just uncommented and updated it.

[coveralls]

Coverage remained the same at 81.88% when pulling 869b4af on oxinabox:ox/sizedarrayvec into 98db703 on JuliaArrays:master.

[c42f]

Interesting, but I'm not sure this helps in practice?

After this patch, StaticArrays still makes the assumption that "oh you have a StaticMatrix, I can just unroll all the things". So after this patch, you'll still have:

julia> @code_typed vec(SizedMatrix{2,2}([1 2; 3 4]))
CodeInfo(
1 ─ %1  = Base.getfield(a, :data)::Array{Int64,2}
│   %2  = Base.arrayref(false, %1, 1)::Int64
│   %3  = Base.getfield(a, :data)::Array{Int64,2}
│   %4  = Base.arrayref(false, %3, 2)::Int64
│   %5  = Base.getfield(a, :data)::Array{Int64,2}
│   %6  = Base.arrayref(false, %5, 3)::Int64
│   %7  = Base.getfield(a, :data)::Array{Int64,2}
│   %8  = Base.arrayref(false, %7, 4)::Int64
│   %9  = $(Expr(:foreigncall, :(:jl_alloc_array_1d), Array{Int64,1}, svec(Any, Int64), 0, :(:ccall), Array{Int64,1}, 4, 4))::Array{Int64,1}
│   %10 = %new(SizedArray{Tuple{4},Int64,1,1}, %9)::SizedArray{Tuple{4},Int64,1,1}
│         Base.arrayset(false, %9, %2, 1)::Array{Int64,1}
│         Base.arrayset(false, %9, %4, 2)::Array{Int64,1}
│         Base.arrayset(false, %9, %6, 3)::Array{Int64,1}
│         Base.arrayset(false, %9, %8, 4)::Array{Int64,1}
└──       return %10
) => SizedArray{Tuple{4},Int64,1,1}

So while the output type may be sensible, the amount of code generated even for simple operations like vec is proportional to the length of the input array. Not good for 10^9 elements!!

[oxinabox]

hmm, I really want to carry around this information in the type in the code I am writing.
I am writing a serializer to output DexLang binary serialized objects.
And DexLang keeps the sizes in their types, so it just makes things easier if I can do the same.

Do you think we should (in general) add a bail-out clause to a bunch of generated functions?

[c42f]

Do you think we should (in general) add a bail-out clause to a bunch of generated functions?

You mean to generate different code for different static array lengths? That's a reasonable workaround for exploding compile times and code size which we've discussed before. But nobody has put in the work to do it systematically across the package.

There are some tricky aspects to doing that:

• The mental model of StaticArrays is currently "expect it to unroll everything" which has the benefit of being simple. Generally my expectation has been that people should use a non-StaticArray if they want to user larger arrays.
• The best code depends on the target architecture so a long term solution has to involve good integration between StaticArrays and the compiler backend. I'm not really sure what that could look like but it's different from what we have now.
• One quite general problem is that unrolling generally allows map-like operations to infer the correct output eltype without any effort from us. If we didn't unroll we'd need another solution to this problem.

[c42f]

Hmm... Regardless of the problems with vec, we have basically done the "always preserve the static array type" change before for MArray and nobody complained: #327

So maybe that's an argument we should just go ahead with this.

It's certainly a more predictable way for the API to behave: just preserve all static array types as much as possible during computations, and let the user be (more) in charge of the data layout by letting them choose which static array type to supply to a function.

[oxinabox]

Hmm... Regardless of the problems with vec, we have basically done the "always preserve the static array type" change before for MArray and nobody complained: #327 So maybe that's an argument we should just go ahead with this.

yes, i think so. It will at least fix my small sized trials.

---

Generally my expectation has been that people should use a non-StaticArray if they want to user larger arrays.

My thoughts when I saw SizedArray was:
"Oh its an array that has size information, but isn't a tuple under-the-hood. That will be great for larger arrays"
I thought it would do most operations, like vec by falling back to calling vec on the backing array, and would just speed up somethings like bounds checking and maybe things that require nowing the size to preallocate (mapslices?)

Clearly I was wrong.

[KristofferC]

The mental model of StaticArrays is currently "expect it to unroll everything" which has the benefit of being simple. Generally my expectation has been that people should use a non-StaticArray if they want to user larger arrays.

FWIW, I think this shouldn't be the mental model. It should be "we give LLVM the explicit size of the arrays" and then it is up to LLVM to decide on unrolling vs not. This would be in a similar spirit to #494. The only problem with that right now is that we cannot mutate immutables so we have to unroll everything when we want the result to be a static array. However, in the future, I would say the static matrix multiply should look something like

function *(SMatrix{M, N, T}, {N, P, T}
    out = MMatrix{T, M, P}
    for ..., for ..., for ...
        out[i, j] += ...
   end
   return SMatrix(out)
end

where the allocation of out is elided. Then LLVM decides for what dim combinations unrolling is useful, how it should be unrolled etc.

That would avoid the amount of code scaling unbounded with the size of the arrays and we could then file performance bugs/fixes against LLVM for cases where it doesn't optimize properly. Giving the code with loops to LLVM should likely allow it to do a better job since it is "higher level", more closely matching the intent of the computation. For example, for SIMD stuff, pattern matching on the loops is likely easier (loop vectorizes) over pattern matching on the unrolled code (SLP vectorizer)

[c42f]

LLVM decides for what dim combinations unrolling is useful, how it should be unrolled etc

Right, I completely agree unrolling everything isn't ideal in principle. It's not a long term solution, but it is a good way to get fairly-ok performance with the current compiler and a really simple heuristic.

See also the parallel discussion in the LoopVectorization thread on discourse (starting about here: https://discourse.julialang.org/t/ann-loopvectorization/32843/63).

[c42f]

I thought it would do most operations, like vec by falling back to calling vec on the backing array, and would just speed up somethings like bounds checking and maybe things that require nowing the size to preallocate (mapslices?)

For good or bad that's not what SizedArray does right now, so changing it would certainly be breaking. I don't know exactly what @andyferris had in mind for SizedArray and I haven't used it in anger myself.

@andyferris @YingboMa @ChrisRackauckas I believe #327 was a success; certainly it addressed a pain point and nobody complained here afterward ;-) Given you were involved in that, I'd value your thoughts on whether preserving SizedArray through computations would also make sense here.

My main thought is that it would make the API more predictable so I'm inclined to do it.

[YingboMa]

I think this patch makes sense since it only changes the behavior of SizedArrays. It is expected that if people are using SizedArray, they have a large enough array that doesn't want to be stack-allocated.

[ChrisRackauckas]

I agree with that assumption. SizedArray is what I'd use only for when things are too big for an MArray or SArray to make sense, so if it was implicitly converting to those then likely anything I was using it for would break (or at least have issues) at the step where it converts.

[c42f]

Thanks people for weighing in on this one. Hearing no objections, and with several at least seemingly-good reasons to do this, I've gone ahead and merged this.