(refac): Slot-first architecture: remove view cache, isolate legacy#23
Merged
(refac): Slot-first architecture: remove view cache, isolate legacy#23
Conversation
…ay/view paths Remove `views` and `view_lengths` fields from `TypedPool` and unify slot management through `_claim_slot!` as the shared primitive for all acquisition paths (`get_view!`, `get_nd_array!`, `get_nd_view!`). Key changes: - `get_view!` always creates a fresh `SubArray` (stack-allocated via SROA, benchmarked faster than cached return due to no memory indirection) - `get_nd_array!` uses `_claim_slot!` directly instead of routing through `get_view!`, with `setfield!` pattern for cache miss (replaces `unsafe_wrap`) - `get_nd_view!` uses `_claim_slot!` directly instead of `get_view!` - `_claim_slot!(tp, n)` overload handles memory provisioning; zero-arg variant retained for `reshape!` wrapper-only slots No public API changes. Checkpoint/rewind unaffected (only uses `n_active`). CUDA extension and legacy path unaffected.
…t! resize coverage - Remove `wrap_array` from src/acquire.jl (dead code on Julia 1.11+; legacy retains it) - Remove CUDA `wrap_array` override and import from dispatch.jl (also dead code) - Remove `wrap_array` testset from CUDA extension tests - Remove tautological `>= 1` assertion in test_state.jl (already covered by `>= 3`) - Add "Slot reuse with resize via _claim_slot!" testset: grow, shrink, slot independence
…ields - get_nd_array! → get_array! - get_nd_view! → consolidated into get_view! (N-D overload) - nd_wrappers → arr_wrappers (struct field) - _store_nd_wrapper! → _store_arr_wrapper!
_claim_slot! is only defined for TypedPool{T}, so the AbstractTypedPool
signature was wider than the actual contract. Other subtypes override
get_view! directly (CuTypedPool) or use a separate path (BitTypedPool
→ get_bitarray!).
Move all BitTypedPool-specific methods from scattered locations into the version-specific bitarray.jl files (modern + legacy): - empty!(::BitTypedPool) from state.jl - _check_bitchunks_overlap from utils.jl - Display helpers (_default_type_name, _vector_bytes, _count_label, _show_type_name) from utils.jl Struct definitions (Bit, BitTypedPool) remain in types.jl as required by AdaptiveArrayPool's field declaration. Pure code movement, no logic changes.
- Remove CACHE_WAYS, set_cache_ways!, and `using Preferences` from src/types.jl — these are unused on Julia 1.11+ (setfield!-based wrapper cache replaced N-way set-associative cache) - Move `export CACHE_WAYS, set_cache_ways!` into the legacy branch of @static if in AdaptiveArrayPools.jl - Add forward-reference comment in utils.jl for _check_bitchunks_overlap (defined in bitarray.jl, included after utils.jl) - Remove corresponding tests from test_nway_cache.jl and test_coverage.jl (legacy tests remain in test/legacy/test_nway_cache.jl)
Codecov Report❌ Patch coverage is
❌ Your patch status has failed because the patch coverage (94.84%) is below the target coverage (95.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## master #23 +/- ##
==========================================
- Coverage 95.82% 95.71% -0.11%
==========================================
Files 13 13
Lines 1820 1820
==========================================
- Hits 1744 1742 -2
- Misses 76 78 +2
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR refactors the Julia ≥1.11 implementation to a slot-first acquisition architecture (centered on _claim_slot!), removes the old 1D view cache, renames nd_wrappers → arr_wrappers, and relocates legacy (≤1.10) behavior under src/legacy/. It also consolidates BitTypedPool-related helpers into the bitarray.jl concept files.
Changes:
- Replace
TypedPool’s cached 1D view machinery with slot-first_claim_slot!, and create freshSubArrayviews per call. - Rename and standardize the N-D wrapper cache as
arr_wrappersand update associated tests/utilities. - Move legacy-only APIs (
CACHE_WAYS,set_cache_ways!) and legacy N-way cache logic undersrc/legacy/, adjusting tests accordingly.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/AdaptiveArrayPools.jl |
Moves CACHE_WAYS/set_cache_ways! export into the legacy branch; adjusts include layout accordingly. |
src/types.jl |
Removes views/view_lengths, renames wrapper cache to arr_wrappers, and updates design notes for ≥1.11. |
src/acquire.jl |
Introduces slot-first _claim_slot!, makes get_view! always produce fresh views, and rewrites get_array! around arr_wrappers. |
src/bitarray.jl |
Updates BitTypedPool wrapper caching to arr_wrappers and relocates BitTypedPool helpers (empty!/debug/display) here. |
src/utils.jl |
Removes BitTypedPool helper methods moved into bitarray.jl; keeps debug validation hooks. |
src/state.jl |
Updates empty!(::TypedPool) to clear arr_wrappers (no more views cache); updates reset! docs accordingly. |
src/legacy/acquire.jl |
Renames legacy get_nd_array! → get_array! while keeping N-way cache behavior for ≤1.10. |
src/legacy/bitarray.jl |
Moves legacy BitTypedPool empty!, debug overlap checks, and display helpers into this concept file. |
src/legacy/state.jl |
Removes legacy BitTypedPool empty! (now in legacy/bitarray.jl). |
ext/AdaptiveArrayPoolsCUDAExt/dispatch.jl |
Removes now-deleted wrap_array dependency from CUDA dispatch imports. |
ext/AdaptiveArrayPoolsCUDAExt/acquire.jl |
Switches CUDA unsafe path to get_array! and removes get_nd_* references. |
test/runtests.jl |
Updates version-specific helper to check arr_wrappers on ≥1.11. |
test/test_basic.jl |
Adds coverage for resize/reuse behavior via _claim_slot!. |
test/test_state.jl |
Removes assertions tied to removed 1D view caches. |
test/test_nway_cache.jl |
Removes legacy-only CACHE_WAYS configuration tests; renames nd_wrappers tests to arr_wrappers. |
test/test_coverage.jl |
Moves set_cache_ways! validation coverage to legacy-only test suite. |
test/cuda/test_extension.jl |
Removes wrap_array test now that API is deleted. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
CPU ≥1.11 no longer uses N-way view caching (slot-first _claim_slot!), so CACHE_WAYS is now defined locally in the CUDA extension via @load_preference. GPU still benefits from caching (~80 bytes per view/reshape on CPU heap). Fixes UndefVarError on extension load.
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.
Summary
Replace the
views/view_lengthscache inTypedPoolwith a slot-first architecturebuilt on
_claim_slot!, and isolate legacy (Julia ≤1.10) code underlegacy/.SubArrayis stack-allocated via SROA in Julia 1.11+ — cachingviews was slower than creating fresh ones due to memory indirection overhead.
_claim_slot!(tp, n)is now the single primitive for allacquisition paths (
get_view!,get_array!), eliminating duplicated resize/growth logic.get_array!uses_claim_slot!directly instead ofrouting through
get_view!, removing an unnecessary intermediateSubArray.empty!,_check_bitchunks_overlap, and display helpersmoved from
state.jl/utils.jlintobitarray.jl(one file per concept).CACHE_WAYS/set_cache_ways!removed from the 1.11+ path;only exported on Julia ≤1.10 where the N-way cache is actually used.
Breaking changes
None for public API. Internal functions renamed:
get_nd_array!->get_array!get_nd_view!-> removed (merged intoget_view!)wrap_array-> removednd_wrappersfield ->arr_wrappers