Make package a bit more lightweight and improve loading time

[devmotion]

In the spirit of #53, this makes the package a bit more lightweight and faster to load, in particular on Julia < 1.9:

• Implement package extensions as described in the Pkg docs without PkgExtensionsCompat.jl. This removes an additional layer of complexity and possible source of bugs (it seems most/all issues with the package are fixed but as former issues regarding unsupported use cases and bugs show this concern is real) and significant overhead in older Julia versions where the code in PkgExtensionsCompat.jl is executed. For instance, on Julia 1.6.5 I get on the master branch

  julia> ] precompile
  
  julia> @time using DynamicQuantities
  0.359792 seconds (1.32 M allocations: 83.207 MiB, 2.38% gc time, 71.25% compilation time)

  and with this PR

  julia> ] precompile
  
  julia> @time using DynamicQuantities
  0.140393 seconds (461.62 k allocations: 29.999 MiB, 28.96% compilation time)

  On Julia 1.9, the effect is less noticeable (since the package is basically empty), on the master branch one gets

  julia> @time_imports using DynamicQuantities
    0.5 ms  Tricks
    0.3 ms  Compat
    0.4 ms  Compat → CompatLinearAlgebraExt
    0.4 ms  PackageExtensionCompat
   44.3 ms  DynamicQuantities
    0.4 ms  DynamicQuantities → DynamicQuantitiesLinearAlgebraExt

  whereas with this PR I obtain

  julia> @time_imports using DynamicQuantities
    0.4 ms  Tricks
    0.3 ms  Compat
    0.4 ms  Compat → CompatLinearAlgebraExt
   44.1 ms  DynamicQuantities
    0.4 ms  DynamicQuantities → DynamicQuantitiesLinearAlgebraExt

• Removes the TOML dependency and its using statement: It's only used for extracting the version number from Project.toml, which IMO is a bit questionable. This feature is not used in the package, and if desired hardcoding the version number and adding a test for its consistency with Project.toml would be much simpler and faster. Generally, I have the feeling though that such a functionality (obtaining the version of an installed package) should rather be provided by Pkg or a Pkg utility package (I would assume such a package already exists?).
• Removes the weak dependency and test dependency on ScientificTypesBase: It is re-exported by ScientificTypes, so it is safe to access it via ScientificTypes.ScientificTypesBase since it can't be removed from ScientificTypes in a non-breaking release. This simplifies the dependency structure and the Requires hooks.

[github-actions[bot]]

Benchmark Results

	main	964323feb8dacf...	t[main]/t[964323feb8dacf...]
Quantity/creation/Quantity(x)	3.4 ± 0 ns	3.4 ± 0 ns	1
Quantity/creation/Quantity(x, length=y)	3.7 ± 0 ns	3.7 ± 0 ns	1
Quantity/with_numbers/*real	3.4 ± 0 ns	3.4 ± 0 ns	1
Quantity/with_numbers/^int	11.5 ± 4.1 ns	11.5 ± 4.1 ns	1
Quantity/with_numbers/^int * real	11.8 ± 4.1 ns	12.1 ± 4 ns	0.975
Quantity/with_quantity/+y	6.8 ± 0.1 ns	6.8 ± 0.1 ns	1
Quantity/with_quantity//y	3.7 ± 0 ns	3.7 ± 0.001 ns	1
Quantity/with_self/dimension	1.7 ± 0 ns	1.7 ± 0 ns	1
Quantity/with_self/inv	3.7 ± 0.001 ns	3.4 ± 0 ns	1.09
Quantity/with_self/ustrip	1.7 ± 0 ns	1.7 ± 0 ns	1
QuantityArray/broadcasting/multi_array_of_quantities	0.225 ± 0.024 ms	0.225 ± 0.025 ms	1
QuantityArray/broadcasting/multi_normal_array	0.0764 ± 0.0022 ms	0.0761 ± 0.0022 ms	1
QuantityArray/broadcasting/multi_quantity_array	0.255 ± 0.0014 ms	0.254 ± 0.0013 ms	1
QuantityArray/broadcasting/x^2_array_of_quantities	0.0435 ± 0.0045 ms	0.0423 ± 0.0047 ms	1.03
QuantityArray/broadcasting/x^2_normal_array	9.4 ± 1.8 μs	9.4 ± 1.8 μs	1
QuantityArray/broadcasting/x^2_quantity_array	10 ± 1.4 μs	9.9 ± 1.4 μs	1.01
QuantityArray/broadcasting/x^4_array_of_quantities	0.135 ± 0.0059 ms	0.136 ± 0.0068 ms	0.994
QuantityArray/broadcasting/x^4_normal_array	0.0692 ± 0.0014 ms	0.0688 ± 0.0011 ms	1.01
QuantityArray/broadcasting/x^4_quantity_array	0.105 ± 0.0023 ms	0.102 ± 0.0023 ms	1.02
time_to_load	0.17 ± 0.0016 s	0.167 ± 0.00018 s	1.01

Benchmark Plots

A plot of the benchmark results have been uploaded as an artifact to the workflow run for this PR. Go to "Actions"->"Benchmark a pull request"->[the most recent run]->"Artifacts" (at the bottom).

[MilesCranmer]

This sounds great, thanks.

However, I would vote for leaving PkgExtensionCompat as-is, since it is zero cost on new versions of Julia.

Is it truly worth the maintenance burden of manually writing extensions all to get 0.2 s startup improvement on old Julia versions that few people use (which already have very slow startups)? I definitely want DynamicQuantities to be (1) correct and (2) fast on old Julia, but startup time seems like a second order effect not worth the effort.

[devmotion]

since it is zero cost on new versions of Julia.

On new Julia versions it has almost zero loading time but it's measurable nevertheless, and it requires an additional dependency with additional compilation that can be avoided.

In my opinion, arguably the officially documented way of writing optional package extensions with backwards compatibility does not involve any significant additional maintenance. In particular, once it is set up, maintenance should be almost equivalent both with and without PackageExtensionsCompat, the only difference being that without PackageExtensionsCompat one has to duplicate modifications to the using ... statements.

Generally, I think it's good to improve performance even on older Julia versions (particularly since the Julia LTS is still 1.6), especially if the necessary modifications are simple. I'd argue following the Pkg docs is sufficiently simple here, but of course not everyone will agree with this opinion 🙂

[MilesCranmer]

PackageExtensionCompat

Would you be okay to split out the PackageExtensionCompat removal into a separate PR as that change is a bit more contentious? We could discuss it there. My motivation for using it – which I do in all of my Julia packages – is that I have run into bugs in the past from the manual approach due to mixing up the correct sequence of import .., isdefined(Base, :get_extension), and the __init__.

I like PackageExtensionCompat because it manages all of this for you. For me it is more about about reducing the rate of human errors (for when I eventually add another extension); I think hurting startup time by 0.2% for a 1% improvement in reliability is a pretty good tradeoff, no?

TOML

TOML is in the Julia image so I don't follow why this needs to be removed as a dep? I don't think there is any startup cost. If you help me understand why I am open to it.

The PACKAGE_VERSION constant is a more robust way to set package versions rather than using Pkg, since it will get compiled into the module when even the Project.toml might not be available. I was recommended this strategy on discourse and have used it in libraries I contribute to ever since. It's zero cost and I find it's more reliable for testing/performance tracking.

ScientificTypesBase

Good point, happy to make this change!

SparseArrays

^ Also, were you thinking of including this change (to use a simple sparse array struct) in this PR or separately?

SparseArrays seems to be about 90% of the startup time on 1.10 so I definitely agree that that would be the biggest change, compared with these smaller tweaks!

[devmotion]

I'll open a new PR with the PackageExtensionsCompat changes. In the end it's of course your call since you're the maintainer of this package, but to me it's still unclear why there should be additional maintenance burden by following the official/vanilla approach, I don't see any challenges with it in particular once the first extension is implemented. I guess I'm more worried about relying on an additional non-official tool that causes loading time regressions and has a history of introducing bugs to the workflow.

I also checked Pkg quickly and indeed there is an official way to retrieve the version of a package that imported a module:

julia> import DynamicQuantities, Pkg

julia> Pkg.pkgversion(DynamicQuantities)
v"0.7.1"

help?> Pkg.pkgversion
  pkgversion(m::Module)

  Return the version of the package that imported module m, or nothing if m was not imported from a package, or imported from a package without a version field set.

  The version is read from the package's Project.toml during package load.

  To get the version of the package that imported the current module the form pkgversion(@__MODULE__) can be used.

  │ Julia 1.9
  │
  │  This function was introduced in Julia 1.9.

To me that seems simpler than defining non-standard constants (many packages don't define PACKAGE_VERSION, so you can't query the version of an arbitrary package based on it). Moreover, pkgversion even works with submodules:

julia> Pkg.pkgversion(DynamicQuantities.Units)
v"0.7.1"

[MilesCranmer]

Pkg.pkgversion

Cool! This must be a new addition to the stdlib.

Okay with this we can definitely remove the TOML business.

PackageExtensionCompat

I would still like to leave this in if that’s okay? The only bugs I have had related to this sort of thing have been from following the official/manual approach to get compatibility for earlier Julia versions and making mistakes. I just find the official syntax to be pretty unintuitive/tedious. PackageExtensionCompat is just so much simpler I don’t really want to go back...

[devmotion]

I reverted the PackageExtensionCompat change.

[MilesCranmer]

Thanks for the PR!