Skip to content

decrease max_methods world-splitting setting for some functions #59377

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.

Sign up for GitHub

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

Jump to bottom
Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

decrease max_methods world-splitting setting for some functions #59377

wants to merge 3 commits into from

Conversation

nsajko
Copy link
Member

@nsajko nsajko commented Aug 24, 2025

A function which is meant to have methods added to it by users should have a small max_methods value, as world-splitting just leads to unnecessary invalidation in that case, in the context of the package ecosystem, where users are allowed to add arbitrarily many methods to such functions.

xref PR #57884

xref PR #58788

xref PR #58829

xref PR #59091

@nsajko nsajko added design Design of APIs or of the language itself invalidations latency Latency labels Aug 24, 2025
@nsajko
Copy link
Member Author

nsajko commented Aug 24, 2025

This must be causing tangible inference issues, because the trimming CI fails. However, the logs don't show what the issue is.

@nsajko
Copy link
Member Author

nsajko commented Aug 24, 2025

Ah, there's a bunch of inference failures in the usual test suite, too. Clearly more work is necessary.

nsajko added a commit to nsajko/julia that referenced this pull request Aug 27, 2025
@nsajko
eachindex for linear indexing for Array and Memory: typeassert
2beae9b 
The method return type is known concretely as `OneTo{Int}`, however the
compiler can't tell because there are too many methods matching
`length(::Array)`, over the world-splitting threshold, `max_methods`.

Cross-reference PR JuliaLang#57627, which might help here in another way by
decreasing the number of unnecessary methods, however merging that PR
would also cause regressions if world-splitting is not disabled first
for `length` (xref PR JuliaLang#59377).

Fix that by asserting the return type of the `length` call as `Int`.

The `typeassert` should improve abstract return type inference, and
consequently make the sysimage more resistant to invalidation.
nsajko added a commit to nsajko/julia that referenced this pull request Aug 27, 2025
@nsajko
eachindex for linear indexing for Array and Memory: typeassert
b02d0c6 
The method return type is known concretely as `OneTo{Int}`, however the
compiler can't tell because there are too many methods matching
`length(::Array)`, over the world-splitting threshold, `max_methods`.

Cross-reference WIP PR JuliaLang#57627, which might help here in another way by
decreasing the number of unnecessary methods, however merging that PR
would also cause regressions if world-splitting is not disabled first
for `length` (xref WIP PR JuliaLang#59377).

Fix that by asserting the return type of the `length` call as `Int`.

The `typeassert` should improve abstract return type inference, and
consequently make the sysimage more resistant to invalidation.
@nsajko nsajko mentioned this pull request Aug 27, 2025
Closed
@nsajko nsajko force-pushed the decrease_max_methods_setting_for_various_interface_functions branch from f9ddf54 to a47559f Compare August 27, 2025 21:22
@nsajko

This comment was marked as outdated.

Sign in to view
@nsajko nsajko mentioned this pull request Aug 27, 2025
Closed
@nsajko
Copy link
Member Author

nsajko commented Aug 27, 2025

I added a bunch of concrete type asserts. Some are necessary to make preexisting tests pass. Also see issue #42372.

nsajko added a commit to nsajko/julia that referenced this pull request Aug 28, 2025
@nsajko
avoid method proliferation for Tuple functions
2b7a231 
* Introducing new types and methods for a callable can invalidate
  already compiled method instances of a function for which
  world-splitting is enabled (`max_methods`).

* Invalidation of sysimage or package precompiled code worsens latency
  due to requiring recompilation.

* Lowering the `max_methods` setting for a function often causes
  inference issues for existing code that is not completely
  type-stable (which is a lot of code). In many cases this is easy to
  fix by avoiding method proliferation, such as by merging some methods
  and introducing branching into the merged method.

This PR aims to fix the latter issue for some `Tuple`-related methods
of some functions where decreasing `max_methods` might be interesting.

Seeing as branching was deliberately avoided in the bodies of many of
these methods, I opted for the approach of introducing local functions
which preserve the dispatch logic as before, without branching. Thus
there should be no regressions, except perhaps because of changed
inlining costs.

This PR is a prerequisite for PRs which try to decrease `max_methods`,
such as PRs:

* JuliaLang#59091

* JuliaLang#59377
@nsajko nsajko mentioned this pull request Aug 28, 2025
Open
nsajko added a commit to nsajko/julia that referenced this pull request Aug 28, 2025
@nsajko
avoid method proliferation for Tuple functions
faa2457 
* Introducing new types and methods for a callable can invalidate
  already compiled method instances of a function for which
  world-splitting is enabled (`max_methods`).

* Invalidation of sysimage or package precompiled code worsens latency
  due to requiring recompilation.

* Lowering the `max_methods` setting for a function often causes
  inference issues for existing code that is not completely
  type-stable (which is a lot of code). In many cases this is easy to
  fix by avoiding method proliferation, such as by merging some methods
  and introducing branching into the merged method.

This PR aims to fix the latter issue for some `Tuple`-related methods
of some functions where decreasing `max_methods` might be interesting.

Seeing as branching was deliberately avoided in the bodies of many of
these methods, I opted for the approach of introducing local functions
which preserve the dispatch logic as before, without branching. Thus
there should be no regressions, except perhaps because of changed
inlining costs.

This PR is a prerequisite for PRs which try to decrease `max_methods`
for select functions, such as PR:

* JuliaLang#59377
nsajko added a commit to nsajko/julia that referenced this pull request Aug 28, 2025
@nsajko
avoid method proliferation for Tuple functions
f733b5b 
* Introducing new types and methods for a callable can invalidate
  already compiled method instances of a function for which
  world-splitting is enabled (`max_methods`).

* Invalidation of sysimage or package precompiled code worsens latency
  due to requiring recompilation.

* Lowering the `max_methods` setting for a function often causes
  inference issues for existing code that is not completely
  type-stable (which is a lot of code). In many cases this is easy to
  fix by avoiding method proliferation, such as by merging some methods
  and introducing branching into the merged method.

This PR aims to fix the latter issue for some `Tuple`-related methods
of some functions where decreasing `max_methods` might be interesting.

Seeing as branching was deliberately avoided in the bodies of many of
these methods, I opted for the approach of introducing local functions
which preserve the dispatch logic as before, without branching. Thus
there should be no regressions, except perhaps because of changed
inlining costs.

This PR is a prerequisite for PRs which try to decrease `max_methods`
for select functions, such as PR:

* JuliaLang#59377
nsajko added a commit to nsajko/julia that referenced this pull request Aug 28, 2025
@nsajko
avoid method proliferation for Tuple functions
53f0434 
* Introducing new types and methods for a callable can invalidate
  already compiled method instances of a function for which
  world-splitting is enabled (`max_methods`).

* Invalidation of sysimage or package precompiled code worsens latency
  due to requiring recompilation.

* Lowering the `max_methods` setting for a function often causes
  inference issues for existing code that is not completely
  type-stable (which is a lot of code). In many cases this is easy to
  fix by avoiding method proliferation, such as by merging some methods
  and introducing branching into the merged method.

This PR aims to fix the latter issue for some `Tuple`-related methods
of some functions where decreasing `max_methods` might be interesting.

Seeing as branching was deliberately avoided in the bodies of many of
these methods, I opted for the approach of introducing local functions
which preserve the dispatch logic as before, without branching. Thus
there should be no regressions, except perhaps because of changed
inlining costs.

This PR is a prerequisite for PRs which try to decrease `max_methods`
for select functions, such as PR:

* JuliaLang#59377
nsajko added a commit to nsajko/julia that referenced this pull request Aug 28, 2025
@nsajko
avoid method proliferation for Tuple functions
3f81a51 
* Introducing new types and methods for a callable can invalidate
  already compiled method instances of a function for which
  world-splitting is enabled (`max_methods`).

* Invalidation of sysimage or package precompiled code worsens latency
  due to requiring recompilation.

* Lowering the `max_methods` setting for a function often causes
  inference issues for existing code that is not completely
  type-stable (which is a lot of code). In many cases this is easy to
  fix by avoiding method proliferation, such as by merging some methods
  and introducing branching into the merged method.

This PR aims to fix the latter issue for some `Tuple`-related methods
of some functions where decreasing `max_methods` might be interesting.

Seeing as branching was deliberately avoided in the bodies of many of
these methods, I opted for the approach of introducing local functions
which preserve the dispatch logic as before, without branching. Thus
there should be no regressions, except perhaps because of changed
inlining costs.

This PR is a prerequisite for PRs which try to decrease `max_methods`
for select functions, such as PR:

* JuliaLang#59377
nsajko added a commit to nsajko/julia that referenced this pull request Aug 28, 2025
@nsajko
avoid method proliferation for Tuple functions
1314ee7 
* Introducing new types and methods for a callable can invalidate
  already compiled method instances of a function for which
  world-splitting is enabled (`max_methods`).

* Invalidation of sysimage or package precompiled code worsens latency
  due to requiring recompilation.

* Lowering the `max_methods` setting for a function often causes
  inference issues for existing code that is not completely
  type-stable (which is a lot of code). In many cases this is easy to
  fix by avoiding method proliferation, such as by merging some methods
  and introducing branching into the merged method.

This PR aims to fix the latter issue for some `Tuple`-related methods
of some functions where decreasing `max_methods` might be interesting.

Seeing as branching was deliberately avoided in the bodies of many of
these methods, I opted for the approach of introducing local functions
which preserve the dispatch logic as before, without branching. Thus
there should be no regressions, except perhaps because of changed
inlining costs.

This PR is a prerequisite for PRs which try to decrease `max_methods`
for select functions, such as PR:

* JuliaLang#59377
nsajko added a commit to nsajko/julia that referenced this pull request Aug 28, 2025
@nsajko
avoid method proliferation for Tuple functions
89ebaa7 
* Introducing new types and methods for a callable can invalidate
  already compiled method instances of a function for which
  world-splitting is enabled (`max_methods`).

* Invalidation of sysimage or package precompiled code worsens latency
  due to requiring recompilation.

* Lowering the `max_methods` setting for a function often causes
  inference issues for existing code that is not completely
  type-stable (which is a lot of code). In many cases this is easy to
  fix by avoiding method proliferation, such as by merging some methods
  and introducing branching into the merged method.

This PR aims to fix the latter issue for some `Tuple`-related methods
of some functions where decreasing `max_methods` might be interesting.

Seeing as branching was deliberately avoided in the bodies of many of
these methods, I opted for the approach of introducing local functions
which preserve the dispatch logic as before, without branching. Thus
there should be no regressions, except perhaps because of changed
inlining costs.

This PR is a prerequisite for PRs which try to decrease `max_methods`
for select functions, such as PR:

* JuliaLang#59377
nsajko added a commit to nsajko/julia that referenced this pull request Aug 28, 2025
@nsajko
avoid method proliferation for Tuple functions
32f6824 
* Introducing new types and methods for a callable can invalidate
  already compiled method instances of a function for which
  world-splitting is enabled (`max_methods`).

* Invalidation of sysimage or package precompiled code worsens latency
  due to requiring recompilation.

* Lowering the `max_methods` setting for a function often causes
  inference issues for existing code that is not completely
  type-stable (which is a lot of code). In many cases this is easy to
  fix by avoiding method proliferation, such as by merging some methods
  and introducing branching into the merged method.

This PR aims to fix the latter issue for some `Tuple`-related methods
of some functions where decreasing `max_methods` might be interesting.

Seeing as branching was deliberately avoided in the bodies of many of
these methods, I opted for the approach of introducing local functions
which preserve the dispatch logic as before, without branching. Thus
there should be no regressions, except perhaps because of changed
inlining costs.

This PR is a prerequisite for PRs which try to decrease `max_methods`
for select functions, such as PR:

* JuliaLang#59377
nsajko added 2 commits August 29, 2025 22:29
@nsajko
avoid method proliferation for Tuple functions
d494973 
* Introducing new types and methods for a callable can invalidate
  already compiled method instances of a function for which
  world-splitting is enabled (`max_methods`).

* Invalidation of sysimage or package precompiled code worsens latency
  due to requiring recompilation.

* Lowering the `max_methods` setting for a function often causes
  inference issues for existing code that is not completely
  type-stable (which is a lot of code). In many cases this is easy to
  fix by avoiding method proliferation, such as by merging some methods
  and introducing branching into the merged method.

This PR aims to fix the latter issue for some `Tuple`-related methods
of some functions where decreasing `max_methods` might be interesting.

Seeing as branching was deliberately avoided in the bodies of many of
these methods, I opted for the approach of introducing local functions
which preserve the dispatch logic as before, without branching. Thus
there should be no regressions, except perhaps because of changed
inlining costs.

This PR is a prerequisite for PRs which try to decrease `max_methods`
for select functions, such as PR:

* JuliaLang#59377
@nsajko
decrease max_methods world-splitting setting for some functions
17ca436 
A function which is meant to have methods added to it by users should
have a small `max_methods` value, as world-splitting just leads to
unnecessary invalidation in that case, in the context of the package
ecosystem, where users are allowed to add arbitrarily many methods to
such functions.

xref PR JuliaLang#57884

xref PR JuliaLang#58788

xref PR JuliaLang#58829

xref PR JuliaLang#59091
@nsajko nsajko force-pushed the decrease_max_methods_setting_for_various_interface_functions branch from a563401 to e7c44cc Compare August 29, 2025 20:30
@nsajko
Copy link
Member Author

nsajko commented Aug 29, 2025

This now temporarily includes the commit from PR #59421, however I expect that one will be merged before this one will be considered.

@nsajko
Copy link
Member Author

nsajko commented Aug 29, 2025

The only test failure is curious:

boundscheck                                     (10) |        started at 2025-08-29T14:05:54.274
      From worker 10: Error During Test at /Users/julia/.julia/scratchspaces/a66863c6-20e8-4ff4-8a62-49f30b1f605e/agent-cache/default-honeycrisp-R17H3W25T9.0/build/default-honeycrisp-R17H3W25T9-0/julialang/julia-master/julia-e7c44cc7ba/share/julia/test/boundscheck_exec.jl:287
      From worker 10:   Test threw exception
      From worker 10:   Expression: getindex_42115(R, -1:3, :) == CartesianIndices((-1:3, 1:5))
      From worker 10:   BoundsError: attempt to access 5×5 CartesianIndices{2, Tuple{Base.OneTo{Int64}, Base.OneTo{Int64}}} at index [-1:3, :]
      From worker 10:   Stacktrace:
      From worker 10:    [1] throw_boundserror(A::CartesianIndices{2, Tuple{Base.OneTo{Int64}, Base.OneTo{Int64}}}, I::Tuple{UnitRange{Int64}, Colon})
      From worker 10:      @ Base ./essentials.jl:13
      From worker 10:    [2] checkbounds
      From worker 10:      @ ./abstractarray.jl:698 [inlined]
      From worker 10:    [3] getindex(::CartesianIndices{2, Tuple{Base.OneTo{Int64}, Base.OneTo{Int64}}}, ::UnitRange{Int64}, ::Colon)
      From worker 10:      @ Base.IteratorsMD ./multidimensional.jl:413
      From worker 10:    [4] getindex_42115(r::CartesianIndices{2, Tuple{Base.OneTo{Int64}, Base.OneTo{Int64}}}, i::UnitRange{Int64}, j::Function)
      From worker 10:      @ Main.TestBoundsCheck /Users/julia/.julia/scratchspaces/a66863c6-20e8-4ff4-8a62-49f30b1f605e/agent-cache/default-honeycrisp-R17H3W25T9.0/build/default-honeycrisp-R17H3W25T9-0/julialang/julia-master/julia-e7c44cc7ba/share/julia/test/boundscheck_exec.jl:271
      From worker 10:    [5] top-level scope
      From worker 10:      @ /Users/julia/.julia/scratchspaces/a66863c6-20e8-4ff4-8a62-49f30b1f605e/agent-cache/default-honeycrisp-R17H3W25T9.0/build/default-honeycrisp-R17H3W25T9-0/julialang/julia-master/julia-e7c44cc7ba/share/julia/test/boundscheck_exec.jl:287
      From worker 10:    [6] macro expansion
      From worker 10:      @ /Users/julia/.julia/scratchspaces/a66863c6-20e8-4ff4-8a62-49f30b1f605e/agent-cache/default-honeycrisp-R17H3W25T9.0/build/default-honeycrisp-R17H3W25T9-0/julialang/julia-master/julia-e7c44cc7ba/share/julia/stdlib/v1.13/Test/src/Test.jl:745 [inlined]
      From worker 10: ERROR: LoadError: There was an error during testing
      From worker 10: in expression starting at /Users/julia/.julia/scratchspaces/a66863c6-20e8-4ff4-8a62-49f30b1f605e/agent-cache/default-honeycrisp-R17H3W25T9.0/build/default-honeycrisp-R17H3W25T9-0/julialang/julia-master/julia-e7c44cc7ba/share/julia/test/boundscheck_exec.jl:3
boundscheck                                     (10) |         failed at 2025-08-29T14:05:57.069
Error During Test at /Users/julia/.julia/scratchspaces/a66863c6-20e8-4ff4-8a62-49f30b1f605e/agent-cache/default-honeycrisp-R17H3W25T9.0/build/default-honeycrisp-R17H3W25T9-0/julialang/julia-master/julia-e7c44cc7ba/share/julia/test/testdefs.jl:26
  Got exception outside of a @test
  LoadError: boundscheck test failed, cmd : `/Users/julia/.julia/scratchspaces/a66863c6-20e8-4ff4-8a62-49f30b1f605e/agent-cache/default-honeycrisp-R17H3W25T9.0/build/default-honeycrisp-R17H3W25T9-0/julialang/julia-master/julia-e7c44cc7ba/bin/julia -C native -J/Users/julia/.julia/scratchspaces/a66863c6-20e8-4ff4-8a62-49f30b1f605e/agent-cache/default-honeycrisp-R17H3W25T9.0/build/default-honeycrisp-R17H3W25T9-0/julialang/julia-master/julia-e7c44cc7ba/lib/julia/sys.dylib --depwarn=error --check-bounds=yes -g1 --startup-file=no --check-bounds=auto --depwarn=error --startup-file=no boundscheck_exec.jl`

CI log link:

The test on line 287 here is the one that fails:

julia/test/boundscheck_exec.jl

Lines 267 to 289 in 3396562

# Given this is a sub-processed test file, not using @testsets avoids
# leaking the report print into the Base test runner report
begin # Pass inbounds meta to getindex on CartesianIndices (#42115)
@inline getindex_42115(r, i) = @inbounds getindex(r, i)
@inline getindex_42115(r, i, j) = @inbounds getindex(r, i, j)
R = CartesianIndices((5, 5))
if bc_opt == bc_on
@test_throws BoundsError getindex_42115(R, -1, -1)
@test_throws BoundsError getindex_42115(R, 1, -1)
else
@test getindex_42115(R, -1, -1) == CartesianIndex(-1, -1)
@test getindex_42115(R, 1, -1) == CartesianIndex(1, -1)
end
if bc_opt == bc_on
@test_throws BoundsError getindex_42115(R, CartesianIndices((6, 6)))
@test_throws BoundsError getindex_42115(R, -1:3, :)
else
@test getindex_42115(R, CartesianIndices((6, 6))) == CartesianIndices((6, 6))
@test getindex_42115(R, -1:3, :) == CartesianIndices((-1:3, 1:5))
end
end

Perhaps a compiler bug?

@nsajko
relax a test in test/rational.jl
78499ab 
As noted by simeonschaub on the original issue JuliaLang#41489, this only used
to work by accident anyway.
@nsajko nsajko force-pushed the decrease_max_methods_setting_for_various_interface_functions branch from e7c44cc to 78499ab Compare August 30, 2025 10:47
@nsajko nsajko mentioned this pull request Aug 30, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
design Design of APIs or of the language itself invalidations latency Latency
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@nsajko