(perf): GPU parity — depth-tagged touched-others stack, PoolCheckpointState, lookup memo (CUDA/Metal)#52
Merged
Merged
Conversation
Mirrors the CPU _poison_fill! contract: custom isbits element types without zero(T) skip the poison pass instead of throwing MethodError mid-rewind. Surfaced by the touched-others struct fallback types under runtime_check=1.
…nothing At S=1, rewind's invalidation poisons released slots and compact! deliberately defers their shrink to preserve the poison (see _maybe_compact_slot!) — the capacity asserts now encode both semantics. Blocks also stop returning pool-backed arrays (PoolEscapeError under runtime_check=1).
Same S=1 hygiene as the auto-manage tests: a @with_pool block whose last expression is a pool-backed array throws PoolEscapeError under runtime_check=1. All blocks now end with nothing; the @allocated / pointer comparisons are unaffected (they never used the scope return value). NOT run locally per user request — verify in the next full suite run.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #52 +/- ##
==========================================
- Coverage 95.83% 95.81% -0.03%
==========================================
Files 16 16
Lines 3677 3678 +1
==========================================
Hits 3524 3524
- Misses 153 154 +1
🚀 New features to boost your workflow:
|
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.
Problem
The GPU extensions still ran the pre-#50 design:
_lazy_checkpoint!eagerly snapshotted every registered fallback type at scope entry (O(registered types) — the pollution bug #50 fixed on CPU), lazy rewinds iterated the wholevalues(pool.others)dict, and every fallback acquire paid an IdDict lookup. #51's reorder deferred GPU parity so both backends could mirror the final design once.Fix
Mirror #51's architecture in both extensions, sharing one implementation where possible:
_touch_fallback_pool!/_drain_touched_others!/_truncate_touched_others!generalized overAbstractArrayPool(+_check_level); CPU, Metal, and CUDA now run the same code. Compile-time_runtime_checkgating keeps the S=0 DCE contract (verified via@code_typed: zero dynamic dispatch, branch fully folded).PoolCheckpointStateon GPU typed pools —MetalTypedPool/CuTypedPoolnest their checkpoint fields into the same concrete struct as CPU (property forwarding keeps all call sites source-compatible), so the S=0 drain runs zero-dispatch onVector{PoolCheckpointState}.(state, depth)entry per first touch; lazy rewinds drain by depth tag; full checkpoint/rewind keep their eager-sweep + truncate pairing.get_typed_pool!memoizes the last fallback lookup.Float16 caveat (GPU-only divergence): Float16 is a fixed slot with
_fixed_slot_bit == 0(bit-7 reassignment), so stack routing uses!(T <: _*_FIXED_TYPES)— never the bit test. Routing it onto the stack would double-rewind against the lazy rewinds' Float16 special case and corrupt parent-scope counts (caught in plan review; regression testsets guard it on both backends).Also: dead
_make_cuda_pool(level, old)/_transfer_cuda_poolremoved (zero callers; same rationale as #51's removal — and its positional constructor would no longer compile). Surfaced along the way and fixed: GPU poison-fill now mirrors the CPU best-effort contract (never throws during rewind onzero-less isbits types), and GPU/auto-manage tests areruntime_check=1-clean (S-adaptive asserts; scopes never return pool-backed arrays).Benchmarks
Metal (Apple Silicon, local), lazy
@with_pool :metalscope acquiring one fallback type — same method as #50/#51:Same-process registry-size-isolated sweep is the signal (a fresh-process JIT ordering artifact adds a ~50 ns constant offset unrelated to registry size — details in the branch notes). Zero-dispatch drain confirmed on the S=0 path.
Testing
test/metal/test_touched_others.jl+ CUDA mirror (~850 lines): pollution regression,f_outerexception-leak recovery, depth-tag exactness/monotonicity, memo lifecycle (empty!clears /reset!preserves), Float16 non-interaction (lazy, typed, mixed), R=1 three-stack lockstep (happy path, exception unwind, nested depths), mixed fixed+fallback tuples.Follow-ups (tracked)
Foo{Float64}static widening, per-scopetphoisting, and an expansion-time escape lint for the two syntactic footguns (direct acquire /x .= vtails).