Skip the row cache in sorted getindex when few columns are selected - #870
Open
ViralBShah wants to merge 3 commits into
Open
ViralBShah wants to merge 3 commits into
ViralBShah wants to merge 3 commits into
Conversation
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #870 +/- ##
==========================================
- Coverage 93.01% 92.06% -0.96%
==========================================
Files 12 12
Lines 9076 9148 +72
==========================================
- Hits 8442 8422 -20
- Misses 634 726 +92 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…regimes Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Member
Author
|
@andreasnoack This is a really old one. Please do chime in (even after merging) with thoughts if any. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #12.
A[I, J]with a sorted vectorIallocates and sweeps a cache of lengthsize(A, 1)ingetindex_I_sorted_bsearch_Iandgetindex_I_sorted_linear, however little is requested, so a loop over columns costs O(n·m) instead of O(nnz). JuliaLang/julia#42647 addressed the linear kernel but was closed unmerged when the stdlib moved.The cache is what makes these kernels fast when many columns are selected (each distinct row is located in
Ionce rather than once per stored entry; removing it outright is 10x+ slower forS[I, :]), so it stays. This addsgetindex_I_sorted_nocache, a two-pass count-and-fill kernel with no scratch space that locates each stored row inIby galloping from the previous hit (short brackets finish with a linear scan), so a search costs the log of the distance moved rather than oflength(I)and one kernel serves both the bsearch and the linear regime.getindex_I_sortedpicks it when32 * nnzJ * log2(gap) < m, withgapthe length ofIper stored entry of an average selected column;nnzJis read offcolptrin O(nJ). The factor is set at the cache's best case, every selected column storing the same rows withI = 1:mfor m up to 5e7, where one search per distinct row is reused by all columns: just under the gate the cacheless kernel takes 0.4–0.55x the cached time there and 0.03–0.8x onsprand, so it is not slower than the current code on any input I could construct. It builds the same result with no scratch, so memory never goes up. On a 1000x1000 9-point Laplacian, 5-point and 3D 7-point Laplacians, tridiagonal, banded, arrowhead and block-diagonal matrices (single columns, red-black splits, subdomain blocks, random subsets), every call above 1 ms keeps its current kernel and timing, and few-column calls drop from 0.07–0.9 ms to microseconds. The algorithm choice among the three existing kernels is unchanged.A[[1], 1]A[collect(1:258), 1]A[collect(1:10^6), collect(1:1000)]A[I, j]forj = 1:1000S[1:2:end, [7]](vectorI)S[1:2:end, :](vectorI, cached path)A[R, R[k:k+19]], R random N/10A[r, r], r random N/1000Tests: the new kernel runs in the existing
test_getindex_algsgrid against dense (stored zeros, duplicates and unsortedI/J), plus an allocation bound on a 10^6-row matrix showing scalar, single and few-columnJreach it in both regimes. Full suite passes on nightly.Not addressed here: when
m > nnz(A)the dispatcher picksgetindex_I_sorted_bsearch_A, which walksIfor every column; withnI = 5e5and 1e4 columns of a hypersparse 1e6 matrix that is 1.96 s against 0.8 ms for the new kernel. That branch is unchanged and is a follow-up. No dispatch or API change; performance only, so no backport.Written by Claude Code (Claude Fable 5.1).
🤖 Generated with Claude Code