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

Use Base parallel precompilation to build stdlibs #53598

Merged
merged 18 commits into from
Mar 8, 2024

Conversation

IanButterworth
Copy link
Member

@IanButterworth IanButterworth commented Mar 5, 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.

Todo

  • Figure out how to avoid printing JULIA all-release when re-build is a noop
julia % make
    JULIA all-release
julia % 

@IanButterworth IanButterworth added building Build system, or building Julia or its dependencies packages Package management and loading labels Mar 5, 2024
@IanButterworth IanButterworth marked this pull request as ready for review March 5, 2024 04:08
Copy link
Member

@vchuravy vchuravy left a comment

Choose a reason for hiding this comment

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

This is fantastic! Unable to comment on the changes to precompilation.jl, but the rest looks great.

Need to adjust some of Makefile handling to ensure that we only run the step when needed and not all the time when calling make.

I wonder if we could parse the project.toml to have a source of truth for all the stdlibs instead of having a manual list (and so that the system complains if someone just updates the Project.toml but not the INDEPENDENT_STDLIBS

pkgimage.mk Show resolved Hide resolved
pkgimage.mk Outdated Show resolved Hide resolved
@@ -0,0 +1,61 @@
[deps]
Copy link
Member

Choose a reason for hiding this comment

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

I think this is absolutely the right step. Thoughts for the future:

Could we get away with the current special way of managing versions in stdlib and instead use compat bounds? But that might require Julia to build Julia something that we have so far avoided. But we could totally use Julia 1.11 to build Julia 1.12

Should we have compat bounds here?

pkgimage.mk Outdated Show resolved Hide resolved
pkgimage.mk Show resolved Hide resolved
base/precompilation.jl Outdated Show resolved Hide resolved
pkgimage.mk Outdated Show resolved Hide resolved
@vchuravy vchuravy requested a review from vtjnash March 7, 2024 19:41
@vchuravy
Copy link
Member

vchuravy commented Mar 7, 2024

@vtjnash could you take a quick look at the Makefile changes?

@IanButterworth IanButterworth added the backport 1.11 Change should be backported to release-1.11 label Mar 8, 2024
@IanButterworth
Copy link
Member Author

Apart from the makefile review, this should be ready to merge

@vchuravy vchuravy merged commit 78351b5 into JuliaLang:master Mar 8, 2024
6 of 8 checks passed
@IanButterworth IanButterworth deleted the ib/stdlib_parallel branch March 8, 2024 15:34
@JeffBezanson
Copy link
Member

This is really nice, thanks!

Keno added a commit that referenced this pull request Mar 9, 2024
The environment variables here need to have target semantics, so
on Windows the path separator needs to be `;` and the path needs
to be converted to windows before being passed through julia.
Because variable assignment is not ordinarily shell expanded,
we also need an extra `$(shell)` to perform this conversion.
Fixes MSYS2 builds of Julia after #53598.
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 <vchuravy@users.noreply.github.com>
(cherry picked from commit 78351b5)
@KristofferC KristofferC mentioned this pull request Mar 15, 2024
60 tasks
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
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 <vchuravy@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
building Build system, or building Julia or its dependencies packages Package management and loading
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants