Skip to content
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

StaticArrays load time, possibility for abstract interface package? #1023

Closed
oschulz opened this issue May 7, 2022 · 38 comments
Closed

StaticArrays load time, possibility for abstract interface package? #1023

oschulz opened this issue May 7, 2022 · 38 comments

Comments

@oschulz
Copy link

oschulz commented May 7, 2022

StaticArrays (together with ArraysInterface) seems to be a driving factor in package load times nowadays. For example, the load time of FiniteDiff, FiniteDifferences and ForwardDiff (common direct/indirect dependencies themselves) themselves is mainly due to StaticArrays (and ArrayInterface):

julia> @time_imports using FiniteDiff
      3.3 ms  ┌ Compat
      0.5 ms  ┌ Requires
    548.3 ms  ┌ StaticArrays
      0.2 ms  ┌ IfElse
     33.7 ms    ┌ Static
    737.6 ms  ┌ ArrayInterface
   1312.8 ms  FiniteDiff
julia> @time_imports using FiniteDifferences
      3.6 ms  ┌ Compat
      0.4 ms  ┌ Richardson
    539.9 ms  ┌ StaticArrays
     62.7 ms  ┌ ChainRulesCore
    617.1 ms  FiniteDifferences
julia> @time_imports using ForwardDiff
    538.5 ms    ┌ StaticArrays
    548.4 ms  ┌ DiffResults
      4.1 ms  ┌ Compat
      0.4 ms  ┌ NaNMath
     60.8 ms    ┌ ChainRulesCore
     62.0 ms  ┌ ChangesOfVariables
      0.7 ms  ┌ OpenLibm_jll
      3.0 ms  ┌ DocStringExtensions
      4.6 ms  ┌ IrrationalConstants
      0.6 ms  ┌ CompilerSupportLibraries_jll
      1.4 ms    ┌ LogExpFunctions
     18.9 ms        ┌ Preferences
     19.8 ms      ┌ JLLWrappers
     23.9 ms    ┌ OpenSpecFun_jll
     37.6 ms  ┌ SpecialFunctions
     10.3 ms    ┌ MacroTools
     11.0 ms  ┌ CommonSubexpressions
      0.8 ms  ┌ DiffRules
    845.2 ms  ForwardDiff

Even the load time of a "large" package like Optim is completely dominated by StaticArrays and ArrayInterface (indirectly, via FiniteDiff and ForwardDiff):

julia> @time using StaticArrays
  0.565813 seconds (2.21 M allocations: 175.407 MiB, 0.51% compilation time)

julia> @time using ArrayInterface
  0.786209 seconds (697.28 k allocations: 38.650 MiB, 4.49% gc time, 89.27% compilation time)

julia> @time using FiniteDiff
  0.037383 seconds (46.28 k allocations: 2.584 MiB, 77.77% compilation time)

julia> @time using ForwardDiff
  0.199031 seconds (543.28 k allocations: 35.849 MiB, 2.18% compilation time)

julia> @time using Optim
  0.302545 seconds (730.67 k allocations: 53.416 MiB)

There's currently and effort to make ArrayInterface more lightweight by splitting it up (JuliaArrays/ArrayInterface.jl#211) - I wonder if the same would be possible for StaticArrays? Would something like an AbstractStaticArrays be enough for packages like the above? I think at least some of them only "support" static arrays but don't use them for their core functionality (I may be wrong).

@mateuszbaran
Copy link
Collaborator

As far as I know the time spent in loading StaticArrays is dominated insertion of methods with complicated signatures (for example five-argument static structured matrix multiplication). I would totally support making something like StaticArraysCore.jl that is just a copy of StaticArrays.jl with some of these complicated methods removed. Then StaticArrays.jl would just import and extend StaticArraysCore.jl. People using StaticArrays.jl wouldn't see any difference but they would have the option to just depend on StaticArraysCore.jl if they don't need that additional functionality.

The most complicated part of work here is determining which methods shouldn't be moved to StaticArraysCore.jl which requires some cooperation from maintainers of major packages that use StaticArrays.jl.

@oschulz
Copy link
Author

oschulz commented May 7, 2022

but they would have the option to just depend on StaticArraysCore.jl

Yes - there are some packages that really utilize StaticArrays for what they do - they're little that can be done there, of course. But then there's packages that only need StaticArrays to add some compatibility with it (e.g. this code I have in ArraysOfArrays, via @require to not depend on StaticArrays, but that's kinda ugly). For those cases a package StaticArraysCore should help a lot.

I'm not quite sure in which of these categories ForwardDiff falls (StaticArrays is a big part of it's load time).

@mateuszbaran
Copy link
Collaborator

I've just made a quick test and just removing advanced linear algebra gets us down to about 0.17-0.20 s (depending on the details). Would that be fine? Or do you think it's still too much?

@oschulz
Copy link
Author

oschulz commented May 7, 2022

I guess it depends on how much it is relative to the package that depends on StaticArrays. :-)

I worry a bit about having a "stripped down" version of StaticArrays that contains concrete types (not just and interface) without full functionality. In applications that use a lot of generic code, the "full" StaticArrays might not get loaded automatically, leading to non-performant behavior (or maybe even errors due to missing functionality).

@oschulz
Copy link
Author

oschulz commented May 7, 2022

Maybe having abstract static arrays would be enough to satisfy the needs of many packages now depending on StaticArrays, esp. if they use similar() and the like to create new arrays?

@mateuszbaran
Copy link
Collaborator

If a package only had the abstract type and some functions, it wouldn't have SVector so it would be insufficient even for your example from ArraysOfArrays. I personally doubt just dispatching on StaticArray is particularly useful but feel free to prove me wrong :).

@oschulz
Copy link
Author

oschulz commented May 8, 2022

it wouldn't have SVector so it would be insufficient even for your example from ArraysOfArrays.

I have to admit that example wasn't ideal. flatview part could be done, though, as something like

@inline flatview(A::AbstractArray{SA,N}) where {S,T,M,N,SA<:AbstractStaticArray{S,T,M}} =
    reshape(reinterpret(T, A), size(SA)..., size(A)...)

And I could actually drop nestedview as currently designed.

But you're right, an abstract interface would not be sufficient. It would be better to move these definitions from ArraysOfArrays into StaticArrays itself. I'm currently in the process of moving the AbstractArrays interface into AbstractArraysOfArrays (currently under design). AbstractArraysOfArrays will be super-lightweight, so it should be an acceptable dependency for StaticArrays.

@ChrisRackauckas
Copy link
Member

ChrisRackauckas commented May 21, 2022

I did an audit of the SciML codebase. We recently removed all usage of Requires.jl. Our measurements show that StaticArrays.jl and GPUArrays.jl are by far and away the majority of the startup time. Period. No questions there anymore, thanks for @time_imports.

Codebase Audit

And from the codebase:

https://github.com/SciML/RecursiveArrayTools.jl/blob/master/src/utils.jl#L4
https://github.com/SciML/SciMLBase.jl/blob/bfc028d395bbc846f9a75a370a74bad41f72a007/src/operators/basic_operators.jl#L108
https://github.com/SciML/DiffEqBase.jl/blob/8e38359319ed5cb9b8bc469ec7975b36db0fee0a/src/solve.jl#L489
https://github.com/SciML/DiffEqBase.jl/blob/b533f1255da9a1461f8c25787cec77ffc6037ff0/src/common_defaults.jl#L12-L26
https://github.com/SciML/ResettableStacks.jl/blob/master/src/core.jl#L28
https://github.com/SciML/DEDataArrays.jl/blob/9f7e853d359aeb14be515943eeb4b98ddb2a2c17/src/DEDataArrays.jl#L51
https://github.com/SciML/DiffEqPhysics.jl/blob/7bec56f72c0c6e71b50e498dfdcb691e92f91d52/src/hamiltonian.jl#L29
https://github.com/SciML/NonlinearSolve.jl/blob/51c443eae5ca5e4d54067c4ca5db14792c3240c2/src/utils.jl#L189
https://github.com/SciML/StochasticDelayDiffEq.jl/blob/233e46dff6cafb938feec059796179b29d092a3f/src/solve.jl#L243
https://github.com/SciML/SimpleDiffEq.jl/blob/ef0cece5781686dbaaebc019de40d927e61fa6f1/src/SimpleDiffEq.jl#L10
https://github.com/SciML/SimpleDiffEq.jl/blob/ef0cece5781686dbaaebc019de40d927e61fa6f1/src/rk4/gpurk4.jl
https://github.com/SciML/SimpleDiffEq.jl/blob/ef0cece5781686dbaaebc019de40d927e61fa6f1/src/tsit5/gpuatsit5.jl
https://github.com/SciML/SimpleDiffEq.jl/blob/ef0cece5781686dbaaebc019de40d927e61fa6f1/src/tsit5/tsit5.jl
https://github.com/SciML/SimpleDiffEq.jl/blob/11c7f9e726f4d4d66cf2903a02f650be72c637e9/src/tsit5/atsit5.jl
https://github.com/SciML/ModelingToolkit.jl/blob/1b98b86ba716a06150fb6a7bf03ca2d52bc5ce9f/src/structural_transformation/codegen.jl#L184
https://github.com/JuliaSymbolics/SymbolicUtils.jl/blob/7f8cfbd026972a61a727a904e9b182367921093f/src/code.jl#L529-L564

If an interface package just had StaticArray, SArray, and MArray (and StaticArrays.StaticArrayStyle, though honestly that usage may be able to be deleted), then one of the libraries everyone points to as the big case of large load times will almost completely eliminate the load times.

There's two other cases in the SciML code base to look at.

With one outlier using a bit more:

https://github.com/SciML/LabelledArrays.jl/blob/edfffa99a85054aa7f184c9b90b4ba8fc6f8858b/src/slarray.jl#L31-L197

LabelledArrays.jl is a bit weird because it's defining a StaticArray kind of type. Even then, it only needs StaticArrays.similar_type and StaticArrays.LU.

The other case could just be tuples:

https://github.com/SciML/OrdinaryDiffEq.jl/blob/3a364726f8a464470a7fa03f1dae3168741124c4/src/caches/bdf_caches.jl#L292
https://github.com/SciML/OrdinaryDiffEq.jl/blob/3076b4ef7015cc3d683a82087761d56f20ac43d9/src/bdf_utils.jl#L80

so ignore those (@YingboMa we might as well just convert them now to clean the code base.

Conclusion

It's clear from the data: if there was a core library with StaticArray, SArray, MArray, StaticArrays.StaticArrayStyle, StaticArrays.similar_type, and StaticArrays.LU, then the majority of the SciML load times would be gone. This is because we only need to check if someone gives us static arrays, there are no static arrays used internally.

To me, that's a very strong indicator that there should be such an interface package.

@oschulz
Copy link
Author

oschulz commented May 21, 2022

if there was a core library with [...] This is because we only need to check if someone gives us static arrays

Does that mean that even an AbstractStaticArray and so on would be sufficient?

@ChrisRackauckas
Copy link
Member

Well, we need to know the difference between MArray and SArray since MArray is mutable, so we want a higher level type for immutable static arrays, but yes if there was an abstract type for those then we'd have all we need because we could just do parameterless_type(x)(y) as a constructor and such.

@oschulz
Copy link
Author

oschulz commented May 22, 2022

Well, we need to know the difference between MArray and SArray since MArray is mutable

So maybe an AbstractStaticArray with subtypes AbstractImmutableStaticArray and AbstractMutableStaticArray?

because we could just do parameterless_type(x)(y) as a constructor and such.

Wouldn't ConstructionBase.constructorof be a clean solution here?

@Tokazama
Copy link
Member

Tokazama commented Jun 2, 2022

ArrayInterface has drastically improved load times recently and provides much of the tooling necessary to propagate static information between methods. The tools to construct an immutable array in place are not yet fully implemented but most of the hesitation in implementing that has been due to the lack of solid details on how ImmutableArray will work when implemented in base.

@mateuszbaran
Copy link
Collaborator

I've started working on an small interface package: https://github.com/JuliaArrays/StaticArraysCore.jl . What do you think?

@ChrisRackauckas
Copy link
Member

I think that's precisely what we need.

@Tokazama
Copy link
Member

Tokazama commented Jun 2, 2022

Should we consider moving ArrayInterfaceStaticArrays stuff into that package?

@mateuszbaran
Copy link
Collaborator

Update: I have finished the first version of StaticArraysCore.jl and triggered registration: JuliaRegistries/General#61901 . Once that goes through I will make a PR to StaticArrays.jl that makes it implement that interface.

@thchr
Copy link
Collaborator

thchr commented Jun 7, 2022

Would it make sense to put StaticArraysCore.jl in the same repo as StaticArrays.jl (but in a subfolder)? (As in Makie, ArrayInterface, etc.)

@mateuszbaran
Copy link
Collaborator

I have never made a repository with more than one package so I don't know.

@thchr
Copy link
Collaborator

thchr commented Jun 7, 2022

It works just the same as the usual thing, except the package lives in a subdirectory and registrations use @JuliaRegistrator register subdir=StaticArraysCore instead of just @JuliaRegistrator register. I have a simple example here.

I think the main things to consider are integration with CI; i.e., does one test StaticArrays.jl on the master of StaticArraysCore or on the most recently released version. One benefit is that all of StaticArrays.jl's test could run on PRs to StaticArraysCore.jl as well.

@JuliaArrays JuliaArrays deleted a comment from JuliaRegistrator Jun 7, 2022
@JuliaArrays JuliaArrays deleted a comment from JuliaRegistrator Jun 7, 2022
@JuliaArrays JuliaArrays deleted a comment from JuliaRegistrator Jun 7, 2022
@Tokazama
Copy link
Member

Tokazama commented Jun 7, 2022

We could just merge the two. We already replace the static ranges with our types in ArrayInterface.

@mateuszbaran
Copy link
Collaborator

OK, so if we wanted to have StaticArraysCore.jl in this repository, how would we do that? Do we keep the current StaticArraysCore.jl registration PR open and then change the repository URL? Or close that PR and trigger a new registration from here?

@thchr
Copy link
Collaborator

thchr commented Jun 7, 2022

I don't know which is easier; either could work, I believe (but maybe just closing that PR and opening a new here would be easier?).

If we do the latter, it could be a PR to StaticArrays.jl that creates a subfolder StaticArraysCore.jl with all the content (modulo the CI stuff) from the current StaticArraysCore.jl repo. After we merge that, we could then register it in the usual way (just with the additional subdir=StaticArraysCore bit).

@Tokazama
Copy link
Member

Tokazama commented Jun 7, 2022

It's really up to you what you want to do. It's easier to keep the conversation for PRs all in one place, but it's also easier to track code coverage in separate repos

@mateuszbaran
Copy link
Collaborator

Since the repository is already created, maybe let's keep it for now. It can always be changed later.

@Tokazama
Copy link
Member

Tokazama commented Jun 7, 2022

I can make a PR to StaticArraysCore in a bit to merge the features of ArrayInterfaceStaticArrays so we can see what that would look like.

@ChrisRackauckas
Copy link
Member

Sub repos seemed to be a lot simpler to setup if the sub is a dependent on the main, not the other way around. ArrayInterfaceCore flipped this and caused a few CI hiccups, but it is nice to be able to test it all together given how it's interrelated. Repos like Optimization.jl where all of the subs depend on Optimization.jl (another case, KernelAbstractions.jl) seem to be the most ideal use case for a multi-package repo.

@mateuszbaran
Copy link
Collaborator

I can make a PR to StaticArraysCore in a bit to merge the features of ArrayInterfaceStaticArrays so we can see what that would look like.

Which features? ArrayInterfaceStaticArrays seems to implement the ArrayInterfaceCore for StaticArrays, and StaticArraysCore is not supposed to depend on ArrayInterfaceCore. As I understand it, ArrayInterfaceStaticArrays should just switch to depending on StaticArraysCore instead of StaticArrays.

@oschulz
Copy link
Author

oschulz commented Jun 7, 2022

Repos like Optimization.jl where all of the subs depend on Optimization.jl

Is Optimization a successor of GalacticOptim?

@JuliaArrays JuliaArrays deleted a comment from JuliaRegistrator Jun 8, 2022
@Tokazama
Copy link
Member

Tokazama commented Jun 8, 2022

I can make a PR to StaticArraysCore in a bit to merge the features of ArrayInterfaceStaticArrays so we can see what that would look like.

Which features? ArrayInterfaceStaticArrays seems to implement the ArrayInterfaceCore for StaticArrays, and StaticArraysCore is not supposed to depend on ArrayInterfaceCore. As I understand it, ArrayInterfaceStaticArrays should just switch to depending on StaticArraysCore instead of StaticArrays.

The only reason it depends on StaticArrays is because we needed to get rid of Requires. The goal with a lot of the "libs/*" in the ArrayInterface repo is to eventually get to a point where things are stable enough for a proper dependency tree. ArrayInterfaceStaticArrays is just dedicated piracy right now.

@ChrisRackauckas
Copy link
Member

Is Optimization a successor of GalacticOptim?

Yes. It's GalacticOptim but everyone on the Discourse and Slack voted for a rename, so it happened. Same with Quadrature -> Integrals. Now https://docs.sciml.ai/dev/ is a lot more clear.

@ChrisRackauckas
Copy link
Member

The only reason it depends on StaticArrays is because we needed to get rid of Requires. The goal with a lot of the "libs/*" in the ArrayInterface repo is to eventually get to a point where things are stable enough for a proper dependency tree. ArrayInterfaceStaticArrays is just dedicated piracy right now.

Or the best thing would be to start upstreaming a lot more of it to Base.

@oschulz
Copy link
Author

oschulz commented Jun 8, 2022

Yes. It's GalacticOptim but everyone on the Discourse and Slack voted for a rename, so it happened.

I like it! Now we just need to get it's load time down further. ;-)

@oschulz
Copy link
Author

oschulz commented Jun 11, 2022

I've started working on an small interface package: https://github.com/JuliaArrays/StaticArraysCore.jl

Awesome! I'd love to start using this, e.g. to make ArraysOfArrays.jl get by without @require! What's the roadmap for making StaticArrays.jl itself use StaticArraysCore.jl ?

@mateuszbaran
Copy link
Collaborator

I'm planning to submit a PR to StaticArrays.jl that makes it depend on StaticArraysCore.jl next week.

@oschulz
Copy link
Author

oschulz commented Jun 11, 2022

Thanks a lot @mateuszbaran !

@mateuszbaran
Copy link
Collaborator

I have made the PR: #1045, now I need a review 🙂 .

@mateuszbaran
Copy link
Collaborator

So, this is now completed by merging #1045.

@oschulz
Copy link
Author

oschulz commented Jun 24, 2022

Thanks so much, @mateuszbaran !

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

No branches or pull requests

5 participants