Repository navigation
Drastically reduce selinv allocations and match Cholesky performance - #10
Merged
Merged
Conversation
- Lift transposed_chunks, symmetric_access, depermuted_access from
runtime Bool fields to type parameters on SupernodalMatrix{Tr,Sym,Dep}.
This eliminates Union return types from get_chunk/get_split_chunk,
enabling the compiler to specialize all code paths.
- Replace allocating partition_Sj with partition_Sj! that writes into
a pre-allocated buffer. The old version allocated a new Vector plus
per-block arrays on every supernode.
- Fix get_Sj to return a view instead of a copy.
- Use clear_indmap! (O(|Sj|)) instead of fill!(indmap, 0) (O(N)).
Reduces selinv time on 50k 2D Laplacian from 101ms to 78ms (23%).
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Rewrite gather_update_matrix! to use direct Z.vals index arithmetic instead of get_chunk/SubArray views (eliminates ~38k allocs) - Fix indmap[block] indexing: use indmap[first(block)] instead of indmap[block][1] which copied the entire range (~370k allocs eliminated) - Set GATHER_MIN_BLOCKS=1 (always gather) since the zero-alloc gather no longer has overhead that justified the scattered path On 50k 2D Laplacian: 414k allocs → 44k allocs, 66ms → 57ms. On ecology2 (1M nodes): ~12s → 2.3s (5x speedup from reduced GC pressure). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Remove nothing return from get_split_chunk (return empty SubArray
for root instead), eliminating Union{Nothing, SubArray} return type
- Inline LL_to_LDL! in selinv_supernodal with direct chunk access,
avoiding type-unstable get_split_chunk calls
On 50k 2D Laplacian: 44k → 38k allocations.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Replace per-chunk copy+transpose+copyback in _transpose_chunks! with single pre-allocated buffer, eliminating ~6k temporary allocations - Revert fused LL_to_LDL (forward pass has better cache locality) On 50k 2D Laplacian: 38k → 19k allocations, 57ms → 52ms. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Move per-supernode logic into a separate @inline function to help the compiler's escape analysis with SubArray views, reducing BLAS dispatch overhead. On 50k 2D Laplacian: 57ms → 52ms, 38k → 18.5k allocations. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Eliminate all SubArray/ReshapedArray allocations from the selinv main loop by calling BLAS/LAPACK routines directly via ccall with computed pointers and leading dimensions. No reshape, no views, no heap allocations in the hot path. Results on 50k 2D Laplacian: 52ms → 47ms, 18.5k → 6.3k allocations. Results on SuiteSparse ecology2 (1M): ~12s → 1.3s (10x speedup). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add _partition_Sj_direct! that works directly with Z.rows array using offset arithmetic, avoiding the SubArray allocation from get_Sj. On 50k 2D Laplacian: 47ms → 44.5ms. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Dead code since gather is now always used with direct BLAS ccalls. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Summary
Systematic elimination of allocations and overhead in
selinv_supernodal, bringing selected inversion time down to roughly match CHOLMOD Cholesky factorization time.Key changes:
transposed_chunks,symmetric_access,depermuted_accessfrom runtimeBoolfields to type parameters onSupernodalMatrix{Tr,Sym,Dep}, eliminating Union return types and enabling compile-time specializationccallusing pointer arithmetic — noreshape, noSubArray, no heap allocations in the hot path_partition_Sj_direct!that works directly withZ.rowsoffsetsgather_update_matrix!using directZ.valsindex arithmetic_transpose_chunks!with single pre-allocated bufferBenchmarks
Allocations on a 50k 2D Laplacian: 490k → 12k.
Test plan
🤖 Generated with Claude Code