Skip to content

Avoid diff allocation for integer range indices#3014

Open
KookiesNKareem wants to merge 2 commits into
EnzymeAD:mainfrom
KookiesNKareem:range-index-fastpath
Open

Avoid diff allocation for integer range indices#3014
KookiesNKareem wants to merge 2 commits into
EnzymeAD:mainfrom
KookiesNKareem:range-index-fastpath

Conversation

@KookiesNKareem

@KookiesNKareem KookiesNKareem commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR adds range-specific fast paths for integer OrdinalRange indices when Reactant checks slice stride and index contiguity.

The existing generic fallback calls diff(x), which materializes an O(n) vector just to inspect the stride/contiguity of ranges such as 1:n, 1:2:n, and colon-derived Base.Slice(Base.OneTo(n)). Integer ordinal ranges already expose this information through step(x), so these checks can be O(1) and allocation-free.

The change is deliberately narrow:

  • applies only to OrdinalRange{<:Integer,<:Integer}
  • preserves empty range behavior for get_slice_stride(1:0) == -1
  • preserves single-element behavior for get_slice_stride(1:1) == 1
  • covers colon-derived Base.Slice(Base.OneTo(n)) indices
  • leaves vector, boolean, traced, non-integer, and custom non-ordinal index paths on the existing fallback

Benchmark evidence

Measured on NVIDIA TITAN RTX with Reactant loaded:

get_slice_stride(1:1_000_000)
before: 5621.363 us, 8,000,072 bytes
after:     0.124 us, 0 bytes

__contiguous_indices(1:1_000_000)
before: 5650.619 us, 8,000,072 bytes
after:     0.024 us, 0 bytes

collect(1:100000) vector inputs remain on the existing fallback path:
~539 us / ~528 us, 800,072 bytes

Validation

Added focused regression coverage in test/core/indexing.jl for:

  • empty integer ranges preserving the existing no-slice sentinel
  • contiguous integer ranges reporting stride 1 and contiguity
  • strided integer ranges reporting their explicit step and non-contiguity
  • colon-derived Base.Slice(Base.OneTo(n)) ranges using the fast path

@KookiesNKareem KookiesNKareem marked this pull request as ready for review June 29, 2026 07:13
@codecov

codecov Bot commented Jun 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 43.44%. Comparing base (b39a1fc) to head (6832236).
⚠️ Report is 1143 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #3014       +/-   ##
===========================================
- Coverage   68.16%   43.44%   -24.73%     
===========================================
  Files         109      223      +114     
  Lines       11779    31598    +19819     
===========================================
+ Hits         8029    13727     +5698     
- Misses       3750    17871    +14121     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@wsmoses wsmoses requested review from Pangoraw, avik-pal and mofeing June 29, 2026 11:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant