Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fast bounds-check for CartesianIndex ranges #55596

Merged
merged 2 commits into from
Aug 28, 2024

Conversation

jishnub
Copy link
Contributor

@jishnub jishnub commented Aug 27, 2024

StepRangeLen{<:CartesianIndex} indices have been supported since v1.11, but bounds-checking for such indices currently falls back to iterating over the entire range. This PR adds a quick checkindex for such ranges.

The performance improvement as a consequence:

julia> D = Diagonal(1:10_000);

julia> @btime checkbounds($D, diagind($D, IndexCartesian()));
  6.697 μs (0 allocations: 0 bytes) # nightly, O(n)
  4.044 ns (0 allocations: 0 bytes) # This PR, O(1)

@jishnub jishnub added performance Must go faster arrays [a, r, r, a, y, s] backport 1.11 Change should be backported to release-1.11 labels Aug 27, 2024
@jishnub jishnub requested a review from N5N3 August 27, 2024 10:15
@N5N3 N5N3 merged commit 6440292 into master Aug 28, 2024
6 of 8 checks passed
@N5N3 N5N3 deleted the jishnub/cartesian_range_checkindex branch August 28, 2024 08:36
KristofferC pushed a commit that referenced this pull request Sep 9, 2024
`StepRangeLen{<:CartesianIndex}` indices have been supported since
v1.11, but bounds-checking for such indices currently falls back to
iterating over the entire range. This PR adds a quick `checkindex` for
such ranges.

The performance improvement as a consequence:
```julia
julia> D = Diagonal(1:10_000);

julia> @Btime checkbounds($D, diagind($D, IndexCartesian()));
  6.697 μs (0 allocations: 0 bytes) # nightly, O(n)
  4.044 ns (0 allocations: 0 bytes) # This PR, O(1)
```

(cherry picked from commit 6440292)
@KristofferC KristofferC mentioned this pull request Sep 11, 2024
33 tasks
KristofferC pushed a commit that referenced this pull request Sep 12, 2024
`StepRangeLen{<:CartesianIndex}` indices have been supported since
v1.11, but bounds-checking for such indices currently falls back to
iterating over the entire range. This PR adds a quick `checkindex` for
such ranges.

The performance improvement as a consequence:
```julia
julia> D = Diagonal(1:10_000);

julia> @Btime checkbounds($D, diagind($D, IndexCartesian()));
  6.697 μs (0 allocations: 0 bytes) # nightly, O(n)
  4.044 ns (0 allocations: 0 bytes) # This PR, O(1)
```
KristofferC added a commit that referenced this pull request Sep 17, 2024
Backported PRs:
- [x] #55480 <!-- Fix push! for OffsetVectors, add tests for push! and
append! on AbstractVector -->
- [x] #55443 <!-- Add test for upper/lower/titlecase and fix call -->
- [x] #55524 <!-- Set `.jl` sources as read-only during installation -->
- [x] #55500 <!-- make jl_thread_suspend_and_get_state safe -->
- [x] #55506 <!-- Fix indexing in _mapreducedim for OffsetArrays -->
- [x] #55564 <!-- Empty out loaded_precompiles dict instead of asserting
it's empty. -->
- [x] #55567 <!-- Initialize threadpools correctly during sysimg build
-->
- [x] #55596 <!-- Fast bounds-check for CartesianIndex ranges -->
- [x] #55605 <!-- Reroute Symmetric/Hermitian + Diagonal through
triangular -->
- [x] #55640 <!-- win: move stack_overflow_warning to the backtrace
fiber -->
- [x] #55715 <!-- Add precompile signatures to Markdown to reduce
latency. -->
- [x] #55593 <!-- Fix invalidations for FileIO -->
- [x] #55555 <!-- Revert "Don't expose guard pages to malloc_stack API
consumers" -->
- [x] #55720 <!-- Fix `pkgdir` for extensions -->
- [x] #55729 <!-- Avoid confounding compilation side effects of
`@time_imports` -->
- [x] #55718 <!-- Fix `@time_imports` extension recognition -->
- [x] #55522 <!-- Fix tr for Symmetric/Hermitian block matrices -->

Contains multiple commits, manual intervention needed:
- [ ] #55509 <!-- Fix cong implementation to be properly random and not
just cycling. -->

Non-merged PRs with backport label:
- [ ] #55641 <!-- fall back to slower stat filesize if optimized
filesize fails -->
- [ ] #55534 <!-- Set stdlib sources as read-only during installation
-->
- [ ] #55499 <!-- propagate the terminal's `displaysize` to the
`IOContext` used by the REPL -->
- [ ] #55458 <!-- Allow for generically extracting unannotated string
-->
- [ ] #55457 <!-- Make AnnotateChar equality consider annotations -->
- [ ] #55453 <!-- Privatise the annotations API, for StyledStrings -->
- [ ] #55355 <!-- relocation: account for trailing path separator in
depot paths -->
- [ ] #55220 <!-- `isfile_casesensitive` fixes on Windows -->
- [ ] #55169 <!-- `propertynames` for SVD respects private argument -->
- [ ] #54457 <!-- Make `String(::Memory)` copy -->
- [ ] #53957 <!-- tweak how filtering is done for what packages should
be precompiled -->
- [ ] #51479 <!-- prevent code loading from lookin in the versioned
environment when building Julia -->
- [ ] #50813 <!-- More doctests for Sockets and capitalization fix -->
- [ ] #50157 <!-- improve docs for `@inbounds` and
`Base.@propagate_inbounds` -->
- [ ] #41244 <!-- Fix shell `cd` error when working dir has been deleted
-->
@KristofferC KristofferC removed the backport 1.11 Change should be backported to release-1.11 label Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arrays [a, r, r, a, y, s] performance Must go faster
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants