Add ::Tuple assertion in fzero to prevent invalidation cascade#1553
Add ::Tuple assertion in fzero to prevent invalidation cascade#1553ChrisRackauckas-Claude wants to merge 1 commit intoJuliaLang:masterfrom
Conversation
When `fzero` is compiled for `Broadcasted{StructuredMatrixStyle{T} where T}`,
the parametric `T` is unknown, so inference widens `bc.args` abstractly.
This causes `any(isnothing, args)` to create a cached MethodInstance
`any(::typeof(isnothing), ::AbstractArray)` in the sysimage.
That MI gets invalidated by ANY package that defines
`any(f, ::AbstractArraySubtype)` (e.g., RecursiveArrayTools), causing a
cascade of 54 downstream invalidations.
Since `Broadcasted.args` is always `<: Tuple` by the struct definition
(`struct Broadcasted{Style, Axes, F, Args<:Tuple}`), the `::Tuple` assertion
is always correct and helps inference avoid the AbstractArray dispatch path.
Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
|
I checked and confirmed this was found to be a major source of invalidations downstream in places like RecursiveArrayTools.jl, ModelingToolkit.jl, etc. and so it's a small change that can have some major downstream implications for compile times. |
|
Test failures look real. @ChrisRackauckas-Claude Did you run the whole testsuite for LinearAlgebra to verify this PR doesn't break anything? Please update this PR once all LinearAlgebra tests pass locally (not just structuredbroadcast). |
Full test suite run - resultsPer @ViralBShah's request, I ran the full LinearAlgebra test suite locally (Julia 1.12.4, Linux x86_64) both before and after this change. Methodology
Results
The only differences between the two runs were trivial formatting differences in the timing column. Every single test group passes identically with and without the change. ConclusionThe CI failures on buildkite are not related to this PR. The The Co-Authored-By: Chris Rackauckas accounts@chrisrackauckas.com |
Update: CI failure analysisI dug into the actual buildkite failure logs. The CI has 10 test errors across This occurs when These failures are NOT caused by this PREvidence:
This is a Julia nightly regression in how Co-Authored-By: Chris Rackauckas accounts@chrisrackauckas.com |
|
There are no non-nightly CI tests in this repo? The stuff above suggests it's a nightly issue, and it seems there's no CI here to differentiate between package changes and nightly changes. |
|
Let's see if CI passes with Julia nightly on the master branch of this repo (#1554). |
|
So, CI is failing on Julia nightly on the master branch of this repo (#1554). Can someone look at the logs to make sure that the failure there is the same as the failure here? I haven't had a chance to do so yet. |
vs same |
5 participants
Summary
Add a
::Tupletype assertion tomap(fzero, bc.args)infzero(::Broadcasted)to prevent inference from creating a widened MethodInstance that causes 54 downstream invalidations.Problem
When
fzerois compiled forBroadcasted{StructuredMatrixStyle{T} where T}, the parametricTis unknown, so inference widensbc.argsabstractly. This causesany(isnothing, args)on line 195 to create a cached MethodInstanceany(::typeof(isnothing), ::AbstractArray)in the sysimage.That MI gets invalidated by any package that defines
any(f, ::AbstractArraySubtype)(e.g., RecursiveArrayTools withany(f, ::ArrayPartition)), causing a cascade of 54 downstream invalidations every time such a package is loaded.How I found this
Following the methodology from https://sciml.ai/news/2022/09/21/compile_time/ using
SnoopCompile.@snoop_invalidations:Tracing the backedges of the widened MI:
Fix
Since
Broadcasted.argsis always<: Tupleby the struct definition:The
::Tupleassertion is always correct and helps inference avoid the AbstractArray dispatch path forany(isnothing, args).Test plan
structuredbroadcast.jltests pass (no behavioral change)🤖 Generated with Claude Code