Backports for 1.12-rc2 #59337
Merged
Backports for 1.12-rc2 #59337
+432
−197
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Should this be rc2? |
|
Yes, (I think this is the third time I make this mistake now...) |
|
REPL tests seem to consistently hang on mac. |
|
Works fine locally though |
|
REPL tests passed on 18f42dc but fails on the first commit on this branch. So either the issue is with that commit or something else changed with the buildkite setup I guess. Edit: Tried rebasing on release-1.12. |
…m)` (#59343) The issue is that JET reported that in `libgfortran_version(p).major` the first arg of `getproperty` could be `nothing`. This is already checked in the previous line, but in a way that the compiler cannot remember until that call. Putting it into a variable should fix that. It would be great if this could get backported to at least 1.12 (1.10 and 1.11 would also be great), since that is where people try to use JET for their packages, and this reduces the Base noise in the output.
The fields of `StringIndexError` are abstractly typed, so there's no reason to specialize on a concrete type. The change seems like it could prevent some invalidation on loading user code. --------- Co-authored-by: Shuhei Kadowaki <[email protected]>
Fixes #59272. This code was originally introduced in e7efe42. The design of the binding partitions underwent several changes, so I'm not fully sure if it was correct at the time, but regardless, it was rendered incorrect by 60a9f69. In the new design, even a change to the value of a binding (not just its visibility) can affect the resolution outcome, so a full re-resolution is always required. Fix identified by Claude (in one of several rollouts). Correct fix among them identified by me. Actual change/test by me.
It's important for this measurement to include indirect compilation (not just "self" time), since the `--trace-compile` log only reports compilation triggers / entrypoints (not all compiled code). (cherry picked from commit 888c2c3)
[The "parsing" section of the "eval" dev docs page](https://docs.julialang.org/en/v1/devdocs/eval/#dev-parsing) was not updated when JuliaSyntax.jl was adopted in 1.10. This updates the text to reflect that it is the default parser and briefly mentions how to switch back. (cherry picked from commit ec27274)
(cherry picked from commit fc0017f)
) Fixes #59326. Change the logic that decides not to specialize a function parameter based on whether or not the supplied argument is a Function, and that function is not used, so that it will still work if the SpecType is a Union{Function,Nothing} or any other union that contains a Function. The logic is changed from a hardcoded rule of `type_i == Function || type_i == Any || type_i == Base.Callable` to `type_i >: Function`. This covers all of the above cases, but also includes custom `Union{Function, T}` such as `Union{Function, Nothing}`. --------- Co-authored-by: Nick Robinson <[email protected]> Co-authored-by: Jameson Nash <[email protected]> (cherry picked from commit 9612115)
(cherry picked from commit 096011c)
…59329) We already save some memory here by deleting the `jl_native_code_desc_t` after we're done serializing the combined module, but some data in the module's `LLVM::Context` lives on until the end of the scope in `jl_dump_native_impl`. Once we're done with the module, move the lock and `ThreadSafeContext` so the reference count drops to zero. A crude measurement shows that when compiling the Base sysimage, about 3 GiB is in use. Deleting the `jl_native_code_desc_t` (as before) saves about 600 MiB, and cleaning up the context saves an additional ~500 MiB. (cherry picked from commit ceeb661)
The `@__artifact_str` macro was updated in #55707. ```julia-repl # Before julia> @time import ZeroMQ_jll 0.119653 seconds (313.44 k allocations: 16.855 MiB, 79.48% compilation time) # After julia> @time import ZeroMQ_jll 0.024658 seconds (22.61 k allocations: 1.830 MiB) ``` (cherry picked from commit d28a587)
…59363) Use a separate inference_start and compilation_start, where compilation_start does not do anything special for reentrancy. Here is an example which has a quick-to-compile nested dynamic dispatch, but that nested function runs for a long time: ```julia ./julia --startup=no --trace-compile=stderr --trace-compile-timing -e ' Base.@assume_effects :foldable function nested1(x) sum(collect(x for _ in 1:1000_000_000)) end f1(x) = nested1(sizeof(x)) + x f1(2)' #= 12.7 ms =# precompile(Tuple{typeof(Main.nested1), Int64}) #= 3809.3 ms =# precompile(Tuple{typeof(Main.f1), Int64}) ``` This fixes a small issue introduced here: https://github.com/JuliaLang/julia/pull/59220/files#r2292021838 (cherry picked from commit be59100)
…59367) The only method of the `__require_prelocked` function in `Base` gets compiled with bad inference for two variables, `path` and `reasons`, which get pessimistically boxed (and inferred as `Any`), even though both of them are only assigned to once across both the method and the closure within (neither is assigned to within the closure). My guess for what causes this would be either of these, or the combination: * (mis)use of GOTO * `try`-`catch` The least demanding way to work around the issue is adding a `let` around the closure, as advised in the Performance tips. That is what this change does. This change should fix several bad inference results within method instances such as `__require_prelocked(::PkgId, ::Nothing)` and `__require_prelocked(::PkgId, ::String)`. Consequently, the sysimage should become less vulnerable to invalidation happening when loading certain packages. (cherry picked from commit 547f858)
…byte pointer (#59395) On 32 bit machines, for an atomic of size 9 to 11 bytes, the result fits in the 16 byte pool, but only with a maximum write of 12 bytes (there is 1 byte reserved for the `success` plus 4 for the type tag, leaving 11 bytes for the data). This was accidentally doing a 16 byte write instead, which could smash the type tag field (usually will NULL) of the next object. Not sure how to test, since just noticed this while reading the code. (cherry picked from commit bbd491a)
(cherry picked from commit a776445)
(cherry picked from commit 6a78395)
KristofferC
force-pushed
the
backports-release-1.12
branch
from
43460a1 to
3a9511e
Compare
|
Seems like it hangs after doesnt say that much though |
|
Now the REPL test did not hang all of a sudden. Is it flaky... Maybe has nothing to do with any commit in here? |
…58238) `_aligned_msize` is now declared in mingw64's crt (mingw-w64/mingw-w64@b40e24a) as `_CRTIMP size_t __cdecl _aligned_msize(void *_Memory,size_t _Alignment,size_t _Offset);`. Renamed our version to prevent the conflict (cherry picked from commit 2270fcd)
Released 3 June 2025 https://github.com/JuliaMath/openlibm/releases/tag/v0.8.7 (cherry picked from commit 61d04b3)
KristofferC
force-pushed
the
backports-release-1.12
branch
2 times, most recently
from
25e4158 to
e437a5a
Compare
This is an alternative to #59161, attempting to fix the same observed CI behavior. I don't think #59161 is the best way to fix it, as the point of these tests is to make sure that REPL completions looks up the PATH internally. Calling the path update function explicitly defeats that somewhat. The extra synchronization here to make this deterministic is messy, but I do think it makes the test closer to real-world usage. The core attempted fix here is to move the read of the PATH_ locals inside `maybe_spawn_cache_PATH` into the locked region. If they are not under the lock, they could be unconditionally overwritten by a second call to this function, causing issues in the state machine. I do not know whether this is the cause of the observed CI hangs, but it's worth fixing anyway. (cherry picked from commit 9a16119)
KristofferC
force-pushed
the
backports-release-1.12
branch
from
063477d to
50e72b3
Compare
|
Omg yes, green CI 😭 |
|
Yay! Been looking forward to this one, thanks for pushing this over the line. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backported PRs:
@nospecializeforstring_index_err#57604Need manual backport:
Contains multiple commits, manual intervention needed:
Non-merged PRs with backport label:
Union{Nothing,Function}params #59327haskey(::@Kwargs{item::Bool}, :item)constant-fold #59320@allocatedagain #59278macroexpand!function and addlegacyscopekwarg #59276--trace-compile#58535transcode: prevent Windows sysimage invalidation #58038maxthreadidfromThreads#57490