Skip to content

Conversation

nsajko
Copy link
Member

@nsajko nsajko commented Jun 21, 2025

The eltype function was one of the few functions in the sysimage with a max_methods value (the world-splitting threshold) greater than the default. This was a workaround for the unnecessarily large number of methods of eltype(::Type{<:Tuple}). The max_methods value was increased in PR #48322 to help effect inference.

Reduce the number of eltype(::Type{<:Tuple}) methods, which then also allows keeping the max_methods for eltype at the default value.

The intent here is to guard against unnecessary invalidation of the sysimage or other precompiled code, which might require decreasing the max_methods value to the natural minimum for many interface functions, by which I mean "generic function meant to have methods added to it by package authors". I intend to approach this issue in following PRs. See also: PR #57884.

Regarding future work: I consider the "natural minimum" value for interface functions taking types to be two, because the bottom type subtypes each type. If the interface function doesn't accept types, the natural minimum should be one.

@nsajko nsajko added iteration Involves iteration or the iteration protocol invalidations labels Jun 21, 2025
@nsajko
Copy link
Member Author

nsajko commented Jun 21, 2025

FTR: in another PR I'll try to reduce the max_methods for these functions (taking types) to 0x2:

  • eltype

  • elsize

  • IteratorSize

  • IteratorEltype

  • IndexStyle

  • BroadcastStyle

  • ndims

  • one

  • zero

  • oneunit

  • widen

Furthermore I'll try reducing the max_methods value for interface functions that don't take types to 0x1, such as the following functions (copyto! already has its max_methods set to 0x1):

  • propertynames

  • getproperty

  • setproperty!

  • show

  • print

  • size

  • axes

  • isdone

  • nextind

  • prevind

  • thisind

  • length

  • isempty

  • iterate

  • firstindex

  • lastindex

  • setindex!

  • isone

  • iszero

  • copy

  • strides

  • stride

  • Base.Broadcast.broadcastable

  • Base.Broadcast.instantiate

The `eltype` function was one of the few functions in the sysimage with
a `max_methods` value (the world-splitting threshold) greater than the
default. This was a workaround for the unnecessarily large number of
methods of `eltype(::Type{<:Tuple})`. The `max_methods` value was
increased in PR JuliaLang#48322 to help effect inference.

Reduce the number of `eltype(::Type{<:Tuple})` methods, which then also
allows keeping the `max_methods` for `eltype` at the default value.

The intent here is to guard against unnecessary invalidation of the
sysimage or other precompiled code, which might require decreasing the
`max_methods` value to the natural minimum for many interface
functions, by which I mean "generic function meant to have methods
added to it by package authors". I intend to approach this issue in
following PRs. See also: PR JuliaLang#57884.

Regarding future work: I consider the "natural minimum" value for
interface functions taking types to be **two**, because the bottom type
subtypes each type. If the interface function doesn't accept types, the
natural minimum should be **one**.
@nsajko nsajko force-pushed the decrease_method_count_for_eltype_Tuple_to_rely_on_world_splitting_less branch from 600869c to 2e32141 Compare June 21, 2025 15:20
@nsajko nsajko marked this pull request as ready for review June 21, 2025 17:02
nsajko added a commit to nsajko/julia that referenced this pull request Jun 27, 2025
I call callables that are meant to have methods added to by package
authors "interface callables".

Test that various interface callables are well-behaved in case of the
introduction of a new applicable type. Only interface callables taking
a single type argument are tested.

The specific properties tested:

* `max_methods` is high enough for good inference

* the number of matching methods for a new type is not excessive

The primary motivation for this change is to make any future lowering
of the `max_methods` setting of the interface callables safer.

To make the tests pass, fix a recent regression in
base/strings/search.jl.

This change depends on PR JuliaLang#58788 for `eltype`.
@nsajko
Copy link
Member Author

nsajko commented Jun 27, 2025

Bump. Other potential changes depend on this change, starting with PR #58829. Seems that PR doesn't depend on this one. Still, both need to be merged before lowering the max_method setting.

nsajko added a commit to nsajko/julia that referenced this pull request Jun 27, 2025
I call callables that are meant to have methods added to by package
authors "interface callables".

Test that various interface callables are well-behaved in case of the
introduction of a new applicable type. Only interface callables taking
a single type argument are tested.

The specific properties tested:

* `max_methods` is high enough for good inference

* the number of matching methods for a new type is not excessive

The primary motivation for this change is to make any future lowering
of the `max_methods` setting of the interface callables safer.

To make the tests pass, fix a recent regression in
base/strings/search.jl.

This change depends on PR JuliaLang#58788 for `eltype`.
nsajko added a commit to nsajko/julia that referenced this pull request Jun 27, 2025
I call callables that are meant to have methods added to by package
authors "interface callables".

Test that various interface callables are well-behaved in case of the
introduction of a new applicable type. Only interface callables taking
a single type argument are tested.

The specific properties tested:

* `max_methods` is high enough for good inference

* the number of matching methods for a new type is not excessive

The primary motivation for this change is to make any future lowering
of the `max_methods` setting of the interface callables safer.

To make the tests pass, fix a recent regression in
base/strings/search.jl.

This change depends on PR JuliaLang#58788 for `eltype`.
@nsajko
Copy link
Member Author

nsajko commented Jul 12, 2025

Bump.

I have to note most text I wrote here on the PR discussion is only motivationally relevant for this PR. So, if you want to review, it's not really necessary to read.

Also, I see Jeff Bezanson gave a thumbs-up, for what it's worth.

@nsajko
Copy link
Member Author

nsajko commented Jul 25, 2025

bump

@oscardssmith oscardssmith added the triage This should be discussed on a triage call label Aug 9, 2025
@nsajko
Copy link
Member Author

nsajko commented Aug 10, 2025

I don't see why this requires triage? Seems like a straightforward improvement. Technically this could cause performance issues for packages that rely on world-splitting too much, but it's still a very conservative change.

To be clear, this change just reduces the max_methods value from four to the default value of three.

@LilithHafner
Copy link
Member

Is there a question for triage?

nsajko added a commit to nsajko/julia that referenced this pull request 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 JuliaLang#57884

xref PR JuliaLang#58788

xref PR JuliaLang#58829

xref PR JuliaLang#59091
@nsajko
Copy link
Member Author

nsajko commented Aug 27, 2025

bump

nsajko added a commit to nsajko/julia that referenced this pull request Aug 27, 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 JuliaLang#57884

xref PR JuliaLang#58788

xref PR JuliaLang#58829

xref PR JuliaLang#59091
@JeffBezanson JeffBezanson merged commit 9ef12b3 into JuliaLang:master Aug 28, 2025
8 checks passed
@JeffBezanson JeffBezanson removed the triage This should be discussed on a triage call label Aug 28, 2025
@nsajko nsajko deleted the decrease_method_count_for_eltype_Tuple_to_rely_on_world_splitting_less branch August 28, 2025 17:43
nsajko added a commit to nsajko/julia that referenced this pull request Aug 29, 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 JuliaLang#57884

xref PR JuliaLang#58788

xref PR JuliaLang#58829

xref PR JuliaLang#59091
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
invalidations iteration Involves iteration or the iteration protocol
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants