JuliaLang / julia

The Julia Programming Language
https://julialang.org/
MIT License
45.72k stars 5.48k forks source link

136x regression for LazyArtifacts #55725

Open PallHaraldsson opened 1 month ago

PallHaraldsson commented 1 month ago

[Backport to 1.11 if the fix is simple?]

Unlike (though Artifacts also with slight 3.8x regression, though less pressing):

julia> @time using Artifacts
  0.015384 seconds (625 allocations: 40.070 KiB)

julia> @time using LazyArtifacts
  0.975359 seconds (554.76 k allocations: 34.849 MiB, 10.52% gc time, 0.69% compilation time)

I'm not sure I think the Lazy variant is supposed to be an optimization, but it depends on Pkg and is thus slow to start, slowing down e.g. micromamba_jll (and I suppose more JLLs), and thus PythonCall...

I don't know enough about it, i.e.:

https://github.com/JuliaLang/LazyArtifacts.jl/blob/master/src/LazyArtifacts.jl

Should packages just no be using it?

Basically because of:

julia> @time_imports using LazyArtifacts
..
    233.9 ms  NetworkOptions 98.77% compilation time
..
    394.3 ms  Pkg

Why "compilation time" there? I know how to fix usually but not the latency in neither case there...

[@cjdoris, I tried just disabling LazyArtifacts (NOT substituting with Artefacts, would that be good?) in micromamba_jll and it seemed to work, predictably loading it got much faster, avoiding the above overhead, though I likely just broke it? I will not make a PR there since it's an auto-generated package, and I'm not sure where and how to make a correct change.

julia> @time using micromamba_jll
  0.103872 seconds (35.29 k allocations: 2.323 MiB, 5.57% compilation time)

At https://github.com/JuliaPackaging/BinaryBuilder.jl ?]

IanButterworth commented 1 month ago

One thing we could do is move the check installed part of Pkg.ensure_artifact_installed into LazyArtifacts and lazily load Pkg if the artifact does need installing https://github.com/JuliaLang/LazyArtifacts.jl/blob/871726e3142c8bc8d1a55a70b98607a5a2c94b7e/src/LazyArtifacts.jl#L13

@staticfloat

vtjnash commented 1 month ago

You have to be a bit careful with that, since it would mean the lazy load artifacts cannot be used during precompile (including of dependent packages). OTOH, it is not exactly lazy if the user demands they get downloaded to precompile, but a downstream package may not want to care about that detail. The appropriate way around that is for LazyArtifacts to never load Pkg, but rather to make a subprocess that does (c.f. the refresh_artifacts.jl in the Artifacts tests, which has exactly that concern)

IanButterworth commented 1 month ago

Something like https://github.com/JuliaPackaging/LazyArtifacts.jl/pull/1 ?

IanButterworth commented 1 month ago

55740 alone shouldn't close this. We need https://github.com/JuliaPackaging/LazyArtifacts.jl/pull/1 and https://github.com/JuliaLang/julia/pull/55739