Merged
Conversation
Eliminate all remaining allocations in the S=1 hot path when non-fixed-slot types (pool.others) are used. Fixes 6 allocation sources: 1. sizeof(eltype) crash on Julia 1.10 for non-isbits types → _safe_elsize 2. IdDict ValueIterator from values(pool.others) → _others_values Vector cache 3. _wrapper_prod_size boxing on wrapper::Any → pointer-first, defer to mismatch 4. Any-typed TypedPool access in overlap check → pre-collected _others_ptr_bounds 5. _check_all_slots unconditional others iteration → _touched_has_others guard 6. get_typed_pool! closure allocation → get + manual insert All new fields and operations guarded by _runtime_check(pool) / S>=1 for complete DCE at S=0. Legacy (Julia ≤1.10) and CUDA/Metal extensions synced.
Pattern 6: Large 1D array (2000 elements) — exercises zero-alloc _check_wrapper_mutation! with pointer-first check. Pattern 7: Large N-D array (4×21×21) — exercises wrapper mutation check on multi-dimensional arrays. These were present in fix/borrow_registry but missing from the clean reimplementation. Renumber existing others patterns to 8 and 9.
… types
sizeof(T) crashes on Julia 1.10 when T is a non-isbits type like
Vector{Float64} (Array is an opaque C-backed type without definite size).
Use inline isbitstype check consistent with _safe_elsize in debug.jl,
avoiding cross-file dependency on include order.
The fast path in _check_others_pointer_overlap was checking ALL entries in _others_ptr_bounds, including bounds from outer scopes. This caused false positives: returning an array acquired in an outer scope from an inner scope would incorrectly trigger PoolRuntimeEscapeError. Fix: use _others_ptr_bounds_checkpoints[end] as scope boundary, only checking bounds recorded after the current scope's checkpoint — matching the _scope_boundary pattern used by _check_tp_pointer_overlap for fixed-slot types.
- others type: outer-scope array returned from inner scope is NOT an escape - others type: inner-scope array returned from inner scope IS an escape - fixed slot: same tests for parity verification - Pattern 10: nested others + cross-scope validate is zero-alloc (S=1)
Codecov Report❌ Patch coverage is
❌ Your patch status has failed because the patch coverage (93.06%) 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 #39 +/- ##
==========================================
- Coverage 96.23% 96.03% -0.20%
==========================================
Files 14 14
Lines 3107 3232 +125
==========================================
+ Hits 2990 3104 +114
- Misses 117 128 +11
🚀 New features to boost your workflow:
|
…rance for Julia 1.11
- acquire_view! now calls _maybe_record_others_bounds! on the backing
vector so the S=1 fast-path escape checker detects views of pool arrays
- Legacy path gains missing _maybe_record_borrow! calls for parity
- S=1 zero-alloc tests allow ≤32 bytes/iter on Julia <1.12 due to
ccall overhead in _check_wrapper_mutation! through Vector{Any}
- New test coverage for acquire_view! escape detection (others + fixed)
There was a problem hiding this comment.
Pull request overview
This PR aims to make RUNTIME_CHECK=1 escape/mutation checking remain zero-allocation by caching “others” TypedPool iteration, pre-recording pointer bounds for overlap checks, and expanding test coverage around runtime checking behavior (including nested scopes and “others” types).
Changes:
- Add “others” iteration/value caching and per-scope pointer-bounds checkpointing to support zero-allocation runtime escape checks.
- Record others-type pointer bounds during
acquire!(and legacyacquire!) to enable a bounds-based overlap fast path. - Expand/adjust tests for zero-allocation runtime checking and structural mutation detection behavior.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
test/test_zero_allocation.jl |
Adds several new S=1 zero-allocation patterns, including “others” + _validate_pool_return exercises. |
test/test_runtime_mutation.jl |
Updates runtime mutation tests to match pointer-based detection behavior (and removal of the old size-inflation warning). |
test/test_debug.jl |
Adds runtime escape validation tests for non-isbits eltypes and cross-scope behavior for fixed-slot vs others types. |
src/types.jl |
Adds new pool fields for others caching and pointer-bounds tracking; updates get_typed_pool!; defines _maybe_record_others_bounds!. |
src/state.jl |
Switches others iteration to _others_values and adds checkpoint/rewind management for _others_ptr_bounds(_checkpoints). |
src/legacy/types.jl |
Mirrors new pool fields and get_typed_pool! changes for legacy pools (but currently missing _maybe_record_others_bounds!). |
src/legacy/state.jl |
Mirrors the state changes for _others_values iteration and pointer-bounds checkpoint/rewind in legacy. |
src/legacy/acquire.jl |
Adds _maybe_record_others_bounds! calls in legacy _acquire_impl! (but currently calls an undefined function and still lacks borrow recording). |
src/debug.jl |
Introduces others-bounds fast path and _safe_elsize; adjusts wrapper mutation checks to be pointer-first and removes the old size-growth check. |
src/acquire.jl |
Records others-type bounds during _acquire_impl! for the non-legacy path (but acquire_view! path is still missing bounds recording). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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
Addresses a false-negative escape detection bug, missing legacy definitions, and
adds test coverage for
acquire_view!withRUNTIME_CHECK=1.Changes
Bug fixes
acquire_view!escape detection (critical):_acquire_view_impl!was notcalling
_maybe_record_others_bounds!, so views of non-fixed-slot types wereinvisible to the pre-collected bounds fast path at S=1. Mixed
acquire!+acquire_view!for different fallback types could silently miss escape detection.Fixed in both
src/acquire.jlandsrc/legacy/acquire.jl.Missing legacy
_maybe_record_others_bounds!: The function was defined insrc/types.jl(Julia 1.11+) but not insrc/legacy/types.jl, causingUndefVarErroron Julia 1.10.Missing legacy
_maybe_record_borrow!: Legacy_acquire_impl!lacked_maybe_record_borrow!(pool, tp)calls, so escape error messages on Julia 1.10had no callsite information.