Skip to content

Commit

Permalink
cache the find_all_in_cache_path call during parallel precompilation (
Browse files Browse the repository at this point in the history 
#56369)

Before (in an environment with DifferentialEquations.jl):

```julia
julia> @time Pkg.precompile()
  0.733576 seconds (3.44 M allocations: 283.676 MiB, 6.24% gc time)

julia> isfile_calls[1:10]
10-element Vector{Pair{String, Int64}}:
        "/home/kc/.julia/juliaup/julia-nightly/share/julia/compiled/v1.12/Printf/3FQLY_zHycD.ji" => 178
        "/home/kc/.julia/juliaup/julia-nightly/share/julia/compiled/v1.12/Printf/3FQLY_xxrt3.ji" => 178
         "/home/kc/.julia/juliaup/julia-nightly/share/julia/compiled/v1.12/Dates/p8See_xxrt3.ji" => 158
         "/home/kc/.julia/juliaup/julia-nightly/share/julia/compiled/v1.12/Dates/p8See_zHycD.ji" => 158
          "/home/kc/.julia/juliaup/julia-nightly/share/julia/compiled/v1.12/TOML/mjrwE_zHycD.ji" => 155
          "/home/kc/.julia/juliaup/julia-nightly/share/julia/compiled/v1.12/TOML/mjrwE_xxrt3.ji" => 155
                                     "/home/kc/.julia/compiled/v1.12/Preferences/pWSk8_4Qv86.ji" => 152
                                     "/home/kc/.julia/compiled/v1.12/Preferences/pWSk8_juhqb.ji" => 152
 "/home/kc/.julia/juliaup/julia-nightly/share/julia/compiled/v1.12/StyledStrings/UcVoM_zHycD.ji" => 144
 "/home/kc/.julia/juliaup/julia-nightly/share/julia/compiled/v1.12/StyledStrings/UcVoM_xxrt3.ji" => 144
 ```  


After:

```julia
julia> @time Pkg.precompile()
  0.460077 seconds (877.59 k allocations: 108.075 MiB, 4.77% gc time)

julia> isfile_calls[1:10]
  10-element Vector{Pair{String, Int64}}:
"/tmp/jl_a5xFWK/Project.toml" => 15
"/tmp/jl_a5xFWK/Manifest.toml" => 7
"/home/kc/.julia/registries/General.toml" => 6

"/home/kc/.julia/juliaup/julia-nightly/share/julia/stdlib/v1.12/Markdown/src/Markdown.jl"
=> 3

"/home/kc/.julia/juliaup/julia-nightly/share/julia/stdlib/v1.12/Serialization/src/Serialization.jl"
=> 3

"/home/kc/.julia/juliaup/julia-nightly/share/julia/stdlib/v1.12/Distributed/src/Distributed.jl"
=> 3

"/home/kc/.julia/juliaup/julia-nightly/share/julia/stdlib/v1.12/UUIDs/src/UUIDs.jl"
=> 3

"/home/kc/.julia/juliaup/julia-nightly/share/julia/stdlib/v1.12/LibCURL/src/LibCURL.jl"
=> 3
```

Performance is improved and we are not calling `isfile` on a bunch of the same ji files hundreds times.

Benchmark is made on a linux machine so performance diff should be a lot better on Windows where these `isfile_casesensitive` call is much more expensive.

Fixes #56366

---------

Co-authored-by: KristofferC <[email protected]>
Co-authored-by: Ian Butterworth <[email protected]>
  • Loading branch information
@KristofferC @IanButterworth
3 people authored Oct 29, 2024
1 parent e4dc9d3 commit 9850a38
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 6 deletions.
10 changes: 6 additions & 4 deletions base/loading.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1815,7 +1815,8 @@ const StaleCacheKey = Tuple{PkgId, UInt128, String, String}
function compilecache_path(pkg::PkgId;
ignore_loaded::Bool=false,
stale_cache::Dict{StaleCacheKey,Bool}=Dict{StaleCacheKey, Bool}(),
cachepaths::Vector{String}=Base.find_all_in_cache_path(pkg),
cachepath_cache::Dict{PkgId, Vector{String}}=Dict{PkgId, Vector{String}}(),
cachepaths::Vector{String}=get!(() -> find_all_in_cache_path(pkg), cachepath_cache, pkg),
sourcepath::Union{String,Nothing}=Base.locate_package(pkg),
flags::CacheFlags=CacheFlags())
path = nothing
Expand All @@ -1830,7 +1831,7 @@ function compilecache_path(pkg::PkgId;
for dep in staledeps
dep isa Module && continue
modpath, modkey, modbuild_id = dep::Tuple{String, PkgId, UInt128}
modpaths = find_all_in_cache_path(modkey)
modpaths = get!(() -> find_all_in_cache_path(modkey), cachepath_cache, modkey)
for modpath_to_try in modpaths::Vector{String}
stale_cache_key = (modkey, modbuild_id, modpath, modpath_to_try)::StaleCacheKey
if get!(() -> stale_cachefile(stale_cache_key...; ignore_loaded, requested_flags=flags) === true,
Expand Down Expand Up @@ -1872,10 +1873,11 @@ fresh julia session specify `ignore_loaded=true`.
function isprecompiled(pkg::PkgId;
ignore_loaded::Bool=false,
stale_cache::Dict{StaleCacheKey,Bool}=Dict{StaleCacheKey, Bool}(),
cachepaths::Vector{String}=Base.find_all_in_cache_path(pkg),
cachepath_cache::Dict{PkgId, Vector{String}}=Dict{PkgId, Vector{String}}(),
cachepaths::Vector{String}=get!(() -> find_all_in_cache_path(pkg), cachepath_cache, pkg),
sourcepath::Union{String,Nothing}=Base.locate_package(pkg),
flags::CacheFlags=CacheFlags())
path = compilecache_path(pkg; ignore_loaded, stale_cache, cachepaths, sourcepath, flags)
path = compilecache_path(pkg; ignore_loaded, stale_cache, cachepath_cache, cachepaths, sourcepath, flags)
return !isnothing(path)
end

Expand Down
6 changes: 4 additions & 2 deletions base/precompilation.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -415,6 +415,8 @@ function _precompilepkgs(pkgs::Vector{String},
color_string(cstr::String, col::Union{Int64, Symbol}) = _color_string(cstr, col, hascolor)

stale_cache = Dict{StaleCacheKey, Bool}()
cachepath_cache = Dict{PkgId, Vector{String}}()

exts = Dict{PkgId, String}() # ext -> parent
# make a flat map of each dep and its direct deps
depsmap = Dict{PkgId, Vector{PkgId}}()
Expand Down Expand Up @@ -785,7 +787,7 @@ function _precompilepkgs(pkgs::Vector{String},
## precompilation loop

for (pkg, deps) in depsmap
cachepaths = Base.find_all_in_cache_path(pkg)
cachepaths = get!(() -> Base.find_all_in_cache_path(pkg), cachepath_cache, pkg)
sourcepath = Base.locate_package(pkg)
single_requested_pkg = length(requested_pkgs) == 1 && only(requested_pkgs) == pkg.name
for config in configs
Expand All @@ -808,7 +810,7 @@ function _precompilepkgs(pkgs::Vector{String},
wait(was_processed[(dep,config)])
end
circular = pkg in circular_deps
is_stale = !Base.isprecompiled(pkg; ignore_loaded=true, stale_cache, cachepaths, sourcepath, flags=cacheflags)
is_stale = !Base.isprecompiled(pkg; ignore_loaded=true, stale_cache, cachepath_cache, cachepaths, sourcepath, flags=cacheflags)
if !circular && is_stale
Base.acquire(parallel_limiter)
is_direct_dep = pkg in direct_deps
Expand Down

10 comments on commit 9850a38

@maleadt
Copy link
Member

Choose a reason for hiding this comment

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

Testing PkgEval:

@nanosoldier runtests(["JSON", "Crayons"])

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

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

The package evaluation job you requested has completed - no issues were detected.
The full report is available.

@maleadt
Copy link
Member

Choose a reason for hiding this comment

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

@nanosoldier runtests(["Sandbox", "BinaryBuilderBase"])

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

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

The package evaluation job you requested has completed - no issues were detected.
The full report is available.

@maleadt
Copy link
Member

Choose a reason for hiding this comment

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

@nanosoldier runtests(["Sandbox", "BinaryBuilderBase", "BinaryBuilder"])

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

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

The package evaluation job you requested has completed - possible issues were detected.
The full report is available.

@vtjnash
Copy link
Member

Choose a reason for hiding this comment

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

@nanosoldier runbenchmarks(ALL, isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

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

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here.

@vtjnash
Copy link
Member

@vtjnash vtjnash commented on 9850a38 Nov 1, 2024

Choose a reason for hiding this comment

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

Rather large, and unexpected, regressions on many inference benchmarks here, but unclear what of those allocations might actually be noise
https://tealquaternion.camdvr.org/compare.html?start=31f7df648f750897e245d169639cb1264ebc7404&end=9850a3881221a57a382e98c9b9ae2bf97ac3966d&stat=min-wall-time&name=inference&nonRelevant=true

@Zentrik
Copy link
Member

@Zentrik Zentrik commented on 9850a38 Nov 2, 2024

Choose a reason for hiding this comment

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

Bisected [["inference", "allinference", "Base.init_stdio(::Ptr{Cvoid})"]] regression to 9dbdeb4. I imagine this is just due to more code being compiled.

Please sign in to comment.