[Experiment]: set max_methods = 1

[KristofferC]

This is just to see the effect of this change on Base tests, PkgEval, benchmarks etc.

Advantages of lowering this:

• Lower inference time
• Less invalidations

Disadvantages:

• Less precise inference results in presence of non-concrete types

[KristofferC]

@nanosoldier runtests().

[KristofferC]

@nanosoldier runbenchmarks(ALL, vs=":master")

[nanosoldier]

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here.

[vtjnash]

While nothing looks like it regressed, it is interesting how much worse that made the performance of the inference benchmarks themselves.

[KristofferC]

This regressed:

["problem", "raytrace", "raytrace"]	3.00 (5%) ❌	2.03 (1%) ❌

But yeah, interesting to see inference itself being so sensitive to this.

[aviatesk]

Interesting. We can have module-wide @max_methods=3 for Core.Compiler, and for the other place it might be okay to set max_methods=1 by default.
@nanosoldier runbenchmarks("inference", vs=":master")

[nanosoldier]

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here.

[KristofferC]

["inference", "abstract interpretation", "many_opaque_closures"]	0.07 (5%) ✅	0.10 (1%) ✅

🎉 😆

[timholy]

I'm not entirely surprised: abstract inference is much slower than concrete, and this may result in more abstract inference because there may be fewer cases where it can determine a concrete return type. Which might mean a reduction of some invalidation-vulnerabilities, but an increase in others.

[aviatesk]

Well, the max_methods=1 configuration doesn't directly indicate that we only do concrete inference (it should certainly increase the chances of doing concrete inference though). We can set up a separate configuration for doing concrete-only inference.

[nanosoldier]

Your package evaluation job has completed - possible new issues were detected. A full report can be found here.

[KristofferC]

People are writing a lot of methods with too strict type parameters that do not have a fallback which causes breakage since the types of empty containers can now change due to explicit use of inference in e.g. map and broadcasting etc.

CairoMakie [13f3f980-e62b-5c42-98c6-ff1f3baf88f0]

Failed to precompile CairoMakie [13f3f980-e62b-5c42-98c6-ff1f3baf88f0] to "/home/pkgeval/.julia/compiled/v1.10/CairoMakie/jl_NdMVzk".
ERROR: LoadError: MethodError: no method matching GeometryBasics.Polygon(::Vector{GeometryBasics.Point{2, Float32}}, ::Vector{Any})

Closest candidates are:
  GeometryBasics.Polygon(::AbstractVector{P}) where {Dim, T, P<:GeometryBasics.AbstractPoint{Dim, T}}
   @ GeometryBasics ~/.julia/packages/GeometryBasics/KE3OI/src/basic_types.jl:273
  GeometryBasics.Polygon(::AbstractVector{P}, !Matched::AbstractVector{<:GeometryBasics.LineFace}) where {Dim, T, P<:GeometryBasics.AbstractPoint{Dim, T}}
   @ GeometryBasics ~/.julia/packages/GeometryBasics/KE3OI/src/basic_types.jl:283
  GeometryBasics.Polygon(::AbstractVector{P}, !Matched::AbstractVector{<:AbstractVector{P}}) where {Dim, T, P<:GeometryBasics.AbstractPoint{Dim, T}}
   @ GeometryBasics ~/.julia/packages/GeometryBasics/KE3OI/src/basic_types.jl:289
  ...

Stacktrace:
  [1] project_polygon(scene::Makie.Scene, space::Symbol, poly::GeometryBasics.Polygon{2, Float32, GeometryBasics.Point{2, Float32}, GeometryBasics.LineString{2, Float32, GeometryBasics.Point{2, Float32}, Base.ReinterpretArray{GeometryBasics.Line{2, Float32}, 1, Tuple{GeometryBasics.Point{2, Float32}, GeometryBasics.Point{2, Float32}}, GeometryBasics.TupleView{Tuple{GeometryBasics.Point{2, Float32}, GeometryBasics.Point{2, Float32}}, 2, 1, Vector{GeometryBasics.Point{2, Float32}}}, false}}, Vector{GeometryBasics.LineString{2, Float32, GeometryBasics.Point{2, Float32}, Base.ReinterpretArray{GeometryBasics.Line{2, Float32}, 1, Tuple{GeometryBasics.Point{2, Float32}, GeometryBasics.Point{2, Float32}}, GeometryBasics.TupleView{Tuple{GeometryBasics.Point{2, Float32}, GeometryBasics.Point{2, Float32}}, 2, 1, Vector{GeometryBasics.Point{2, Float32}}}, false}}}}, model::StaticArraysCore.SMatrix{4, 4, Float32, 16})
    @ CairoMakie ~/.julia/packages/CairoMakie/DhW2d/src/utils.jl:58
  [2] _broadcast_getindex_evalf
    @ ./broadcast.jl:683 [inlined]
  [3] _broadcast_getindex
    @ ./broadcast.jl:656 [inlined]
  [4] getindex
    @ ./broadcast.jl:610 [inlined]

ç

[PallHaraldsson]

Does it make sense to merge this on master? Or maybe with max_methods=2? I'm trying to understand this, want to know all potentially helpful tricks to reduce latency, and if I understand correctly =1 is one of them, fastest/least conservative, but =2 might always work?

Supported values are 1, 2, 3, 4, and default (currently equivalent to 3).

We don't want to break packages, but there's no such promise on master, so I'm thinking would merging =1 there help to force users to fix their code, and then we would change to =2 (or =3 for the release on 1.10, which is still far off). E.g. CairoMakie could set at module level. Does this also apply to non-package code, which I guess can't set max_methods? When does =4 make sense?