Specialize tuple setindex to avoid ntuple-related performance regression.

[maleadt]

Fixes #46049. Not sure what a good test would be, as this does infer correctly.

cc @OlivierHnt

[gbaraldi]

Maybe add a test so that this won't be regressed in the future?

[maleadt]

Not sure what a good test would be, as this does infer correctly.

Any suggestions? string-matching code_llvm output isn't pretty.

[gbaraldi]

What about something like your example, if there are dynamic dispatches there will be allocation so
@allocated(kernel()) == 0

[N5N3]

Not sure what a good test would be, as this does infer correctly.

kernal() is stable just because the wrapper implies an annotation. We can test setindex(::Tuple)'s stability directly:

julia> Base.return_types(Base.setindex, Tuple{NTuple{16,Int},Int,Int})
1-element Vector{Any}:
 Tuple{Vararg{Int64}}  #  NTuple{16, Int64} after this PR

The improvement comes from the fact that ntuple(f, ::Val) is a generated function.
While ntuple(f, n::Int) only tries to const-propagation when n<=10.
So I believe that we don't need _setindex. The following version seems working well

function setindex(x::Tuple, v, i::Integer)
    @boundscheck 1 <= i <= length(x) || throw(BoundsError(x, i))
    @inline
    ntuple(j -> ifelse(j == i, v, x[j]), Val(length(x)))
end

[maleadt]

So I believe that we don't need _setindex. The following version seems working well

Thanks, I've pushed this version.

[N5N3]

Sorry for my recklessness, we still need _setindex to skip boundscheck in selectdim. Has been fixed.

[maleadt]

we still need _setindex to skip boundscheck in selectdim

Why not add @inbounds to the setindex call?

[N5N3]

Why not add @inbounds to the setindex call?

We test Base with --check-bounds=yes thus @inbounds would be ignored during CI.

julia/test/Makefile

Line 26 in 82c3a6f

$(call PRINT_JULIA, $(call spawn,$(JULIA_EXECUTABLE)) --check-bounds=yes --startup-file=no --depwarn=error ./runtests.jl $@)

[maleadt]

We test Base with --check-bounds=yes thus @inbounds would be ignored during CI.

Ah, I thought it was a performance consideration, and didn't realize it actually triggers during CI. I'm not sure it's worth reusing _setindex for that, but 🤷

[maleadt]

I've rebased-out your change and revert, @N5N3, because it included a bit of unnecessary churn. I've left the explicit ::VarArgs{N} in so that we ensure specialization instead of relying on constant-propagation to the Val constructor.

[N5N3]

I think we'd better avoid unnecessary splat. As our compiler might refuse to inline _setindex if x is too long (>=33)

julia> a = ntuple(identity, 33);

julia> @btime setindex($a, 1, 1);
  352.995 ns (2 allocations: 544 bytes)  # no splat : 3.200 ns (0 allocations: 0 bytes)

[KristofferC]

I'll take this as a fix for the regression, further improvements can of course be made as follow ups.