perf(gc): the loop back-edge poll's no-work path becomes one global load (#7721) - #7735
Conversation
c271815 to
0739c2f
Compare
📝 WalkthroughWalkthroughThe runtime adds a process-global poll-arm counter. Generated GC loop safepoints load this counter and call the runtime only when polling is armed. Safepoint state transitions, pressure handling, Zeal, code generation, tests, documentation, and version metadata now use the coordinated poll state. ChangesGC loop poll arming
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant GeneratedLoop
participant PERRY_GC_POLL_ARMED
participant js_gc_loop_safepoint
participant js_gc_loop_safepoint_armed
GeneratedLoop->>PERRY_GC_POLL_ARMED: volatile load
alt Poll is armed
GeneratedLoop->>js_gc_loop_safepoint: invoke
js_gc_loop_safepoint->>js_gc_loop_safepoint_armed: resolve and process poll
else Poll is unarmed
GeneratedLoop-->>GeneratedLoop: continue loop
end
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/perry-codegen/src/stmt/loops.rs`:
- Around line 5451-5459: Update the back-edge poll in loops.rs to load
`@PERRY_GC_POLL_ARMED` atomically with monotonic ordering and 4-byte alignment
before branching to js_gc_loop_safepoint; replace the current volatile-only
load. In loop_safepoint_purity.rs lines 442-452, update assertions to require
the atomic load form and the expected per-poll occurrence count.
In `@crates/perry-runtime/src/gc/poll_arm.rs`:
- Around line 130-141: Serialize every test that modifies PERRY_GC_POLL_ARMED
with the process-global GcTestIsolationGuard, including the tests using
Restore::capture. Acquire the guard before changing the counter and hold it
through Restore’s drop, rather than introducing a local mutex, so these tests
cannot overlap with other GC tests or leave a pending safepoint inconsistent.
In `@crates/perry-runtime/src/gc/tests/triggers.rs`:
- Around line 758-770: Update
zeal_holds_the_poll_word_armed_with_nothing_pending to capture
PERRY_GC_POLL_ARMED’s baseline before creating ZealGuard, then assert after the
guard scope ends that the counter equals that baseline. Remove the misleading
post-scope set_safepoint_pending(false) call, since it can return without
validating ZealGuard::drop released its arm.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 98b5f0bc-ef4f-4650-a547-eabb5cb4cbc5
📒 Files selected for processing (13)
changelog.d/7735-gc-loop-poll-arming-word.mdcrates/perry-codegen/src/runtime_decls/arrays.rscrates/perry-codegen/src/stmt/loops.rscrates/perry-codegen/tests/loop_safepoint_purity.rscrates/perry-runtime/src/gc/mod.rscrates/perry-runtime/src/gc/policy.rscrates/perry-runtime/src/gc/poll_arm.rscrates/perry-runtime/src/gc/pressure.rscrates/perry-runtime/src/gc/tests/global_bootstrap.rscrates/perry-runtime/src/gc/tests/runtime_roots/generator_attach_prototype.rscrates/perry-runtime/src/gc/tests/scan_fallback.rscrates/perry-runtime/src/gc/tests/triggers.rscrates/perry-runtime/src/gc/zeal.rs
| let armed = blk.load_volatile(I32, "@PERRY_GC_POLL_ARMED"); | ||
| let due = blk.icmp_ne(I32, &armed, "0"); | ||
| blk.cond_br(&due, &poll_label, &done_label); | ||
| } | ||
| ctx.current_block = poll_idx; | ||
| { | ||
| let blk = ctx.block(); | ||
| blk.call_void("js_gc_loop_safepoint", &[]); | ||
| blk.br(&done_label); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the IR instruction and printer definitions before inspecting load support.
ast-grep outline crates/perry-codegen/src/block.rs --items all
# Find the load instruction representation, its text emitter, and existing
# atomic-load support. Expect an atomic load form with explicit ordering/alignment.
rg -n -C 5 'load_volatile|LoadFlavor|LlInst::Load|enum LlInst|atomic.*load|load.*atomic' \
crates/perry-codegen/src
# Confirm every use and IR assertion for the poll-arm global is updated together.
rg -n -C 4 'PERRY_GC_POLL_ARMED|load volatile i32|load atomic' \
crates/perry-codegen/src crates/perry-codegen/tests crates/perry-runtime/src/gcRepository: PerryTS/perry
Length of output: 50369
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the runtime definition and writer path for the poll-arm global.
sed -n '50,90p' crates/perry-runtime/src/gc/poll_arm.rs
sed -n '870,895p' crates/perry-codegen/src/stmt/loops.rs
sed -n '2220,2320p' crates/perry-runtime/src/gc/policy.rs
# Inspect the corresponding module/global declaration and any volatile/raw uses.
rg -n -C 3 'PERRY_GC_POLL_ARMED|atomic_store|store_release|store_relaxed|load_volatile|store_volatile|load.*atomic' crates/perry-runtime/src crates/perry-codegen/src crates/perry-codegen/tests
# Inspect the declaration file section if present.
rg -n -C 5 'arrays|PARRY|external global|declare global|PERRY_GC_POLL_ARMED' crates/perry-codegen/src/runtime_decls -SRepository: PerryTS/perry
Length of output: 50369
🌐 Web query:
LLVM LangRef load volatile atomic semantics synchronization
💡 Result:
In the LLVM IR, the volatile and atomic semantics for memory operations are distinct and orthogonal concepts, each serving specific purposes [1][2]. 1. Volatile Semantics The volatile attribute (as in C/C++ volatile) ensures that every load or store is performed exactly as written in the program [1][3]. The compiler is strictly prohibited from adding, removing, or reordering volatile accesses [4]. Volatile operations are not atomic and do not provide inter-thread synchronization; they are primarily intended for interacting with memory-mapped I/O or hardware devices where the access itself is the desired side effect [1][4]. 2. Atomic Semantics Atomic load and store instructions provide guarantees for concurrent access by multiple threads [1][5]. They utilize specific memory ordering constraints (e.g., monotonic, acquire, release, sequentially consistent) to define how memory operations synchronize across threads [1][3]. These operations can participate in the happens-before relationship defined by the LLVM memory model [6]. 3. Combining Volatile and Atomic While the LLVM IR permits instructions to be marked as both volatile and atomic, this is rarely used and has specific implications [4][5]: - Orthogonality: Because they are orthogonal, a load marked as both atomic and volatile will maintain the strict access guarantees of volatile (it will not be eliminated or reordered) while also satisfying the atomic memory ordering constraints specified for it [1][4]. - Use Case: The primary justification for combining these is for specialized communication with hardware (e.g., a DMA engine), where a thread needs to perform a synchronizing write (atomic release) that must also be guaranteed to occur without compiler interference (volatile) [4]. - Synchronization: Volatile operations do not inherently provide synchronization; synchronization is solely the responsibility of the atomic ordering [1]. Using volatile to attempt inter-thread synchronization is considered a poor practice compared to using atomic operations [4]. In summary, volatile controls the visibility and persistence of an access to the compiler, whereas atomics control the synchronization and ordering of the memory operation in the presence of concurrency [1][4]. Using both ensures the access occurs as explicitly requested while also participating in the memory model's synchronization guarantees [4].
Citations:
- 1: https://llvm.org/docs/Atomics.html
- 2: https://releases.llvm.org/22.1.0/docs/Atomics.html
- 3: https://github.com/llvm/llvm-project/blob/main/llvm/docs/Atomics.rst
- 4: https://discourse.llvm.org/t/meaning-of-loads-stores-marked-both-atomic-and-volatile/46906
- 5: https://llvm.org/docs/doxygen/classllvm_1_1LoadInst.html
- 6: https://llvm.org/docs/AMDGPUMemoryModel.html
🌐 Web query:
LLVM VolatileIntrinsic atomic instruction semantics volatile load atomic order
💡 Result:
In LLVM IR, volatile and atomic memory access semantics are orthogonal, meaning they serve distinct purposes and are governed by different rules regarding compiler optimization and instruction reordering [1][2]. Volatile semantics are primarily designed for memory-mapped I/O (MMIO) and signal handlers [1][3]. When an instruction is marked volatile, the compiler is prohibited from adding, removing, or reordering these accesses [1][3]. Every volatile load and store must be performed exactly as specified in the IR [1][2]. Importantly, volatile does not imply atomicity, and non-atomic volatile operations should not be used as a substitute for atomic operations when thread synchronization is required [1][4]. Atomic instructions (load atomic, store atomic) provide specific memory ordering guarantees necessary for thread synchronization, such as Monotonic, Acquire, Release, or SequentiallyConsistent ordering [1][2]. Unlike volatile accesses, atomic operations permit certain compiler optimizations—such as reordering or elision—provided those optimizations do not violate the specified memory consistency model [1][3]. When an instruction is marked as both atomic and volatile, it must adhere to both sets of constraints [3]. The volatile requirement ensures the access is not elided or reordered by the compiler, while the atomic requirement ensures that the operation interacts correctly with the memory model and other threads [3]. While rare, this combination is sometimes used in specialized scenarios, such as communicating with hardware (e.g., triggering a DMA engine via a volatile release store) where both external visibility and specific memory ordering are required [3]. In summary, they are distinct concepts that can be combined: volatile governs the compiler's ability to manipulate the instruction stream (preventing optimizations), while atomic governs how the memory operation interacts with other threads in the context of a memory consistency model [1][3].
Citations:
- 1: https://llvm.org/docs/Atomics.html
- 2: https://github.com/llvm/llvm-project/blob/main/llvm/docs/Atomics.rst
- 3: https://discourse.llvm.org/t/meaning-of-loads-stores-marked-both-atomic-and-volatile/46906
- 4: https://releases.llvm.org/22.1.0/docs/Atomics.html
🌐 Web query:
LLVM load volatile i1 order Monotonic semantics
💡 Result:
In LLVM IR, atomic operations and volatile qualifiers are orthogonal concepts, and they can be combined on a single instruction [1][2]. When you specify an atomic load with Monotonic ordering and the volatile qualifier (e.g., load atomic volatile i1 %ptr, monotonic), the operation combines the semantics of both [1][3]: 1. Atomic Monotonic Semantics: The load provides the atomicity guarantees defined by the Monotonic memory order. Monotonic is the weakest level of atomicity in LLVM, ensuring that if you observe all operations affecting a specific address, a consistent total ordering exists for those operations [1][2]. It does not provide general synchronization (like Acquire or Release) on its own, but it ensures that the operation is atomic and prevents certain compiler optimizations, such as unrestricted store-forwarding within a thread [1][3]. 2. Volatile Semantics: The volatile qualifier dictates that the load must be performed exactly as specified in the IR [1][2]. The compiler is prohibited from adding, removing, or reordering volatile accesses [3]. This is critical for memory-mapped I/O (MMIO) and similar scenarios where memory access itself has side effects that the compiler must not optimize away [3]. By combining them, you get an operation that is guaranteed to be performed exactly as described (volatile) while also adhering to the atomic visibility and consistency constraints of the Monotonic memory ordering [1][3]. This is distinct from a simple non-atomic volatile load, which does not provide the same atomic guarantees [4][5]. [1][3][2]
Citations:
- 1: https://llvm.org/docs/Atomics.html
- 2: https://releases.llvm.org/19.1.0/docs/Atomics.html
- 3: https://discourse.llvm.org/t/meaning-of-loads-stores-marked-both-atomic-and-volatile/46906
- 4: https://releases.llvm.org/14.0.0/docs/Atomics.html
- 5: https://releases.llvm.org/18.1.0/docs/Atomics.html
Use an atomic load for PERRY_GC_POLL_ARMED.
PERRY_GC_POLL_ARMED is a process-global AtomicU32, and LLVM volatile loads do not provide atomic memory-read semantics. A concurrent arm can be missed, so the back-edge poll can skip js_gc_loop_safepoint and leave deferred collection behind. Emit load atomic i32, ptr @PERRY_GC_POLL_ARMED monotonic, align 4, and update crates/perry-codegen/tests/loop_safepoint_purity.rs to require atomic form ordering and per-poll count.
📍 Affects 2 files
crates/perry-codegen/src/stmt/loops.rs#L5451-L5459(this comment)crates/perry-codegen/tests/loop_safepoint_purity.rs#L442-L452
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/perry-codegen/src/stmt/loops.rs` around lines 5451 - 5459, Update the
back-edge poll in loops.rs to load `@PERRY_GC_POLL_ARMED` atomically with
monotonic ordering and 4-byte alignment before branching to
js_gc_loop_safepoint; replace the current volatile-only load. In
loop_safepoint_purity.rs lines 442-452, update assertions to require the atomic
load form and the expected per-poll occurrence count.
| fn zeal_holds_the_poll_word_armed_with_nothing_pending() { | ||
| let _isolation = GcTestIsolationGuard::new(); | ||
| crate::gc::set_safepoint_pending(false); | ||
| { | ||
| let _zeal = super::super::zeal::ZealGuard::set(true); | ||
| assert!( | ||
| crate::gc::PERRY_GC_POLL_ARMED.load(std::sync::atomic::Ordering::Relaxed) > 0, | ||
| "zeal must keep the poll reachable even with no deferral outstanding" | ||
| ); | ||
| } | ||
| // And it gives the arm back, so one zeal test does not leave every later | ||
| // test in this binary paying for the slow path. | ||
| crate::gc::set_safepoint_pending(false); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert that ZealGuard releases its arm.
Save the counter baseline before creating the guard. Assert that the counter equals the baseline after the guard drops.
Line 770 calls set_safepoint_pending(false). It returns early when the flag is already false. The test therefore passes if ZealGuard::drop leaks its poll arm.
Proposed test change
crate::gc::set_safepoint_pending(false);
+let base = crate::gc::PERRY_GC_POLL_ARMED.load(std::sync::atomic::Ordering::Relaxed);
{
let _zeal = super::super::zeal::ZealGuard::set(true);
assert!(/* unchanged */);
}
-// And it gives the arm back, so one zeal test does not leave every later
-// test in this binary paying for the slow path.
-crate::gc::set_safepoint_pending(false);
+assert_eq!(
+ crate::gc::PERRY_GC_POLL_ARMED.load(std::sync::atomic::Ordering::Relaxed),
+ base,
+ "dropping zeal must return its poll arm"
+);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fn zeal_holds_the_poll_word_armed_with_nothing_pending() { | |
| let _isolation = GcTestIsolationGuard::new(); | |
| crate::gc::set_safepoint_pending(false); | |
| { | |
| let _zeal = super::super::zeal::ZealGuard::set(true); | |
| assert!( | |
| crate::gc::PERRY_GC_POLL_ARMED.load(std::sync::atomic::Ordering::Relaxed) > 0, | |
| "zeal must keep the poll reachable even with no deferral outstanding" | |
| ); | |
| } | |
| // And it gives the arm back, so one zeal test does not leave every later | |
| // test in this binary paying for the slow path. | |
| crate::gc::set_safepoint_pending(false); | |
| fn zeal_holds_the_poll_word_armed_with_nothing_pending() { | |
| let _isolation = GcTestIsolationGuard::new(); | |
| crate::gc::set_safepoint_pending(false); | |
| let base = crate::gc::PERRY_GC_POLL_ARMED.load(std::sync::atomic::Ordering::Relaxed); | |
| { | |
| let _zeal = super::super::zeal::ZealGuard::set(true); | |
| assert!( | |
| crate::gc::PERRY_GC_POLL_ARMED.load(std::sync::atomic::Ordering::Relaxed) > 0, | |
| "zeal must keep the poll reachable even with no deferral outstanding" | |
| ); | |
| } | |
| assert_eq!( | |
| crate::gc::PERRY_GC_POLL_ARMED.load(std::sync::atomic::Ordering::Relaxed), | |
| base, | |
| "dropping zeal must return its poll arm" | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/perry-runtime/src/gc/tests/triggers.rs` around lines 758 - 770, Update
zeal_holds_the_poll_word_armed_with_nothing_pending to capture
PERRY_GC_POLL_ARMED’s baseline before creating ZealGuard, then assert after the
guard scope ends that the counter equals that baseline. Remove the misleading
post-scope set_safepoint_pending(false) call, since it can return without
validating ZealGuard::drop released its arm.
0739c2f to
7620f6e
Compare
…oad (#7721) the collector and wrong about its price. The poll is emitted at EVERY allocating loop back-edge — 20 M of them in `bench/churn_alloc.ts` — so its no-work path is a per-iteration cost of the language, and that path was an out-of-line call into two `OnceLock` acquire loads, an unconditional atomic increment, and a thread-local read that on Darwin is a CALL to `_tlv_get_addr`. ~3 ns per back-edge: `churn_alloc` 0.367 s -> 0.419, `push_cls` 0.350 -> 0.408, `push_num` 0.131 -> 0.178. `gc/poll_arm.rs` adds `PERRY_GC_POLL_ARMED`, a process-global counter of the reasons the poll must do more than return. Zero is a PROOF the poll is a no-op, so codegen loads it inline and branches around the call (two aarch64 instructions, address hoisted into the preheader) and the runtime entry point re-checks it for modules from any other emission path. `GC_SAFEPOINT_PENDING` now has exactly one writer, `policy::set_safepoint_pending`, which moves the flag and the global together — the word reading zero while a deferral is outstanding is the one unsound direction, and it would strand that collection until an event-loop boundary a compute-only program never reaches. Measured best-of-7 interleaved on the quiet M1 bench host, outputs verified against `node --experimental-strip-types`: | bench | main | this | 0.5.1384 | |---|--:|--:|--:| | churn_alloc | 0.419 | 0.376 | 0.367 | | push_cls | 0.408 | 0.357 | 0.350 | | push_num | 0.178 | 0.144 | 0.131 | | churn | 0.45 | 0.41 | — | | churn_read | 0.02 | 0.02 | — | | cycles | 0.19 | 0.19 | — | | deeplist | 0.31 | 0.31 | — | | tree | 1.64 | 1.64 | — | | tree_wide | 2.10 | 2.12 | — | GC behaviour is unchanged: `churn` runs 105 minors in both arms with positive reclamation every cycle, max pause 3.63 ms -> 1.78 ms. `gc-handoff/apps/iso_miss.ts` prints `checksum 437840 misses 0`.
7620f6e to
5e9119b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Cargo.toml`:
- Line 318: Revert the unapproved release-version updates in Cargo.toml lines
318-318 and CLAUDE.md lines 11-11: restore the previous workspace package
version in Cargo.toml and the prior documented current version in CLAUDE.md,
leaving release metadata changes to maintainers.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a208eb16-6e79-4d20-b7b6-df38bb7df781
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
CLAUDE.mdCargo.tomlcrates/perry-runtime/src/gc/mod.rscrates/perry-runtime/src/gc/policy.rscrates/perry-runtime/src/gc/tests/triggers.rscrates/perry-runtime/src/gc/zeal.rs
🚧 Files skipped from review as they are similar to previous changes (4)
- crates/perry-runtime/src/gc/tests/triggers.rs
- crates/perry-runtime/src/gc/zeal.rs
- crates/perry-runtime/src/gc/mod.rs
- crates/perry-runtime/src/gc/policy.rs
|
|
||
| [workspace.package] | ||
| version = "0.5.1428" | ||
| version = "0.5.1429" |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== relevant files and git status =="
git status --short --ignore-submodules || true
git diff -- Cargo.toml CLAUDE.md || true
echo "== diff stat =="
git diff --stat -- Cargo.toml CLAUDE.md || true
echo "== exact diff hunks =="
git diff -- Cargo.toml CLAUDE.md | sed -n '1,220p'
echo "== workspace version context =="
sed -n '308,322p' Cargo.toml
echo "== CLAUDE version context =="
sed -n '1,20p' CLAUDE.md
echo "== contributor/maintainer hints =="
git remote -v || true
git config --get remote.origin.url || true
git diff --name-only HEAD~1..HEAD 2>/dev/null || trueRepository: PerryTS/perry
Length of output: 3032
Keep release metadata maintainer-owned.
Unless this is a maintainer release-prepared PR, revert the synchronized version changes in both release-facing files.
Cargo.toml#L318: revert the[workspace.package.version]bump.CLAUDE.md#L11: revert the documented current-version update.
📍 Affects 2 files
Cargo.toml#L318-L318(this comment)CLAUDE.md#L11-L11
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Cargo.toml` at line 318, Revert the unapproved release-version updates in
Cargo.toml lines 318-318 and CLAUDE.md lines 11-11: restore the previous
workspace package version in Cargo.toml and the prior documented current version
in CLAUDE.md, leaving release metadata changes to maintainers.
Sources: Coding guidelines, Learnings
Merging as v0.5.1429 — reconciled with #7729 by hand, not auto-mergedThe audit predicted this collision precisely: #7729 and this PR rewrite the same lines of The conflict turned out to be purely additive — #7733's major-pacing tests and this PR's poll-arming tests appended at the same point in
The semantics are compatible for a reason worth stating: this PR keeps the arming word nonzero for the life of the process under zeal, which is exactly what #7729's allocation-paced stride needs — every poll must still reach the runtime for the stride check to run. The design is the good part
Over-arming costs a wasted call; under-arming would be unsound; wrapping would be neither detected nor recoverable. Making the undetectable outcome impossible, rather than merely unlikely, is the right ordering of those risks. The ordering hazard that would have reintroduced the bug is also handled: Relaxed ordering is sufficient, and for a stateable reason: the only requirement is same-thread visibility — each And the diagnosis refuted the obvious suspect rather than confirming it: the layout/store symbols are unchanged ( Three follow-ups, none blocking
Gates 21/21. |
1 participant
What regressed, and it was not the pointer-field work
Three all-numeric benchmarks lost 15–30 % in the #7686..#7721 window. The obvious
suspect was #7686/#7698's typed pointer-field stores generalising a path that used
to have an all-numeric special case. Measured, that hypothesis is refuted: a
symbolicated profile of
churn_alloc_bigat 0.5.1384 and at main has the same shapesymbol for symbol —
gc::layout::init_typed_shape_layoutAnonShapeconstructorperry_fn_..._chunklayout_tables::layout_forget_objectNothing in the layout or store path grew. What appeared instead were two symbols not
present in the 0.5.1384 profile at all:
js_gc_loop_safepointat 8.2 % and_tlv_get_addrback at 8–9 %, having been driven to 0 % by #7469. Both arrivewith #7721, which turned the moving-loop back-edge poll on by default.
Mechanism
#7721 was right about the collector — the poll is the only precise
nursery-collection point a compute-only program ever reaches, and without it every
nursery collection happens at the register-imprecise allocation point where #7682
made it correctly non-moving. That is worth
tree_wide7.26 s instead of 2.11.What was wrong was the poll's price.
A poll is emitted at every allocating loop back-edge: 20 M of them in
bench/churn_alloc.ts. So its no-work path is a per-iteration cost of the language,paid whether or not a collection is ever due — and that path was an out-of-line
extern "C"call intogc_moving_loop_polls_enabled()— aOnceLockacquire load,note_loop_poll_reached()— an unconditionalAtomicU64::fetch_addon aprocess-shared line,
GC_SAFEPOINT_PENDING.with(Cell::get)— a thread-local read, which on Darwin isa CALL to
_tlv_get_addr, Mach-O having no local-exec TLS model,gc_zeal_enabled()— a secondOnceLockacquire load,plus the caller-side spill/reload the opaque call forces. ~3 ns per back-edge, which
is the regression to the millisecond: 20 M × 3 ns = 60 ms against a
churn_allocgap of 51.5 ms.
The fix
gc/poll_arm.rsaddsPERRY_GC_POLL_ARMED, a plain process-globalAtomicU32counting the reasons
js_gc_loop_safepointmust do more than return. Zero is aproof the poll is a no-op, so both ends answer on one ordinary load: codegen emits
the load inline and branches around the call, and the runtime entry point re-checks
it so a module from any other emission path still gets the cheap answer. The emitted
guard, with the address hoisted into the preheader:
The word is a deliberate conservative SUPERSET. It is process-global — a
thread-local would reintroduce the
_tlv_get_addrthis removes — so it countsthreads with a deferral outstanding, and a poll on thread B can be woken by a
deferral on thread A and find nothing to do. The unsound direction is the word
reading zero while a deferral is outstanding, which would strand that collection
until an event-loop boundary a compute-only program never reaches. Hence
GC_SAFEPOINT_PENDINGnow has exactly one writer,policy::set_safepoint_pending,which moves both representations together.
The load is volatile: the runtime writes this word from calls LLVM cannot see
through, and a guard whose load got hoisted out of its loop would read a stale zero
and silently stop draining — #7721's failure mode returning as a codegen bug instead
of a default. One
ldreither way.Zeal keeps the word armed for the life of the process, because
PERRY_GC_ZEAL'scontract is a collection at every safepoint, not only at ones already deferred.
ZealGuardmirrors it so a unit test under zeal cannot silently poll into a no-op,and
loop_polls_reached()now documents that it is exhaustive exactly under zeal —the one place
zeal_verdictreads it.Measurements
Best-of-5, all four arms interleaved in one session, quiet M1 bench host,
PERRY_NO_AUTO_OPTIMIZE=1with apinned
PERRY_RUNTIME_DIR; every output verified byte-identical tonode --experimental-strip-typesbefore timing.Every protected benchmark holds;
churnimproves as well.GC behaviour is unchanged where it matters:
PERRY_GC_TRACE=1 ./n_churnruns105 minor cycles in both arms, positive reclamation on every cycle, max pause
3.63 ms → 1.78 ms.
gc-handoff/apps/iso_miss.tsprintschecksum 437840 misses 0.What is left, and why
The residual to 0.5.1384 is 9.4 ms on
churn_alloc, 6.2 onpush_cls, 12.6 onpush_num— 0.5–0.6 ns per back-edge, i.e. exactly the two guard instructions,confirmed by disassembly rather than inferred. It is not overhead that can be
tightened away: it is the price of the poll existing, and the poll is what makes the
nursery evacuate. The 0.5.1384 numbers came from a moving minor at the allocation
point, which #7682 removed as unsound — compiling and running today's main with
PERRY_GC_MOVING_LOOP_POLLS=0, the 0.5.1384 configuration, giveschurn_alloc0.91 s, not 0.36. Driving the guard to one instruction would mean a
signal-backed polling page (
ldr wzr, [x26]+ an mprotect'd page); not proportionateto 5 ms, so it is noted rather than done.
Gates run
perry-runtime --lib gc::— 725 pass, plus the 6 new ones.perry-codegen— full suite, includingloop_safepoint_purity(8/8).gc_root_dominance_corpus.sh+ both gated checker modes: 131/131 sources, 0skipped, 0 violations in
--moving-only(40/40 seeded violations caught) and0 in
--unrooted-allocas.check_file_size.sh,addr_class_inventory.py,cargo fmt --all -- --check.Two failures reproduce identically with this branch's sources reverted to
origin/mainand are not from this change: the threegenerator_attach_prototypecases (fixed by #7731, now merged) andlarge_object_barriers::large_local_array_push_inbounds_store_emits_precise_slot_barrier.Summary by CodeRabbit
Performance
Reliability
Tests