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. (#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 JuliaLang/Pkg.jl#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]

When put into extension, these functions are kinda-piracy: loading Tables changes a completely independent method, eg Base.push!(::StructVector, ::Any).

[aplavin]

Maybe they aren't needed at all? Not sure.

[oschulz]

Hm, that's a very good point, we really shouldn't do that. That code may be necessary - we may have to keep Tables as a hard dependency. But maybe Tables itself could be made more lightweight using Pkg extensions or so?

[piever]

Ah, that's a very good point, I hadn't thought about that. Yes, we do need Tables for basic functionality like pushing or appending to a StructVector effectively, so that should definitely stay a hard dependency. Still, it is supposed to be an interface package, so that should be OK.

[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.