Skip to content

Commit

Permalink
Assume size is non-negative for increased efficiency (#50530)
Browse files Browse the repository at this point in the history
I noticed
[here](#50467 (comment))
that `lastindex(x::Base.OneTo)` is not simply `x.stop`. This PR performs
that optimization and many more by assuming `size` always returns
positive numbers.
```
julia> @code_native lastindex(Base.OneTo(5)) # master
        .section        __TEXT,__text,regular,pure_instructions
        .build_version macos, 13, 0
        .globl  _julia_lastindex_81             ; -- Begin function julia_lastindex_81
        .p2align        2
_julia_lastindex_81:                    ; @julia_lastindex_81
; ┌ @ abstractarray.jl:423 within `lastindex`
; %bb.0:                                ; %top
; │┌ @ abstractarray.jl:386 within `eachindex`
; ││┌ @ abstractarray.jl:134 within `axes1`
; │││┌ @ range.jl:708 within `axes`
; ││││┌ @ range.jl:471 within `oneto`
; │││││┌ @ range.jl:469 within `OneTo` @ range.jl:454
; ││││││┌ @ promotion.jl:532 within `max`
; │││││││┌ @ int.jl:83 within `<`
        ldr     x8, [x0]
; │││││││└
; │││││││┌ @ essentials.jl:642 within `ifelse`
        cmp     x8, #0
        csel    x0, x8, xzr, gt
; │└└└└└└└
        ret
; └
                                        ; -- End function
.subsections_via_symbols

julia> @code_native lastindex(Base.OneTo(5)) # pr
        .section        __TEXT,__text,regular,pure_instructions
        .build_version macos, 13, 0
        .globl  _julia_lastindex_13253          ; -- Begin function julia_lastindex_13253
        .p2align        2
_julia_lastindex_13253:                 ; @julia_lastindex_13253
; ┌ @ abstractarray.jl:423 within `lastindex`
; %bb.0:                                ; %top
        ldr     x0, [x0]
        ret
; └
                                        ; -- End function
.subsections_via_symbols
```

Also removed `axes(r::AbstractRange) = (oneto(length(r)),)` (added in
#40382, @vtjnash) as redundant with the general `axes` method.

The obvious downside here is that if someone defines an object with
negative size, its axes will include Base.OneTo with negative stop. I
think that is acceptable, but if not, we can gate this optimization to a
set of known types (all AbstractArray types defined in Base should have
non-negative size)
  • Loading branch information
LilithHafner authored Jul 15, 2023
2 parents 99e2604 + cae9576 commit 191256e
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 4 deletions.
2 changes: 1 addition & 1 deletion base/abstractarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ julia> axes(A)
"""
function axes(A)
@inline
map(oneto, size(A))
map(unchecked_oneto, size(A))
end

"""
Expand Down
6 changes: 3 additions & 3 deletions base/range.jl
Original file line number Diff line number Diff line change
Expand Up @@ -447,7 +447,7 @@ distinction that the lower limit is guaranteed (by the type system) to
be 1.
"""
struct OneTo{T<:Integer} <: AbstractUnitRange{T}
stop::T
stop::T # invariant: stop >= zero(stop)
function OneTo{T}(stop) where {T<:Integer}
throwbool(r) = (@noinline; throw(ArgumentError("invalid index: $r of type Bool")))
T === Bool && throwbool(stop)
Expand All @@ -463,6 +463,8 @@ struct OneTo{T<:Integer} <: AbstractUnitRange{T}
T === Bool && throwbool(r)
return new(max(zero(T), last(r)))
end

global unchecked_oneto(stop::Integer) = new{typeof(stop)}(stop)
end
OneTo(stop::T) where {T<:Integer} = OneTo{T}(stop)
OneTo(r::AbstractRange{T}) where {T<:Integer} = OneTo{T}(r)
Expand Down Expand Up @@ -703,8 +705,6 @@ step(r::LinRange) = (last(r)-first(r))/r.lendiv
step_hp(r::StepRangeLen) = r.step
step_hp(r::AbstractRange) = step(r)

axes(r::AbstractRange) = (oneto(length(r)),)

# Needed to ensure `has_offset_axes` can constant-fold.
has_offset_axes(::StepRange) = false

Expand Down
1 change: 1 addition & 0 deletions test/testhelpers/InfiniteArrays.jl
Original file line number Diff line number Diff line change
Expand Up @@ -48,5 +48,6 @@ Base.length(r::OneToInf) = Infinity()
Base.last(r::OneToInf) = Infinity()
Base.unitrange(r::OneToInf) = r
Base.oneto(::Infinity) = OneToInf()
Base.unchecked_oneto(::Infinity) = OneToInf()

end

0 comments on commit 191256e

Please sign in to comment.