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

Move parallel precompilation to Base #53403

Merged
merged 6 commits into from
Mar 5, 2024
Merged

Conversation

KristofferC
Copy link
Member

@KristofferC KristofferC commented Feb 20, 2024

Parallel precompilation is more or less now required in order to use somewhat large packages unless you want to wait an obscene long time for it to complete. Right now, we even start a parallel precompilation on a package load if we notice that the package you are loading is not precompiled.

This functionally has typically been implemented in Pkg but with Pkg not being in the sysimage it becomes a bit awkward because we then need to load Pkg from Base. The only real reason this functionality has been implemented in Pkg is that Pkg has some useful features for parsing environments. Moving precompilation to Base has typically been stalled on such an environment parser not existing in Base.

However, in #46690 I started implemented code loading on top of a more up front environment parser (instead of the "incremental" one that currently exists in loading.jl) and we can retro fit this to be used as the basis of parallel precompilation. At some later point code loading could be implemented on top of it but that is for now considered future work.

This PR thus adds the environment parser from the codeloading PR and implementes the parallel precompilation feature from Pkg on top of it (instead of on top of the EnvCache in Pkg).

Some points to bring up here:

  • This copy pastes the progress bar implementation in Pkg into here. It is probably a bit excessive to use so we can simplify that significantly.
  • Parallel precompilation uses the FileWatching module to avoid different processes trying to precompile the same package concurrently. Right now, I used grab this from Base.loaded_modules relying on it being in the sysimage.
  • This removes the "suspended" functionality from the Pkg precompilation which does not try to precompile packages if they have "recently" failed which is unclear how useful it is in practice. This also requires the Serialization stdlib and uses data structures defined in Pkg so it is hard to keep when moving this to Base.

@KristofferC KristofferC added packages Package management and loading backport 1.11 Change should be backported to release-1.11 labels Feb 20, 2024
@KristofferC KristofferC force-pushed the kc/base_precompile branch 2 times, most recently from 1666dc0 to 58e3580 Compare February 20, 2024 17:34
@KristofferC
Copy link
Member Author

LoadError: SystemError: opening file "/tmp/jl_5IpsjQLoxE": Operation not permitted
      From worker 2:	Stacktrace:
      From worker 2:	  [1] systemerror(p::String, errno::Int32; extrainfo::Nothing)
      From worker 2:	    @ Base ./error.jl:176
      From worker 2:	  [2] systemerror
      From worker 2:	    @ ./error.jl:175 [inlined]
      From worker 2:	  [3] open(fname::String; lock::Bool, read::Nothing, write::Bool, create::Nothing, truncate::Nothing, append::Nothing)
      From worker 2:	    @ Base ./iostream.jl:295
      From worker 2:	  [4] open
      From worker 2:	    @ ./iostream.jl:277 [inlined]
      From worker 2:	  [5] open_nolock
      From worker 2:	    @ ~/.julia/scratchspaces/a66863c6-20e8-4ff4-8a62-49f30b1f605e/agent-cache/default-honeycrisp-HL2F7YQ3XH.0/build/default-honeycrisp-HL2F7YQ3XH-0/julialang/julia-master/julia-95a886400b/share/julia/stdlib/v1.12/ArgTools/src/ArgTools.jl:35 [inlined]
      From worker 2:	  [6] arg_write(f::Downloads.var"#3#4"{Nothing, Vector{Pair{String, String}}, Float64, Nothing, Bool, Nothing, Downloads.Downloader, String}, arg::Nothing)
      From worker 2:	    @ ArgTools ~/.julia/scratchspaces/a66863c6-20e8-4ff4-8a62-49f30b1f605e/agent-cache/default-honeycrisp-HL2F7YQ3XH.0/build/default-honeycrisp-HL2F7YQ3XH-0/julialang/julia-master/julia-95a886400b/share/julia/stdlib/v1.12/ArgTools/src/ArgTools.jl:119
      From worker 2:	  [7] #download#2
      From worker 2:	    @ ~/.julia/scratchspaces/a66863c6-20e8-4ff4-8a62-49f30b1f605e/agent-cache/default-honeycrisp-HL2F7YQ3XH.0/build/default-honeycrisp-HL2F7YQ3XH-0/julialang/julia-master/julia-95a886400b/share/julia/stdlib/v1.12/Downloads/src/Downloads.jl:257 [inlined]

https://github.com/JuliaLang/Downloads.jl/blob/2dd891ab237ad6a58b11337027325fb571f43cb7/src/Downloads.jl#L469-L474 is buggy and will not be relocatable since @__FILE__ is expanded at compile time.

@KristofferC KristofferC mentioned this pull request Feb 26, 2024
28 tasks
base/precompile.jl Outdated Show resolved Hide resolved
base/precompile.jl Outdated Show resolved Hide resolved
@KristofferC KristofferC force-pushed the kc/base_precompile branch 2 times, most recently from 866cc1c to bf196e4 Compare February 28, 2024 20:35
KristofferC added a commit that referenced this pull request Mar 1, 2024
Backported PRs:
- [x] #53361 <!-- 🤖 [master] Bump the SparseArrays stdlib from c9f7293
to cb602d7 -->
- [x] #53300 <!-- allow external absint to hold custom data in
`codeinst.inferred` -->
- [x] #53342 <!-- Add `Base.wrap` to docs -->
- [x] #53372 <!-- Silence warnings in `test/file.jl` -->
- [x] #53357 <!-- 🤖 [master] Bump the Pkg stdlib from 6dd0e7c9e to
76070d295 -->
- [x] #53373 <!-- fix sysimage-native-code=no option with pkgimages -->
- [x] #53333 <!-- More consistent return value for annotations, take 2
-->
- [x] #53354 <!-- fix code coverage bug in tail position and `else` -->
- [x] #53407 <!-- fix sysimage-native-code=yes option -->
- [x] #53388 <!-- Fix documentation: thread pool of main thread -->
- [x] #53355 <!-- Fix synchronization issues on the GC scheduler. -->
- [x] #53429 <!-- Subtype: skip slow-path in `local_∀_∃_subtype` if
inputs contain no ∃ typevar. -->
- [x] #53437 <!-- Add debug variant of loader_trampolines.o -->
- [x] #53284 <!-- Add annotate! method for AnnotatedIOBuffer -->
- [x] #53466 <!-- [MozillaCACerts_jll] Update to v2023-12-12 -->
- [x] #53467 <!-- [LibGit2_jll] Update to v1.7.2 -->
- [x] #53326 <!-- RFC: when loading code for internal purposes, load
stdlib files directly, bypassing DEPOT_PATH, LOAD_PATH, and stale checks
-->
- [x] #53332
- [x] #53320 <!-- Add `Sys.isreadable, Sys.iswriteable`, update `ispath`
-->
- [x] #53476

Contains multiple commits, manual intervention needed:
- [ ] #53285 <!-- Add update mechanism for Terminfo, and common
user-alias data -->

Non-merged PRs with backport label:
- [ ] #53424 <!-- yet more atomics & cache-line fixes on work-stealing
queue -->
- [ ] #53408 <!-- task splitting: change additive accumulation to
multiplicative -->
- [ ] #53403 <!-- Move parallel precompilation to Base -->
- [ ] #53402 <!-- Add `jl_getaffinity` and `jl_setaffinity` -->
- [ ] #53391 <!-- Default to the medium code model in x86 linux -->
- [ ] #53125 <!-- coverage: count coverage where explicitly requested by
inference only -->
- [ ] #52694 <!-- Reinstate similar for AbstractQ for backward
compatibility -->
@KristofferC KristofferC mentioned this pull request Mar 1, 2024
60 tasks
@KristofferC KristofferC force-pushed the kc/base_precompile branch 2 times, most recently from 894439e to f316bf8 Compare March 1, 2024 15:46
base/loading.jl Outdated Show resolved Hide resolved
@Keno Keno added this to the 1.11 milestone Mar 3, 2024
Copy link
Member

@IanButterworth IanButterworth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've renamed the module PrecompilePkgs->Precompilation and file to precompilation.jl to match the form of loading.jl.

I've also made non-interactive printing always show with timing. It is probably helpful to do-so for CI.
Note that loading-invoked precompilation in a noninteractive session is still silent, like it is for serial precompile in julia currently.

Thank you @KristofferC for making this happen.

@IanButterworth IanButterworth added the merge me PR is reviewed. Merge when all tests are passing label Mar 4, 2024
@IanButterworth IanButterworth merged commit 6745160 into master Mar 5, 2024
8 checks passed
@IanButterworth IanButterworth deleted the kc/base_precompile branch March 5, 2024 01:07
@IanButterworth IanButterworth removed the merge me PR is reviewed. Merge when all tests are passing label Mar 5, 2024
KristofferC added a commit that referenced this pull request Mar 6, 2024
Parallel precompilation is more or less now required in order to use
somewhat large packages unless you want to wait an obscene long time for
it to complete. Right now, we even start a parallel precompilation on a
package load if we notice that the package you are loading is not
precompiled.

This functionally has typically been implemented in Pkg but with Pkg not
being in the sysimage it becomes a bit awkward because we then need to
load Pkg from Base. The only real reason this functionality has been
implemented in Pkg is that Pkg has some useful features for parsing
environments. Moving precompilation to Base has typically been stalled
on such an environment parser not existing in Base.

However, in #46690 I started
implemented code loading on top of a more up front environment parser
(instead of the "incremental" one that currently exists in `loading.jl`)
and we can retro fit this to be used as the basis of parallel
precompilation. At some later point code loading could be implemented on
top of it but that is for now considered future work.

This PR thus adds the environment parser from the codeloading PR and
implementes the parallel precompilation feature from Pkg on top of it
(instead of on top of the `EnvCache` in Pkg).

Some points to bring up here:

- This copy pastes the progress bar implementation in Pkg into here. It
is probably a bit excessive to use so we can simplify that
significantly.
- Parallel precompilation uses the `FileWatching` module to avoid
different processes trying to precompile the same package concurrently.
Right now, I used grab this from `Base.loaded_modules` relying on it
being in the sysimage.
- This removes the "suspended" functionality from the Pkg precompilation
which does not try to precompile packages if they have "recently" failed
which is unclear how useful it is in practice. This also requires the
Serialization stdlib and uses data structures defined in Pkg so it is
hard to keep when moving this to Base.

---------

Co-authored-by: Ian Butterworth <[email protected]>
(cherry picked from commit 6745160)
vchuravy added a commit that referenced this pull request Mar 8, 2024
Follow-on from #53403

This extends `Base.Precompilation.precompilepkgs` to take a list of
configurations to precompile each package with, while parallelizing
across all packages and configurations, and uses it to build the stdlib
pkgimages.

It simplifies the stdlib pkgimage build process but is (currently)
dependent on having an accurately resolved Manifest.toml (Project.toml
included to make the manifest easier to make). Any new/removed stdlibs
or changes their dependencies will require updating the Manifest.toml.
It's a bit chicken and egg, but should be manageable with manual editing
of the Manifest.toml.

In terms of speed improvement:
MacOS aarch64 CI runner 6m19s before, 5m19 with this

Note that CI builds will show the basic print with timing of each
package, whereas local build will be the tidier fancy print without
timings.

Co-authored-by: Valentin Churavy <[email protected]>
KristofferC pushed a commit that referenced this pull request Mar 15, 2024
Follow-on from #53403

This extends `Base.Precompilation.precompilepkgs` to take a list of
configurations to precompile each package with, while parallelizing
across all packages and configurations, and uses it to build the stdlib
pkgimages.

It simplifies the stdlib pkgimage build process but is (currently)
dependent on having an accurately resolved Manifest.toml (Project.toml
included to make the manifest easier to make). Any new/removed stdlibs
or changes their dependencies will require updating the Manifest.toml.
It's a bit chicken and egg, but should be manageable with manual editing
of the Manifest.toml.

In terms of speed improvement:
MacOS aarch64 CI runner 6m19s before, 5m19 with this

Note that CI builds will show the basic print with timing of each
package, whereas local build will be the tidier fancy print without
timings.

Co-authored-by: Valentin Churavy <[email protected]>
(cherry picked from commit 78351b5)
KristofferC added a commit that referenced this pull request Mar 17, 2024
Backported PRs:
- [x] #39071 <!-- Add a lazy `logrange` function and `LogRange` type -->
- [x] #51802 <!-- Allow AnnotatedStrings in log messages -->
- [x] #53369 <!-- Orthogonalize re-indexing for FastSubArrays -->
- [x] #48050 <!-- improve `--heap-size-hint` arg handling -->
- [x] #53482 <!-- add IR encoding for EnterNode -->
- [x] #53499 <!-- Avoid compiler warning about redefining jl_globalref_t
-->
- [x] #53507 <!-- update staled `Core.Compiler.Effects` documentation
-->
- [x] #53408 <!-- task splitting: change additive accumulation to
multiplicative -->
- [x] #53523 <!-- add back an alias for `check_top_bit` -->
- [x] #53377 <!-- add _readdirx for returning more object info gathered
during dir scan -->
- [x] #53525 <!-- fix InteractiveUtils call in Base.runtests on failure
-->
- [x] #53540 <!-- use more efficient `_readdirx` for tab completion -->
- [x] #53545 <!-- use `_readdirx` for `walkdir` -->
- [x] #53551 <!-- revert "Add @create_log_macro for making custom styled
logging macros (#52196)" -->
- [x] #53554 <!-- Always return a value in 1-d circshift! of
abstractarray.jl -->
- [x] #53424 <!-- yet more atomics & cache-line fixes on work-stealing
queue -->
- [x] #53571 <!-- Update Documenter to v1.3 for inventory writing -->
- [x] #53403 <!-- Move parallel precompilation to Base -->
- [x] #53589 <!-- add back `unsafe_convert` to pointer for arrays -->
- [x] #53596 <!-- build: remove extra .a file -->
- [x] #53606 <!-- fix error path in `precompilepkgs` -->
- [x] #53004 <!-- Unexport with, at_with, and ScopedValue from Base -->
- [x] #53629 <!-- typo fix in scoped values docs -->
- [x] #53630 <!-- sroa: Fix incorrect scope counting -->
- [x] #53598 <!-- Use Base parallel precompilation to build stdlibs -->
- [x] #53649 <!-- precompilepkgs: package in boths deps and weakdeps are
in fact only weak -->
- [x] #53671 <!-- Fix bootstrap Base precompile in cross compile
configuration -->
- [x] #52125 <!-- Load Pkg if not already to reinstate missing package
add prompt -->
- [x] #53602 <!-- Handle zero on arrays of unions of number types and
missings -->
- [x] #53516 <!-- permit NamedTuple{<:Any, Union{}} to be created -->
- [x] #53643 <!-- Bump CSL to 1.1.1 to fix libgomp bug -->
- [x] #53679 <!-- move precompile workload back from Base -->
- [x] #53663 <!-- add isassigned methods for reinterpretarray -->
- [x] #53662 <!-- [REPL] fix incorrectly cleared line after completions
accepted -->
- [x] #53611 <!-- Linalg: matprod_dest for Diagonal and adjvec -->
- [x] #53659 <!-- fix #52025, re-allow all implicit pointer casts in
cconvert for Array -->
- [x] #53631 <!-- LAPACK: validate input parameters to throw informative
errors -->
- [x] #53628 <!-- Make some improvements to the Scoped Values
documentation. -->
- [x] #53655 <!-- Change tbaa of ptr_phi to tbaa_value  -->
- [x] #53391 <!-- Default to the medium code model in x86 linux -->
- [x] #53699 <!-- Move `isexecutable, isreadable, iswritable` to
`filesystem.jl` -->
- [x] #41232 <!-- Fix linear indexing for ReshapedArray if the parent
has offset axes -->
- [x] #53527 <!-- Enable analyzegc checks for try catch and fix found
issues -->
- [x] #52092 
- [x] #53682 <!-- Increase build precompilation -->
- [x] #53720 
- [x] #53553 <!-- typeintersect: fix `UnionAll` unaliasing bug caused by
innervars. -->

Contains multiple commits, manual intervention needed:
- [ ] #53305 <!-- Propagate inbounds in isassigned with CartesianIndex
indices -->

Non-merged PRs with backport label:
- [ ] #53736 <!-- fix literal-pow to return the right type when the base
is -1 -->
- [ ] #53707 <!-- Make ScopedValue public -->
- [ ] #53696 <!-- add invokelatest to on_done callback in bracketed
paste -->
- [ ] #53660 <!-- put Logging back in default sysimage -->
- [ ] #53509 <!-- revert moving "creating packages" from Pkg.jl -->
- [ ] #53452 <!-- RFC: allow Tuple{Union{}}, returning Union{} -->
- [ ] #53402 <!-- Add `jl_getaffinity` and `jl_setaffinity` -->
- [ ] #52694 <!-- Reinstate similar for AbstractQ for backward
compatibility -->
- [ ] #51928 <!-- Styled markdown, with a few tweaks -->
- [ ] #51816 <!-- User-themable stacktraces -->
- [ ] #51811 <!-- Make banner size depend on terminal size -->
- [ ] #51479 <!-- prevent code loading from lookin in the versioned
environment when building Julia -->
@KristofferC KristofferC removed the backport 1.11 Change should be backported to release-1.11 label Mar 18, 2024
mkitti pushed a commit to mkitti/julia that referenced this pull request Apr 13, 2024
Parallel precompilation is more or less now required in order to use
somewhat large packages unless you want to wait an obscene long time for
it to complete. Right now, we even start a parallel precompilation on a
package load if we notice that the package you are loading is not
precompiled.

This functionally has typically been implemented in Pkg but with Pkg not
being in the sysimage it becomes a bit awkward because we then need to
load Pkg from Base. The only real reason this functionality has been
implemented in Pkg is that Pkg has some useful features for parsing
environments. Moving precompilation to Base has typically been stalled
on such an environment parser not existing in Base.

However, in JuliaLang#46690 I started
implemented code loading on top of a more up front environment parser
(instead of the "incremental" one that currently exists in `loading.jl`)
and we can retro fit this to be used as the basis of parallel
precompilation. At some later point code loading could be implemented on
top of it but that is for now considered future work.

This PR thus adds the environment parser from the codeloading PR and
implementes the parallel precompilation feature from Pkg on top of it
(instead of on top of the `EnvCache` in Pkg).

Some points to bring up here:

- This copy pastes the progress bar implementation in Pkg into here. It
is probably a bit excessive to use so we can simplify that
significantly.
- Parallel precompilation uses the `FileWatching` module to avoid
different processes trying to precompile the same package concurrently.
Right now, I used grab this from `Base.loaded_modules` relying on it
being in the sysimage.
- This removes the "suspended" functionality from the Pkg precompilation
which does not try to precompile packages if they have "recently" failed
which is unclear how useful it is in practice. This also requires the
Serialization stdlib and uses data structures defined in Pkg so it is
hard to keep when moving this to Base.

---------

Co-authored-by: Ian Butterworth <[email protected]>
mkitti pushed a commit to mkitti/julia that referenced this pull request Apr 13, 2024
Follow-on from JuliaLang#53403

This extends `Base.Precompilation.precompilepkgs` to take a list of
configurations to precompile each package with, while parallelizing
across all packages and configurations, and uses it to build the stdlib
pkgimages.

It simplifies the stdlib pkgimage build process but is (currently)
dependent on having an accurately resolved Manifest.toml (Project.toml
included to make the manifest easier to make). Any new/removed stdlibs
or changes their dependencies will require updating the Manifest.toml.
It's a bit chicken and egg, but should be manageable with manual editing
of the Manifest.toml.

In terms of speed improvement:
MacOS aarch64 CI runner 6m19s before, 5m19 with this

Note that CI builds will show the basic print with timing of each
package, whereas local build will be the tidier fancy print without
timings.

Co-authored-by: Valentin Churavy <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
packages Package management and loading
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants