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

Backports for 1.11.0-beta2 #54112

Merged
merged 75 commits into from
May 28, 2024
Merged

Backports for 1.11.0-beta2 #54112

merged 75 commits into from
May 28, 2024

Conversation

KristofferC
Copy link
Member

@KristofferC KristofferC commented Apr 17, 2024

Backported PRs:

Non-merged PRs with backport label:

jishnub and others added 7 commits April 17, 2024 10:43
…reters (#54069)

Partially reverts #49391

PrecompileTools uses the timing infrastructure to snoop on the inference
process.
The reason for #49391 was that this could lead to accidental pollution
of the caches
with foreign results
(timholy/SnoopCompile.jl#338)

After #52233 and especially #53336 we now filter results by cache owner
and
don't try to cache foreign code using the native pipeline.

Motivated by JuliaGPU/GPUCompiler.jl#567 which
demonstrated
that a foreign code instance would not be cached without
PrecompileTools.

(cherry picked from commit c0611e8)
Particularly under precompilation, fields and globals can revert to
being undef, which is heavily relied upon by many `__init__` methods in
the ecosystem (because JLL emits this pattern, so we cannot simply
disallow it).

While here, also fix and improve several places where we compute or use
the isdefined check poorly.

(cherry picked from commit fb3ae01)
Followup to #53833
Fixes a failure seen in #53974
(below)

I believe this is the more correct check to make?

The heapsnapshot generated from this PR is viewable in vscode.

```
2024-04-06 09:33:58 EDT	      From worker 7:	ERROR: Base.InvalidCharError{Char}('\xc1\xae')
2024-04-06 09:33:58 EDT	      From worker 7:	Stacktrace:
2024-04-06 09:33:58 EDT	      From worker 7:	  [1] throw_invalid_char(c::Char)
2024-04-06 09:33:58 EDT	      From worker 7:	    @ Base ./char.jl:86
2024-04-06 09:33:58 EDT	      From worker 7:	  [2] UInt32
2024-04-06 09:33:58 EDT	      From worker 7:	    @ ./char.jl:133 [inlined]
2024-04-06 09:33:58 EDT	      From worker 7:	  [3] category_code
2024-04-06 09:33:58 EDT	      From worker 7:	    @ ./strings/unicode.jl:339 [inlined]
2024-04-06 09:33:58 EDT	      From worker 7:	  [4] isassigned
2024-04-06 09:33:58 EDT	      From worker 7:	    @ ./strings/unicode.jl:355 [inlined]
2024-04-06 09:33:58 EDT	      From worker 7:	  [5] isassigned
2024-04-06 09:33:58 EDT	      From worker 7:	    @ /cache/build/tester-amdci5-14/julialang/julia-master/julia-41d026beaf/share/julia/stdlib/v1.12/Unicode/src/Unicode.jl:138 [inlined]
2024-04-06 09:33:58 EDT	      From worker 7:	  [6] print_str_escape_json(stream::IOStream, s::String)
2024-04-06 09:33:58 EDT	      From worker 7:	    @ Profile.HeapSnapshot /cache/build/tester-amdci5-14/julialang/julia-master/julia-41d026beaf/share/julia/stdlib/v1.12/Profile/src/heapsnapshot_reassemble.jl:239
2024-04-06 09:33:59 EDT	      From worker 7:	  [7] (::Profile.HeapSnapshot.var"#5#6"{IOStream})(strings_io::IOStream)
2024-04-06 09:33:59 EDT	      From worker 7:	    @ Profile.HeapSnapshot /cache/build/tester-amdci5-14/julialang/julia-master/julia-41d026beaf/share/julia/stdlib/v1.12/Profile/src/heapsnapshot_reassemble.jl:192
```

(cherry picked from commit c557636)
Fix #54100 by computing the
stride and offset explicitly. This is unlikely to be a performance
concern, so we don't need to hard-code this.

(cherry picked from commit 159f4d7)
@KristofferC KristofferC added the release Release management and versioning. label Apr 17, 2024
@KristofferC
Copy link
Member Author

@nanosoldier runtests(vs =":release-1.10")

@nanosoldier
Copy link
Collaborator

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

@PallHaraldsson
Copy link
Contributor

PallHaraldsson commented Apr 18, 2024

@tecosaur, I'm confused, annotate! is new, but public, not exported so why:

Failed to precompile InMemoryDatasets [5c01b14b-ab03-46ff-b164-14c663efdd9f] to "/home/pkgeval/.julia/compiled/v1.11/InMemoryDatasets/jl_TqjVub".
ERROR: LoadError: UndefVarError: annotate! not defined in InMemoryDatasets
Suggestion: check for spelling errors or missing imports.
Hint: a global variable of this name also exists in Base.

I'm concerned how many packages are failing [EDIT: it seems because PkgEval doesn't test latest version such as in this case (I've now reported that at #54150): , and why, e.g. QML, because of CxxWrap seemingly. It's bad the CxxWrap gets broken repeatedly by Julia. I'm not exactly sure why (each time), I just understand it happens... And we now have a brand-new competitor to it, not sure if it's immune to such. Can we declare the C API (at least) CxxWrap depends on stable, or at least not keep breaking it?]

vtjnash and others added 2 commits April 18, 2024 10:40
It is easy to accidentally call these functions (they are used by vcat,
which is syntax) with very long lists of values, causing inference to
crash and take a long time. The `afoldl` function can handle that very
well however, while naive recursion did not.

Fixes #53585

(cherry picked from commit df28bf7)
…54130)

`abstract_eval_statement` may return `LimitedAccuracy` so we need to
handle it before applying `widenconst`.

- fixes #54125
@tecosaur
Copy link
Contributor

@PallHaraldsson I don't see what

ERROR: LoadError: UndefVarError: annotate! not defined in InMemoryDatasets

has to do with Base.annotate!

@KristofferC
Copy link
Member Author

Regarding annotate! in InMemoryDataSets.jl, probably fixed in sl-solution/InMemoryDatasets.jl#117. Not sure what that code is really doing though.

@PallHaraldsson, please try to keep on topic for this PR, if you want to discuss stability of the C-API, this is not the place.

vchuravy and others added 14 commits April 19, 2024 11:13
For GPUCompiler we would like to support a native on disk cache of LLVM
IR.
One of the longstanding issues has been the cache invalidation of such
an on disk cache.

With #52233 we now have an integrated cache for the inference results
and we can rely
on `CodeInstance` to be stable across sessions. Due to #52119 we can
also rely on the
`objectid` to be stable.

My inital thought was to key the native disk cache in GPUCompiler on the
objectid of
the corresponding CodeInstance (+ some compilation parameters).

While discussing this with @rayegun yesterday we noted that having a
CodeInstance with
the same objectid might not be enough provenance. E.g we are not
gurantueed that the
CodeInstance is from the same build artifact and the same precise source
code.

For the package images we are tracking this during loading and validate
all contents
at once, and we keep explicitly track of the provenance chain.

This PR adds a lookup up table where we map from "external_blobs" e.g.
loaded images,
to the corresponding top module of each image, and uses this to
determine the
build_id of the package image.

(cherry picked from commit d47cbf6)
… to bfdb4c3 (#54127)

Stdlib: StyledStrings
URL: https://github.com/JuliaLang/StyledStrings.jl.git
Stdlib branch: main
Julia branch: backports-release-1.11
Old commit: e0ca0f8
New commit: bfdb4c3
Julia version: 1.11.0-beta1
StyledStrings version: 1.11.0
Bump invoked by: @tecosaur
Powered by:
[BumpStdlibs.jl](https://github.com/JuliaLang/BumpStdlibs.jl)

Diff:
JuliaLang/StyledStrings.jl@e0ca0f8...bfdb4c3

```
$ git log --oneline e0ca0f8..bfdb4c3
bfdb4c3 Modify tests to also pass when run as stdlib
c084718 Fix return type of AnnotatedString write
180ab6c Try fixing the non-stdlib tests via refactor
528f245 Docs: minor index copyedits, and americanizations
9c015e2 Docs: create an examples page
a9772d4 Markup annot keys cannot contain null character
243d959 Allow interpolation of underline properties
fd2adcc Docs: tweak face description for clarity
4b06b79 Docs: clarify that AbstractString is wrapped
7f07b1b Docs: second paragraph reads better as not a note
a3d15cb Docs: forgot to mention font attribute
9c10614 Show colour and styling in docs
59fd944 Add docs previews to PR CI
9709612 Mark styled and withfaces functions as public API
a4c7678 Make withfaces behave more consistently
50d4198 Add speculative consideration of future face keys
04b5031 Add fuzzing to the tests set
7dc3c26 Allow color strings as in Face constructor
c419317 Don't annotate interpolated empty strings
dfef96d Adjust prioritisation of composed styling in parse
9a23e9e Test the display of parsing errors
1d7f42a Test parsing of each and every inline face attr
84ba211 No need to escape a string when parsing
e3c0453 Add missing is-macro check to face value interp
db006ed Mistyped font attribute as "face" in the parser
230fa8e Test errors emitted with various malformed stystrs
31f4c1d Overly aggressive color names check in styled strs
bec9216 Expand on faces tests
093385e Improve showing of RGB SimpleColors without MIME
d60d545 Test the show methods of SimpleColor and Face
cb05225 Test the AnnotatedIOBuffer printstyled method
c36911c Test the (legacy) loading of colors from envvars
14b4c6e Reduce test code verbosity by importing more names
3db948f Add a battery of HTML encoding tests
316bdd5 Remove extranious spaces from HTML underline style
62a7d25 Adjust named HTML colours to be not-garish
81e031e Add a battery of ANSI encoding tests
a14c3b1 Check the Smulx termcap instead of Su
b9d4aea Use the redmean colour distance in 8-bit approx
f9976ad More careful comma handling with inline face specs
24e10e4 Accept a style symbol as the sole underline value
2ba234a Use the hardcoded bold ANSI code
ab4f681 Refactro termcolor8bit to be less magic
a8b8aaf Fix off-by-one errors in termcolor8bit
21e127a Introduce fuzzer for styled markup
a3b40b7 Mention the loading of faces.toml in the docs
16c0e4f Fix functional parsing of inline face weight
7da631f Consolidate use of "ws" and "whitespace" in EBNF
b76c1ce Introduce ismacro convenience func to parser
b1cb60c Fix handling of space around inline face attrs
e22d058 Clarification in styled markup grammar docs
701d29f Introduce isnextchar convenience func to parser
6efb352 Fix edge-case parsing of empty face construct
10f6839 Implement new functional styled markup interpreter
e2d2d5f Refactor stylemacro into a submodule
11c5bd9 Introduce specialised write for AnnotatedIOBuffer
```

Co-authored-by: Dilum Aluthge <dilum@aluthge.com>
This is an alternative to #53642

The `dom_edges()` for an exit block in the CFG are empty when computing
the PostDomTree so the loop below this may not actually run. In that
case, the right semidominator is the ancestor from the DFSTree, which is
the "virtual" -1 block.

This resolves half of the issue in
#53613:
```julia
julia> let code = Any[
               # block 1
               GotoIfNot(Argument(2), 3),
               # block 2
               ReturnNode(Argument(3)),
               # block 3 (we should visit this block)
               Expr(:call, throw, "potential throw"),
               ReturnNode(), # unreachable
           ]
           ir = make_ircode(code; slottypes=Any[Any,Bool,Bool])
           visited = BitSet()
           @test !Core.Compiler.visit_conditional_successors(CC.LazyPostDomtree(ir), ir, #=bb=#1) do succ::Int
               push!(visited, succ)
               return false
           end
           @test 2 ∈ visited
           @test 3 ∈ visited
       end
Test Passed
```

This needs some tests (esp. since I don't think we have any DomTree
tests at all right now), but otherwise should be good to go.
)

This commit fixes the first problem that was found while digging into
#53613. It turns out that the post-domtree constructed
from regular `IRCode` doesn't work for visiting conditional successors
for post-opt analysis in cases like:
```julia
julia> let code = Any[
               # block 1
               GotoIfNot(Argument(2), 3),
               # block 2
               ReturnNode(Argument(3)),
               # block 3 (we should visit this block)
               Expr(:call, throw, "potential throw"),
               ReturnNode(), # unreachable
           ]
           ir = make_ircode(code; slottypes=Any[Any,Bool,Bool])
           visited = BitSet()
           @test !Core.Compiler.visit_conditional_successors(CC.LazyPostDomtree(ir), ir, #=bb=#1) do succ::Int
               push!(visited, succ)
               return false
           end
           @test 2 ∉ visited
           @test 3 ∈ visited
       end
Test Failed at REPL[14]:16
  Expression: 2 ∉ visited
   Evaluated: 2 ∉ BitSet([2])
```

This might mean that we need to fix on the `postdominates` end, but for
now, this commit tries to get around it by using the augmented post
domtree in `visit_conditional_successors`. Since the augmented post
domtree is enforced to have a single return, we can keep using the
current `postdominates` to fix the issue.

However, this commit isn't enough to fix the NeuralNetworkReachability
segfault as reported in #53613, and we need to tackle the second issue
reported there too
(#53613 (comment)).
…54219)

When an inconsistent statement doesn’t affect the return value, post-opt
analysis will try to refine it to `:consistent`. However this refinement
is invalid if the statement could throw, as `:consistent` requires
consistent termination.

For the time being, this commit implements the most conservative fix.
There might be a need to analyze `:nothrow` in a data-flow sensitive way
in the post-opt analysis as like we do for `:consistent`.

- closes #53613
Fixes #53908  by clamping before doing addition.

This also fixes an issue with negative skips if `io.offset` isn't zero.

I am assuming that `io.size+1` cannot overflow.

(cherry picked from commit 306124c)
Previously, any case changes to Annotated{String,Char} types triggered
"fall back to non-annotated type" non-specialised methods. It would be
nice to keep the annotations though, and that can be done so long as we
keep track of any potential changes to the number of bytes taken by each
character on case changes. This is unusual, but can happen with some
letters (e.g. the upper case of 'ſ' is 'S').

To handle this, a helper function annotated_chartransform is introduced.
This allows for efficient uppercase/lowercase methods (about 50%
overhead in managing the annotation ranges, compared to just
transforming a String). The {upper,lower}casefirst and titlecase
transformations are much more inefficient with this style of
implementation, but not prohibitively so. If somebody has a bright idea,
or they emerge as an area deserving of more attention, the performance
characteristics can be improved.

As a bonus, a specialised textwidth method is implemented to avoid the
generic fallback, providing a ~12x performance improvement.

To check that annotated_chartransform is accurate, as are the
specialised case-transformations, a few million random collections of
strings were pre- and post-annotated and checked to be the same in a
fuzzing check performed with Supposition.jl.

    const short_str = Data.Text(Data.Characters(), max_len=20)
    const short_strs = Data.Vectors(short_str, max_size=10)
    const case_transform_fn = Data.SampledFrom((uppercase, lowercase))

    function annot_caseinvariant(f::Function, strs::Vector{String})
        annot_strs =
            map(((i, s),) -> AnnotatedString(s, [(1:ncodeunits(s), :i => i)]),
                enumerate(strs))
        f_annot_strs =
            map(((i, s),) -> AnnotatedString(s, [(1:ncodeunits(s), :i => i)]),
                enumerate(map(f, strs)))
        pre_join = Base.annotated_chartransform(join(annot_strs), f)
        post_join = join(f_annot_strs)
        pre_join == post_join
    end

    @check max_examples=1_000_000 annot_caseinvariant(case_transform_fn, short_strs)

This helped me determine that in annotated_chartransform the "- 1" was
needed with offset position calculation, and that in the "findlast"
calls that less than *or equal* was the correct equality test.

(cherry picked from commit 38a9725)
The AnnotatedString(::AnnotatedChar) constructor actually does not
exist. Considering that String(::Char) is not defined, and we don't try
this anywhere else, the obvious fix is to just construct the appropriate
AnnotatedString here.

We can think about more properly Char-optimised writes in the future if
it comes up.

(cherry picked from commit 42b3134)
PR from #54097.

Fixes #54097

Co-authored-by: Nathan Zimmerberg
(cherry picked from commit fda02ac)
* `IdSet{T}` has values of type `T`, not `V`
* Test the example
* Properly format the example

(cherry picked from commit a84ed82)
This reverts part of 67b8ac0
(#47596 (comment)).
That change broke `make install` from tarballs due to building docs
again, which fails as there's no git repo (and also requires Internet
access to download UnicodeData.txt.

Fixes #54037.

(cherry picked from commit d6dda7c)
…k adj/trans (#54151)

Fixes the following issue on master, where the zero element is computed
incorrectly (but subsequent terms are computed correctly):
```julia
julia> using LinearAlgebra

julia> x = [1 2 3; 4 5 6];

julia> A = reshape([x,2x,3x,4x],2,2);

julia> b = fill(x, 2);

julia> A' * b
ERROR: DimensionMismatch: matrix A has axes (Base.OneTo(2),Base.OneTo(3)), matrix B has axes (Base.OneTo(2),Base.OneTo(3))
Stacktrace:
  [1] _generic_matmatmul!(C::Matrix{Int64}, A::Matrix{Int64}, B::Matrix{Int64}, _add::LinearAlgebra.MulAddMul{true, true, Bool, Bool})
    @ LinearAlgebra ~/.julia/juliaup/julia-nightly/share/julia/stdlib/v1.12/LinearAlgebra/src/matmul.jl:849
  [2] generic_matmatmul!
    @ ~/.julia/juliaup/julia-nightly/share/julia/stdlib/v1.12/LinearAlgebra/src/matmul.jl:834 [inlined]
  [3] _mul!
    @ ~/.julia/juliaup/julia-nightly/share/julia/stdlib/v1.12/LinearAlgebra/src/matmul.jl:287 [inlined]
  [4] mul!
    @ ~/.julia/juliaup/julia-nightly/share/julia/stdlib/v1.12/LinearAlgebra/src/matmul.jl:285 [inlined]
  [5] mul!
    @ ~/.julia/juliaup/julia-nightly/share/julia/stdlib/v1.12/LinearAlgebra/src/matmul.jl:253 [inlined]
  [6] *
    @ ~/.julia/juliaup/julia-nightly/share/julia/stdlib/v1.12/LinearAlgebra/src/matmul.jl:114 [inlined]
  [7] _generic_matvecmul!(C::Vector{Matrix{…}}, tA::Char, A::Matrix{Matrix{…}}, B::Vector{Matrix{…}}, _add::LinearAlgebra.MulAddMul{true, true, Bool, Bool})
    @ LinearAlgebra ~/.julia/juliaup/julia-nightly/share/julia/stdlib/v1.12/LinearAlgebra/src/matmul.jl:797
  [8] generic_matvecmul!
    @ ~/.julia/juliaup/julia-nightly/share/julia/stdlib/v1.12/LinearAlgebra/src/matmul.jl:755 [inlined]
  [9] _mul!
    @ ~/.julia/juliaup/julia-nightly/share/julia/stdlib/v1.12/LinearAlgebra/src/matmul.jl:73 [inlined]
 [10] mul!
    @ ~/.julia/juliaup/julia-nightly/share/julia/stdlib/v1.12/LinearAlgebra/src/matmul.jl:70 [inlined]
 [11] mul!
    @ ~/.julia/juliaup/julia-nightly/share/julia/stdlib/v1.12/LinearAlgebra/src/matmul.jl:253 [inlined]
 [12] *(A::Adjoint{Adjoint{Int64, Matrix{Int64}}, Matrix{Matrix{Int64}}}, x::Vector{Matrix{Int64}})
    @ LinearAlgebra ~/.julia/juliaup/julia-nightly/share/julia/stdlib/v1.12/LinearAlgebra/src/matmul.jl:60
 [13] top-level scope
    @ REPL[10]:1
Some type information was truncated. Use `show(err)` to see complete types.
```
After this PR,
```julia
julia> A' * b
2-element Vector{Matrix{Int64}}:
 [51 66 81; 66 87 108; 81 108 135]
 [119 154 189; 154 203 252; 189 252 315]
```

(cherry picked from commit 2b878f0)
(cherry picked from commit e644ebd)
LilithHafner and others added 12 commits May 23, 2024 16:56
- fix #52355 using option 4 (round to nearest representable integer)
- update docstrings *including documenting convert to Inf behavior even
though Inf is not the "closest" floating point value*
- add some assorted tests

---------

Co-authored-by: mikmoore <95002244+mikmoore@users.noreply.github.com>
(cherry picked from commit e7a1def)
Co-authored-by: Lilith Orion Hafner <lilithhafner@gmail.com>
(cherry picked from commit c28a9de)
Memo to self:

* update version number in `stdlib/OpenBLAS_jll/Project.toml`
* update version number and sha in `deps/openblas.version`
* refresh checksums with `make -f contrib/refresh_checksums.mk -j
openblas`

See the [release notes of v0.3.27](https://github.com/OpenMathLib/OpenBLAS/releases/tag/v0.3.27).

(cherry picked from commit 8423426)
A common idiom used throughout the codebase is to get a pointer to
thread-local-state through `jl_current_task->ptls`.

Create a phantom task for GC threads so that we can make use of this
idiom when running in the GC threads as well.

Idea originally suggested by @vchuravy, bugs are mine.

(cherry picked from commit 9636ef7)
I checked and updated the three related files to make sure command-line
documentations are the same in all of them.

Previously mentioned in
#52645 (comment)

---------

Co-authored-by: Matt Bauman <mbauman@gmail.com>
(cherry picked from commit a931fbe)
Fixes #51493, fixes
#52075

(cherry picked from commit 8742d3c)
…ar. (#53675)

Noticed when working on 02f27c2. The
substitution and re-sorting of inner vars are incomplete on master. This
commit re-organized the code by:
1. Flatten the inner vars into a reversed list and handling them just
like vars in norm bindings.
2. Then perform a global re-sorting on all vars.
3. After that, the inner vars get frozen and dependent bounds are
refreshed.

(cherry picked from commit 3d34f11)
…ll` (#54465)

This commit adds a nothrow path for type instantiation, which eliminates
the bad `Union` elements from the result rather than returns the bottom
type.
close #54404

(cherry picked from commit a946631)
…ation (#54514)

Adopt suggestions from
#54465 (review)
and fix various added regession & residual MWE.

(cherry picked from commit af545b9)
@KristofferC KristofferC force-pushed the backports-release-1.11 branch from d67cc7d to 5b15ed7 Compare May 23, 2024 14:57
@JamesWrigley
Copy link
Contributor

If it's merged in time, could #54571 be backported too? It's pretty important for developing Distributed on 1.11.

KristofferC and others added 3 commits May 27, 2024 09:58
fix #52141, fix #52986

(cherry picked from commit 6f569c7)
TL;DR: we changed to loading stdlibs exclusively from the default sysimg
in #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 3b922b0)
@KristofferC
Copy link
Member Author

@nanosoldier runtests()

KristofferC and others added 6 commits May 27, 2024 15:00
… package precompilation (#53972)

In the parallel package precompilation code we are mapping what packages
depend on what other packages so that we precompile things in the
correct order ("bottom up") and so what we can also detect cycles and
avoid precompiling packages in those cycles. However, we fail to detect
one type of dependency which is that an extension can "indirectly"
depend on another extension. This happens when the transitive
dependencies of the extension (it's parent + triggers) are a superset of
the dependencies of another extension. In other words, this means that
the other extension will get loaded into the first extension once it
gets loaded, effectively being a dependency.

The failure to model this leads to some issues, for example using one of
the examples in our own tests:

```julia
julia> Base.active_project()
"/home/kc/julia/test/project/Extensions/HasDepWithExtensions.jl/Project.toml"

(HasDepWithExtensions) pkg> status --extensions
Project HasDepWithExtensions v0.1.0
Status `~/julia/test/project/Extensions/HasDepWithExtensions.jl/Project.toml`
  [4d3288b3] HasExtensions v0.1.0 `../HasExtensions.jl`
              ├─ ExtensionFolder [ExtDep, ExtDep2]
              ├─ Extension [ExtDep]
              └─ LinearAlgebraExt [LinearAlgebra]

julia> Base.Precompilation.precompilepkgs(; timing=true)
Precompiling all packages...
    196.1 ms  ✓ HasExtensions
    244.4 ms  ✓ ExtDep2
    207.9 ms  ✓ SomePackage
    201.6 ms  ✓ ExtDep
    462.5 ms  ✓ HasExtensions → ExtensionFolder
    200.1 ms  ✓ HasExtensions → Extension
    173.1 ms  ✓ HasExtensions → LinearAlgebraExt
    222.2 ms  ✓ HasDepWithExtensions
  8 dependencies successfully precompiled in 2 seconds

julia> Base.Precompilation.precompilepkgs(; timing=true)
Precompiling all packages...
    213.4 ms  ✓ HasExtensions → ExtensionFolder
  1 dependency successfully precompiled in 0 seconds. 7 already precompiled.

julia> Base.Precompilation.precompilepkgs(; timing=true)

julia>
```

We can see here that `ExtensionFolder` gets precompiled twice which is
due to `Extension` actually being an "indirect dependency" of
`ExtensionFolder` and therefore should be precompiled before it.

With this PR we instead get:

```julia
julia> Precompilation.precompilepkgs(; timing=true)
Precompiling all packages...
    347.5 ms  ✓ ExtDep2
    294.0 ms  ✓ SomePackage
    293.2 ms  ✓ HasExtensions
    216.5 ms  ✓ HasExtensions → LinearAlgebraExt
    554.9 ms  ✓ ExtDep
    580.9 ms  ✓ HasExtensions → Extension
    593.8 ms  ✓ HasExtensions → ExtensionFolder
    261.3 ms  ✓ HasDepWithExtensions
  8 dependencies successfully precompiled in 2 seconds

julia> Precompilation.precompilepkgs(; timing=true)

julia>
```

`Extension` is precompiled after `ExtensionFolder` and nothing happens
on the second call.

Also, with this PR we get for the issue in
#53081 (comment):

```julia
(jl_zuuRGt) pkg> st
Status `/private/var/folders/tp/2p4x9ygx48sgsdl1ccg1mp_40000gn/T/jl_zuuRGt/Project.toml`
⌃ [d236fae5] PreallocationTools v0.4.17
⌃ [0c5d862f] Symbolics v5.16.1
Info Packages marked with ⌃ have new versions available and may be upgradable.

julia> Precompilation.precompilepkgs(; timing=true)
┌ Warning: Circular dependency detected. Precompilation will be skipped for:
│   SymbolicsPreallocationToolsExt
│   SymbolicsForwardDiffExt
```

and we avoid precompiling the problematic extensions.

This should also allow extensions to precompile in parallel which I
think was prevented before (from the code that is removed in the
beginning of the diff).

(cherry picked from commit df89440)
Fixes #54439.

- Lock around concurrent accesses to .logs, .message_limits, and
   .shouldlog_args.
- Copy the vector out of the logger at the end, to shield against
   dangling Tasks.

Before:
```julia
julia> Threads.nthreads()
8

julia> function foo(n)
           @info "Doing foo with n=$n"
           @sync for i=1:n
               Threads.@Spawn @info "Iteration $i"
           end
           42
       end
foo (generic function with 1 method)

julia> for _ in 1:1000
           @test_logs (:info,"Doing foo with n=10000") match_mode=:any foo(10_000)
       end
julia+RAI(56155,0x1f5157ac0) malloc: double free for ptr 0x128248000
julia+RAI(56155,0x1f5157ac0) malloc: *** set a breakpoint in malloc_error_break to debug

[56155] signal (6): Abort trap: 6
in expression starting at REPL[8]:1

signal (6) thread (1) __pthread_kill at /usr/lib/system/libsystem_kernel.dylib (unknown line)
Allocations: 54370881 (Pool: 54363911; Big: 6970); GC: 119
[1]    56154 abort      julia -tauto
```
After:
```julia
julia> Threads.nthreads()
8

julia> function foo(n)
           @info "Doing foo with n=$n"
           @sync for i=1:n
               Threads.@Spawn @info "Iteration $i"
           end
           42
       end
foo (generic function with 1 method)

julia> for _ in 1:1000
           @test_logs (:info,"Doing foo with n=10000") match_mode=:any foo(10_000)
       end
```
(no crash) :)

(cherry picked from commit 0437210)
Same as #54496 for some more
methods.
I added some more testcases than the changed methods. For
`SimpleVector`, I don't know how to construct an instance for a
testcase.

This could be backported to 1.11.

(cherry picked from commit 7273b9a)
Might as well avoid the effort on following Pkg repl switches.

It doesn't make a noticeable difference on my machine, but who knows on
slower systems.

(cherry picked from commit 5a5624c)
(cherry picked from commit 0c65f6f)
@nanosoldier
Copy link
Collaborator

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

barucden and others added 2 commits May 28, 2024 10:06
Fixes #54022

Co-authored-by: Klaus Crusius <KlausC@users.noreply.github.com>
(cherry picked from commit a721658)
@KristofferC KristofferC merged commit f7295bf into release-1.11 May 28, 2024
13 checks passed
@KristofferC KristofferC deleted the backports-release-1.11 branch May 28, 2024 12:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release Release management and versioning.
Projects
None yet
Development

Successfully merging this pull request may close these issues.