Skip to content

Commit

Permalink
Use Base.require() to load Distributed internally (#54571)
Browse files Browse the repository at this point in the history
TL;DR: we changed to loading stdlibs exclusively from the default sysimg
in JuliaLang/julia#53326, and this breaks
developing Distributed.jl because the workers will load the builtin
version instead of the development version. I think this should be
backported to 1.11?

CC @jpsamaroo

---

`Base.require_stdlib()` will exclusively load a stdlib from whatever was
shipped with Julia. The problem is that when developing Distributed.jl
this will cause the workers to always load the builtin Distributed
module instead of the development version, which can break everything
because now the master and worker are running different (potentially
incompatible) versions of Distributed.

This commit changes Base and LinearAlgebra to use `Base.require()`
instead, which will respect `Base.LOAD_PATH`. I argue that this is safe
because unlike the other stdlibs like REPL or Pkg, Distributed is only
loaded if explicitly requested by the user with `-p` or through
`addprocs()` or something, so it shouldn't be possible to get into quite
the same
tools-are-broken-because-I-broke-my-tools situation that motivated using
`Base.require_stdlib()` in the first place.

An alternative design would be to:
1. Move the if-block loading Distributed in `exec_options()` below the
for-loop where it will execute `-e` options.
2. Require any implementation of `Distributed.launch(::ClusterManager)`
to pass `-e 'using Distributed'` in their command to ensure that
Distributed is loaded in a way respecting `Base.LOAD_PATH`.

This would be more consistent with how the other stdlibs must be
developed, but it requires implementers (i.e. Distributed and
ClusterManagers.jl) to opt-in to allowing development versions of
Distributed, which feels very annoying and easy to miss so I decided not
to implement that.

(cherry picked from commit 3b922b0a129e89b5fed0045f20f51f3c16a75079)
(cherry picked from commit af49d9f1c2eb917628ca47d29f5ffbef79463101)
  • Loading branch information
JamesWrigley authored and KristofferC committed Dec 2, 2024
1 parent 2ee110e commit 9104915
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion src/LinearAlgebra.jl
Original file line number Diff line number Diff line change
Expand Up @@ -717,7 +717,7 @@ function peakflops(n::Integer=4096; eltype::DataType=Float64, ntrials::Integer=3
end

if parallel
let Distributed = Base.require_stdlib(Base.PkgId(
let Distributed = Base.require(Base.PkgId(
Base.UUID((0x8ba89e20_285c_5b6f, 0x9357_94700520ee1b)), "Distributed"))
nworkers = @invokelatest Distributed.nworkers()
results = @invokelatest Distributed.pmap(peakflops, fill(n, nworkers))
Expand Down

0 comments on commit 9104915

Please sign in to comment.