Using weakdeps to reduce load time on Julia v1.9

[oschulz]

Before (StructArrays v0.6.15):

julia> @time_imports import StructArrays, StaticArraysCore, Tables
      0.5 ms  DataValueInterfaces
      4.2 ms  DataAPI
      0.5 ms  IteratorInterfaceExtensions
      0.4 ms  TableTraits
    127.2 ms  Tables
      4.1 ms  StaticArraysCore
      0.2 ms  Adapt
      1.9 ms  GPUArraysCore
     34.5 ms  StructArrays

This PR:

julia> @time_imports import StructArrays, StaticArraysCore, Tables
      1.1 ms  DataAPI
      0.4 ms  Adapt
      1.7 ms  GPUArraysCore
     27.5 ms  StructArrays
      3.0 ms  StaticArraysCore
      0.7 ms  StructArrays → StructArraysStaticArraysCoreExt
      0.2 ms  DataValueInterfaces
      0.2 ms  IteratorInterfaceExtensions
      0.1 ms  TableTraits
     31.4 ms  Tables
      0.4 ms  StructArrays → StructArraysTablesExt

[oschulz]

Looks like the test failures on nightly are due to changed output formatting in doctests.

[oschulz]

@piever , @aplavin what do you think?

[aplavin]

I'm totally indifferent to this, because these packages are lightweight "*Core" interface packages anyway. Given extensions, is there any need to use eg StaticArraysCore instead of StaticArrays itself?

And I'm not a member of this repo anyway :)

[oschulz]

Given extensions, is there any need to use eg StaticArraysCore instead of StaticArrays itself?

I think for now it needs to be StaticArraysCore, because StructArrays will still depend on it with pre-v1.9 Julia.

[oschulz]

@aplavin: And I'm not a member of this repo anyway :)

I just looked at recent contributions. :-) But I think I should have asked @jishnub and @piever instead?

[jishnub]

I'm not a member either

[piever]

Makes sense, but maybe it'd be good for @N5N3 to look at this because of the overlap with #265. If I understand correctly, the issue is that depending on StaticArrays proper (rather than StaticArraysCore) allows for a more efficient implementation of broadcasting. In that case, maybe the simplest is to wait that 1.9 comes out and then use a weak dependency on StaticArrays?

Intuitively, I would think that the extra load time on older julia versions is a lesser issue that either worse performance or increased code complexity.

[oschulz]

If I understand correctly, the issue is that depending on StaticArrays proper (rather than StaticArraysCore) allows for a more efficient

Oh, right, then we should go for StaticArrays proper as in #265 .

[oschulz]

Shall I take StaticArrays out of this PR then, and only do Tables and GPUArraysCore in here?

[piever]

Let's wait just a little bit longer to see if @N5N3 has any opinion on this. But yes, maybe the simplest is to just do Tables and GPUArraysCore in here so that we avoid conflict with #265.

[N5N3]

LGTM. I think we can merge this without change. (https://github.com/JuliaArrays/StructArrays.jl/pull/265 needs rebasing anyway as IIUC we have decided not to use Requires.jl)

[piever]

Thanks for the input, then I imagine it can be merged as is (moving all functionality to extensions). Any ideas why CI is failing?

[N5N3]

IIRC, NamedTuple has been better printed on master. All failures are doctest error.

[piever]

I see, but what about the invalidation action? It seems as though it is struggling with Pkg operations.

[N5N3]

Looks like an upstream issue https://github.com/JuliaLang/Pkg.jl/issues/3327

[oschulz]

So can we merge this?

[oschulz]

Bump @piever

[piever]

The PR itself LGTM. I'm a bit uncomfortable that using weak dependencies breaks the invalidation CI, but it doesn't look like there's a way around this. I'm planning to merge and tag in the next few days (there are a few PRs about to be merged that can probably go in the same release).

[oschulz]

Thanks @piever !

[aplavin]

Can this get merged without Tables part, keeping Tables intergration as is?

[oschulz]

Can this get merged without Tables part, keeping Tables intergration as is?

I think so - should I just take the Tables part out?

[piever]

Sorry for the slow reply! Yes, this but without the Tables.jl part can be merged.

[oschulz]

Sorry, I lost track of this PR, I'll take Tables out.

[piever]

Sorry, I lost track of this PR, I'll take Tables out.

No worries! In the end, the relevant part of this PR has been incorporated in #265 so they'll end up merged together, I imagine this one here can be closed.

[oschulz]

Good point. let's just move on with #265, less merge conflicts then.