julia icon indicating copy to clipboard operation
julia copied to clipboard

reimplement `_setindex` using recursion

Open nsajko opened this issue 1 year ago • 4 comments

FTR, this PR is part of a series of changes which (re)implement many of the operations on tuples using a new recursive technique. The ultimate goal is to hopefully increase the value of the loop-vs-recurse cutoff (Any32, sometimes hardcoded 32) for tuple operations.

Updates #54462

Closes #54463

Example type inference improvement:

julia> Core.Compiler.return_type(Base.setindex, Tuple{Tuple{Vararg{Int}},Int,Int})
Tuple{Vararg{Int64}}

nsajko avatar May 15 '24 18:05 nsajko

It is difficult to understand the relationship to the issue and prior PR. (typo?)

from https://github.com/JuliaLang/julia/pull/54463#issuecomment-2111227953

(I think it is also misleading that the function _setindex exists in Base. Can't we use _setindex_tuple or something like that?)

I think there are two problems with the current (master) _setindex that have nothing to do with the inference.

  • Although the Base.setindex is also implemented by NamedTuple, etc., the signature does not indicate that it is for Tuple (due to the splatting).
  • The order of arguments is different from setindex due to the Vararg technique.

If we don't use Vararg, I don't see the need to change the order of the arguments from setindex.

kimikage avatar May 15 '24 21:05 kimikage

I think the true NTuple cases (x::NTuple{N, T}, v::T, i::Integer) where {N, T} are worth specializing. It is easier to understand for humans as well as the compiler.

kimikage avatar May 15 '24 22:05 kimikage

Regarding the naming of the functions, that's way off-topic IMO (to this PR).

I think the true NTuple cases ... are worth specializing

Why? As shown above, the generic implementation seems to handle this just fine.

nsajko avatar May 15 '24 22:05 nsajko

Regarding the naming of the functions, that's way off-topic IMO (to this PR).

Agreed. However, we would probably lose the opportunity to change it.

I think the true NTuple cases ... are worth specializing

Why?

As noted above, it is because it is easy for humans to understand. TBH, I still don't understand this PR. Also, I believe that not only inference but also runtime performance is important. Some benchmarks would be helpful.

If we were to try to support Base.setindex((1, 2, 3), 0, 2:3), would this PR technique work well? (Edit: cf. #54508) Of course that has nothing to do with this PR, but I don't think having a simple implementation is a bad thing.

kimikage avatar May 15 '24 22:05 kimikage