perf(gc): stop the globalThis bootstrap from disabling the per-object layout fast path - #7809
Conversation
276c636 to
c97f4a9
Compare
📝 WalkthroughWalkthroughThe GC now uses ChangesGlobalThis layout optimization
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant GlobalThisBootstrap
participant ImmortalLayoutScope
participant LayoutTables
participant GCDiagnostics
GlobalThisBootstrap->>ImmortalLayoutScope: Enter scope
GlobalThisBootstrap->>LayoutTables: Allocate and populate bootstrap objects
LayoutTables-->>GlobalThisBootstrap: Avoid persistent masks for eligible objects
GlobalThisBootstrap->>GCDiagnostics: Record bootstrap duration and table sizes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
… layout fast path Any plain-object or array property MISS forces the lazy `globalThis` bootstrap, and the bootstrap left 1113 immortal entries in the per-object GC slot-layout side tables. `PER_OBJECT_LAYOUTS_NONEMPTY` — the emptiness proof that keeps `layout_forget_object` off the allocation, death and relocation paths — could then never go false again, so every real TypeScript program ran the full two-map probe on every allocation. Measured: `churn` +28%, `tree` +29% from one `for…of` in `main()`, with `layout_forget_object` self time 112 -> 916 ms and 194 -> 740 ms. It is not the ~1.15 MB the bootstrap allocates and it is not GC pacing: 105 minors on `churn` either way, ~616 KB more copied over the whole run. Two changes, both load-bearing: * `gc::ImmortalLayoutScope` around `populate_global_this_builtins` — objects built inside it declare `GC_LAYOUT_UNKNOWN` (the tag-checked scan the code already falls back to for the same case) instead of minting a mask nothing will ever remove. Residue 1113 -> 0. NOT applied to typed-shape layouts, whose raw-f64 slots a conservative scan would misread as pointers and, under the copying collector, rewrite. * An 8192-bit thread-local address filter replacing the global flag as the hot guard. The scope alone moved nothing measurable: ordinary runtime init still leaves one or two immortal records, and for a single global bit two entries are exactly as bad as 1113. The filter answers "can THIS ADDRESS have an entry" instead. It replaces rather than joins the flag because testing both cost a second thread-local resolution on legitimately-armed workloads (+3.4% interp, +4.6% iso_miss). Five tests, none able to pass vacuously: the bootstrap leaves the tables empty (with a subject-live check on `globalThis.Array`); the same store outside a scope still mints a mask; a scoped object still traces its children; a live record survives a filter rebuild; and the filter still proves unrelated addresses absent while the global flag is armed. Claude-Session: https://claude.ai/code/session_012B8z92S82sCfqCrVqrFgS2
c97f4a9 to
ba19360
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
crates/perry-runtime/src/gc/layout_tables.rs (2)
152-163: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe stated precondition does not hold at two of the four call sites.
The comment says the rebuild check must run before the bit is set because "this key is not in the map yet at any call site". That is true for
typed_layouts_insert(Line 345 precedes the insert at Line 346) andslot_masks_insert(Line 355 precedes Line 356). It is false fortransfer_per_object_descriptor(Line 434 runs aftertyped.insertat Line 432) andtransfer_per_object_slot_mask(Line 454 runs aftermasks.insertat Line 452).Those two sites are still safe, but for the opposite reason: the key is already live in the map, so a rebuild reconstructs its bit. Correct the comment so a future call site is not written against a false invariant.
📝 Proposed comment fix
/// Record that `user_ptr` now has an entry. Called by every insert site. -/// The rebuild check runs BEFORE the bit is set, never after: a rebuild -/// reconstructs the filter from the maps' live keys, and this key is not in -/// the map yet at any call site, so rebuilding afterwards would erase the bit -/// just set and make a live record invisible to the filter. +/// The rebuild check runs BEFORE the bit is set, never after: a rebuild +/// reconstructs the filter from the maps' live keys. Where the caller has not +/// inserted the key yet (`typed_layouts_insert`, `slot_masks_insert`), +/// rebuilding afterwards would erase the bit just set and make a live record +/// invisible to the filter. Where the caller has already inserted it +/// (`transfer_per_object_descriptor`, `transfer_per_object_slot_mask`), the +/// rebuild reconstructs the bit from the map, so either order is sound.🤖 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/layout_tables.rs` around lines 152 - 163, Update the comment above layout_addr_filter_add to remove the false claim that user_ptr is absent from every map at every call site. State instead that callers either invoke it before inserting the key, or after insertion when a rebuild can recover the key from the live map, and preserve the requirement that rebuilding occurs before layout_addr_filter_note.
362-362: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winThese four guards resolve the hot thread-local slot twice.
per_object_layouts_maybe_nonempty()callshot_per_object_layout_hint()at Line 309, andlayout_addr_filter_may_hold()calls it again at Line 141. On Darwin each resolution is an out-of-line_tlv_get_addrcall. The doc on Lines 134-138 identifies that second resolution as the residual cost and states it "goes away by co-locating the flag and this filter in one thread-local". This PR did co-locate them, andlayout_forget_objectat Lines 474-475 already takes the single-resolution form. These four sites were not updated.
typed_layouts_remove,slot_masks_remove,with_per_object_descriptor, andper_object_slot_maskall run on the same allocation, death, and relocation paths that motivated the change, so they should use the resolved hint too. Extract a helper and use it at all five sites.⚡ Proposed refactor: one shared single-resolution guard
+/// The full guard — flag then address filter — against ONE hot-slot +/// resolution. `false` proves `user_ptr` has no entry in either side table. +#[inline(always)] +pub(in crate::gc) fn per_object_layouts_may_hold(user_ptr: usize) -> bool { + let hint = hot_per_object_layout_hint(); + hint.nonempty.get() && hint_may_hold(hint, user_ptr) +}pub(in crate::gc) fn typed_layouts_remove(user_ptr: usize) { - if !per_object_layouts_maybe_nonempty() || !layout_addr_filter_may_hold(user_ptr) { + if !per_object_layouts_may_hold(user_ptr) { return; }pub(in crate::gc) fn slot_masks_remove(user_ptr: usize) { - if !per_object_layouts_maybe_nonempty() || !layout_addr_filter_may_hold(user_ptr) { + if !per_object_layouts_may_hold(user_ptr) { return; }) -> Option<R> { - if !per_object_layouts_maybe_nonempty() || !layout_addr_filter_may_hold(user_ptr) { + if !per_object_layouts_may_hold(user_ptr) { return None; }pub(in crate::gc) fn per_object_slot_mask(user_ptr: usize) -> Option<LayoutSlotMask> { - if !per_object_layouts_maybe_nonempty() || !layout_addr_filter_may_hold(user_ptr) { + if !per_object_layouts_may_hold(user_ptr) { return None; }
layout_forget_objectcan then reuse the same helper in place of its inline form.Validate with
cargo check -p perryand theperry-devprofile before any release-profile timing run.Also applies to: 375-375, 393-393, 404-404
🤖 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/layout_tables.rs` at line 362, Extract a shared guard helper that resolves the thread-local hot layout hint once and checks both the nonempty hint and address filter. Update typed_layouts_remove, slot_masks_remove, with_per_object_descriptor, per_object_slot_mask, and layout_forget_object to use this helper, preserving their existing guard behavior. Validate with cargo check -p perry and the perry-dev profile before release-profile timing.Source: Coding guidelines
crates/perry-runtime/src/gc/layout.rs (1)
1357-1368: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUpdate the
ImmortalLayoutScopedocumentation to cover non-typed rebuilds.
layout_rebuild_from_slots_with_policyclearsGC_OBJ_TYPED_LAYOUT_INTACTbefore scanning, so itsGC_LAYOUT_UNKNOWNbranch cannot scan an active typed descriptor. During theglobalThisbootstrap, rebuild callers receive only NaN-boxed object or keys-array slots, not raw-f64 array contents. Document this exception and keep the prohibition for typed layout installation.🤖 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/layout.rs` around lines 1357 - 1368, Update the ImmortalLayoutScope documentation to state that non-typed layout rebuilds are permitted because layout_rebuild_from_slots_with_policy clears GC_OBJ_TYPED_LAYOUT_INTACT and bootstrap rebuilds scan only NaN-boxed object or keys-array slots. Retain the prohibition against applying the scope to typed descriptors containing raw-f64 slots.
🤖 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 `@changelog.d/7809-globalthis-bootstrap-layout-latch.md`:
- Around line 49-56: Update the stale filter-size prose to match
LAYOUT_ADDR_FILTER_BITS: in
changelog.d/7809-globalthis-bootstrap-layout-latch.md lines 49-56, change
8192-bit to 4096-bit; in
crates/perry-runtime/src/gc/tests/layout_trace/per_object_tables.rs lines
439-440, change 8192 bits to 4096 bits. No assertion or implementation changes
are needed.
In `@crates/perry-runtime/src/gc/tests/layout_trace/per_object_tables.rs`:
- Around line 366-406: Update
test_layout_addr_filter_never_hides_a_live_record_across_a_rebuild so its
inserts use the path that invokes layout_addr_filter_add, rather than relying on
layout_note_slot, which skips the rebuild threshold check. Preserve the existing
live-record lookup and removal assertions, and ensure enough inserts still force
at least one filter rebuild.
In `@crates/perry-runtime/src/object/global_this/populate.rs`:
- Around line 762-775: Capture per_object_layout_table_sizes() alongside
Instant::now() in crates/perry-runtime/src/object/global_this/populate.rs at
lines 762-775, report the bootstrap-created deltas instead of absolute counts,
and soften the comment’s claim that counts must be 0 0. In
crates/perry-runtime/src/gc/tests/layout_trace/per_object_tables.rs at lines
339-364, record table sizes before js_get_global_this() and assert the resulting
delta is (0, 0).
---
Nitpick comments:
In `@crates/perry-runtime/src/gc/layout_tables.rs`:
- Around line 152-163: Update the comment above layout_addr_filter_add to remove
the false claim that user_ptr is absent from every map at every call site. State
instead that callers either invoke it before inserting the key, or after
insertion when a rebuild can recover the key from the live map, and preserve the
requirement that rebuilding occurs before layout_addr_filter_note.
- Line 362: Extract a shared guard helper that resolves the thread-local hot
layout hint once and checks both the nonempty hint and address filter. Update
typed_layouts_remove, slot_masks_remove, with_per_object_descriptor,
per_object_slot_mask, and layout_forget_object to use this helper, preserving
their existing guard behavior. Validate with cargo check -p perry and the
perry-dev profile before release-profile timing.
In `@crates/perry-runtime/src/gc/layout.rs`:
- Around line 1357-1368: Update the ImmortalLayoutScope documentation to state
that non-typed layout rebuilds are permitted because
layout_rebuild_from_slots_with_policy clears GC_OBJ_TYPED_LAYOUT_INTACT and
bootstrap rebuilds scan only NaN-boxed object or keys-array slots. Retain the
prohibition against applying the scope to typed descriptors containing raw-f64
slots.
🪄 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: 9d5f9a59-caef-4f9d-855f-60e05471f7c7
📒 Files selected for processing (8)
changelog.d/7809-globalthis-bootstrap-layout-latch.mdcrates/perry-runtime/src/gc/hot_tls.rscrates/perry-runtime/src/gc/layout.rscrates/perry-runtime/src/gc/layout_tables.rscrates/perry-runtime/src/gc/mod.rscrates/perry-runtime/src/gc/telemetry.rscrates/perry-runtime/src/gc/tests/layout_trace/per_object_tables.rscrates/perry-runtime/src/object/global_this/populate.rs
| 2. An **address filter** replacing `PER_OBJECT_LAYOUTS_NONEMPTY` as the hot | ||
| guard. Change 1 alone moved nothing measurable, and that is the important | ||
| finding: ordinary runtime init still leaves one or two long-lived records | ||
| behind, and for a single global bit two entries are exactly as bad as 1113. | ||
| An 8192-bit thread-local filter over the key addresses turns "is either | ||
| table empty?" into "can this *address* have an entry?", so a nursery | ||
| address the tables have never seen is proved absent in one multiply and one | ||
| load even while immortal records exist elsewhere. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Stale 8192-bit filter size in prose. crates/perry-runtime/src/gc/layout_tables.rs Line 92 sets LAYOUT_ADDR_FILTER_BITS to 4096, and its doc comment describes 4096 bits / 512 B. Two prose sites still describe an 8192-bit filter, so the filter size was reduced without updating the text.
changelog.d/7809-globalthis-bootstrap-layout-latch.md#L49-L56: change "An 8192-bit thread-local filter" to "A 4096-bit thread-local filter". Release notes are assembled from this fragment, so the published number would be wrong.crates/perry-runtime/src/gc/tests/layout_trace/per_object_tables.rs#L439-L440: change "one live record in 8192 bits" to "one live record in 4096 bits". The assertion threshold on Line 455 still holds at 4096 bits, so only the comment needs the edit.
📍 Affects 2 files
changelog.d/7809-globalthis-bootstrap-layout-latch.md#L49-L56(this comment)crates/perry-runtime/src/gc/tests/layout_trace/per_object_tables.rs#L439-L440
🤖 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 `@changelog.d/7809-globalthis-bootstrap-layout-latch.md` around lines 49 - 56,
Update the stale filter-size prose to match LAYOUT_ADDR_FILTER_BITS: in
changelog.d/7809-globalthis-bootstrap-layout-latch.md lines 49-56, change
8192-bit to 4096-bit; in
crates/perry-runtime/src/gc/tests/layout_trace/per_object_tables.rs lines
439-440, change 8192 bits to 4096 bits. No assertion or implementation changes
are needed.
| /// The address filter is an *accelerator*, never an authority: `false` must be | ||
| /// a proof of absence and nothing else may rest on it. This drives enough | ||
| /// inserts to force at least one filter rebuild and then checks, for every | ||
| /// live record, that the guarded accessors still find it and that | ||
| /// `layout_forget_object` still removes it. | ||
| #[test] | ||
| fn test_addr_filter_never_hides_a_live_record_across_a_rebuild() { | ||
| clear_marks(); | ||
| clear_mark_seeds(); | ||
|
|
||
| // Comfortably more inserts than the rebuild threshold (half the bits), so | ||
| // the rebuild path is exercised rather than merely reachable. | ||
| let mut objs = Vec::new(); | ||
| for _ in 0..6000 { | ||
| let obj = crate::object::js_object_alloc(0, 2); | ||
| let child = crate::object::js_object_alloc(0, 0); | ||
| crate::gc::layout_note_slot(obj as usize, 1, POINTER_TAG | (child as u64 & POINTER_MASK)); | ||
| objs.push(obj); | ||
| } | ||
| assert!(flag(), "6000 masks must arm the flag"); | ||
|
|
||
| for (i, obj) in objs.iter().enumerate() { | ||
| assert_eq!( | ||
| test_layout_pointer_slot_count(*obj as usize, 2), | ||
| Some(1), | ||
| "record {i} became invisible — the filter proved absence for an \ | ||
| address that has a live entry" | ||
| ); | ||
| } | ||
| for obj in &objs { | ||
| crate::gc::layout_clear_for_ptr(*obj as usize); | ||
| } | ||
| assert!( | ||
| test_per_object_tables_are_empty(), | ||
| "every record must still be removable after a filter rebuild" | ||
| ); | ||
| assert!(!flag()); | ||
|
|
||
| clear_marks(); | ||
| clear_mark_seeds(); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Confirm which call sites can reach the rebuild check.
set -euo pipefail
echo '--- layout_addr_filter_add call sites (rebuilding form) ---'
rg -nP --type=rust -C3 '\blayout_addr_filter_add\s*\(' crates/perry-runtime/src
echo '--- layout_addr_filter_note call sites (non-rebuilding form) ---'
rg -nP --type=rust -C3 '\blayout_addr_filter_note\s*\(' crates/perry-runtime/src
echo '--- layout_addr_filter_rebuild call sites ---'
rg -nP --type=rust -C3 '\blayout_addr_filter_rebuild\s*\(' crates/perry-runtime/srcRepository: PerryTS/perry
Length of output: 6477
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- relevant symbols and constants ---'
rg -n -C4 \
'LAYOUT_ADDR_FILTER_REBUILD_AFTER|fn layout_addr_filter_(add|note|rebuild)|fn layout_note_slot|test_addr_filter_never_hides_a_live_record_across_a_rebuild' \
crates/perry-runtime/src/gc/layout_tables.rs \
crates/perry-runtime/src/gc/layout.rs \
crates/perry-runtime/src/gc/tests/layout_trace/per_object_tables.rs
echo '--- direct call graph facts ---'
python3 - <<'PY'
from pathlib import Path
for name in [
"crates/perry-runtime/src/gc/layout_tables.rs",
"crates/perry-runtime/src/gc/layout.rs",
"crates/perry-runtime/src/gc/tests/layout_trace/per_object_tables.rs",
]:
text = Path(name).read_text()
print(f"\n{name}")
for needle in [
"layout_addr_filter_add(",
"layout_addr_filter_note(",
"layout_addr_filter_rebuild(",
"layout_note_slot(",
"test_addr_filter_never_hides_a_live_record_across_a_rebuild(",
]:
lines = [i + 1 for i, line in enumerate(text.splitlines()) if needle in line]
if lines:
print(f"{needle}: {lines}")
PYRepository: PerryTS/perry
Length of output: 6089
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- layout_note_slot implementation ---'
sed -n '771,825p' crates/perry-runtime/src/gc/layout.rs
echo '--- all test function bodies around relevant calls ---'
sed -n '350,445p' crates/perry-runtime/src/gc/tests/layout_trace/per_object_tables.rs
echo '--- filter counter implementation ---'
sed -n '145,180p' crates/perry-runtime/src/gc/layout_tables.rs
echo '--- calls in the target test body ---'
python3 - <<'PY'
from pathlib import Path
p = Path("crates/perry-runtime/src/gc/tests/layout_trace/per_object_tables.rs")
lines = p.read_text().splitlines()
start = next(i for i, line in enumerate(lines) if "fn test_addr_filter_never_hides_a_live_record_across_a_rebuild" in line)
depth = 0
seen_open = False
for i in range(start, len(lines)):
line = lines[i]
depth += line.count("{") - line.count("}")
if "{" in line:
seen_open = True
if seen_open and any(token in line for token in (
"layout_note_slot(",
"layout_addr_filter_add(",
"layout_rebuild_from_slots(",
"typed_layouts_insert(",
"slot_masks_insert(",
"layout_addr_filter_rebuild(",
)):
print(f"{i+1}: {line.strip()}")
if seen_open and depth == 0:
break
PYRepository: PerryTS/perry
Length of output: 8803
Exercise the rebuild path in this test
layout_note_slot calls layout_addr_filter_note, which skips the threshold check. Therefore, the 6000 inserts do not call layout_addr_filter_rebuild. Insert records through a path that calls layout_addr_filter_add.
🤖 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/layout_trace/per_object_tables.rs` around
lines 366 - 406, Update
test_layout_addr_filter_never_hides_a_live_record_across_a_rebuild so its
inserts use the path that invokes layout_addr_filter_add, rather than relying on
layout_note_slot, which skips the rebuild threshold check. Preserve the existing
live-record lookup and removal assertions, and ensure enough inserts still force
at least one filter rebuild.
| // The bootstrap's own cost, and the evidence that the `ImmortalLayoutScope` | ||
| // above actually did its job. `slot_masks`/`typed` are the live entry | ||
| // counts of the two per-object layout side tables: they must still read | ||
| // `0 0` here, because a non-zero count is exactly what disables | ||
| // `PER_OBJECT_LAYOUTS_NONEMPTY` for the rest of the process. | ||
| if let Some(started) = bootstrap_started { | ||
| let (slot_masks, typed) = crate::gc::per_object_layout_table_sizes(); | ||
| eprintln!( | ||
| "[gc-globalthis-bootstrap] elapsed_us={} per_object_slot_masks={} per_object_typed_layouts={}", | ||
| started.elapsed().as_micros(), | ||
| slot_masks, | ||
| typed | ||
| ); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Whole-table sizes are reported and asserted as bootstrap residue. per_object_layout_table_sizes() returns the live size of both thread-local side tables. It does not isolate the records this bootstrap created. The changelog fragment states that ordinary runtime init still leaves one or two long-lived records behind, so a value of 0 0 is not guaranteed even when the ImmortalLayoutScope works correctly. Capture the sizes before the scope opens and compare the delta at both sites.
crates/perry-runtime/src/object/global_this/populate.rs#L762-L775: captureper_object_layout_table_sizes()alongsideInstant::now()on Line 98, then print the difference instead of the absolute counts, and soften the comment claim that the values "must still read0 0here".crates/perry-runtime/src/gc/tests/layout_trace/per_object_tables.rs#L339-L364: record the table sizes beforejs_get_global_this()and assert the delta is(0, 0), which removes this test's dependence on every other test in the process cleaning up completely.
📍 Affects 2 files
crates/perry-runtime/src/object/global_this/populate.rs#L762-L775(this comment)crates/perry-runtime/src/gc/tests/layout_trace/per_object_tables.rs#L339-L364
🤖 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/object/global_this/populate.rs` around lines 762 -
775, Capture per_object_layout_table_sizes() alongside Instant::now() in
crates/perry-runtime/src/object/global_this/populate.rs at lines 762-775, report
the bootstrap-created deltas instead of absolute counts, and soften the
comment’s claim that counts must be 0 0. In
crates/perry-runtime/src/gc/tests/layout_trace/per_object_tables.rs at lines
339-364, record table sizes before js_get_global_this() and assert the resulting
delta is (0, 0).
* fix(lint): unbreak the 2000-line file-size gate on main Two files crossed the cap in the 2026-08-11 batch and `lint` has been red on `main` ever since: crates/perry-hir/src/lower/pre_scan.rs 1985 -> 2011 (#7828) crates/perry-runtime/src/gc/layout.rs 1982 -> 2023 (#7809, #7812) Both are pure code moves, no logic change: * `pre_scan_weakref_locals` and its doc comment move to `lower/pre_scan/weakref_locals.rs` (1653 lines left behind). It is the cohesive unit — one top-level pre-scan with its own five local sets — and it is where #7828 added the lines. * `LayoutSlotMask` (the enum and its whole `impl`) moves to `gc/layout/slot_mask.rs` (1807 lines left behind). Its visibility widens from `pub(super)` to `pub(in crate::gc)` because the type is now one module deeper and `layout_tables.rs` / `hot_tls.rs` still name it; the reachable set is unchanged. `scripts/check_file_size.sh` passes. * docs: changelog fragment for #7830 * docs: condense #7830 changelog entry --------- Co-authored-by: Ralph Küpper <ralph3@skelpo.com> Co-authored-by: Ralph Küpper <ralph@skelpo.com>
What
Touching
globalThis— which any plain-object or array property missdoes, via
builtin_prototype_value→js_get_global_this_builtin_value— usedto permanently disable the per-object GC slot-layout fast path for the rest of
the process. Adding a single
for (const _x of [1]) {}tomain()costchurn+28% and
tree+29%.Perry's whole benchmark corpus happens never to take that path (no
for…of, nospread, no
Symbol, no property miss anywhere inchurn/tree/interp/shapes/asyncpipe/retain), so the cost wasinvisible to the perf campaign while real TypeScript programs paid it before
their first line of work.
The mechanism (not what it looked like)
It is not the ~1.15 MB the bootstrap allocates, and it is not GC pacing.
GC behaviour is effectively identical either way — 105 minors on
churnwithand without, ~616 KB more copied across the entire run.
It is a global latch. The bootstrap builds hundreds of permanently-rooted
plain objects; each one's first pointer field minted an entry in
LAYOUT_SLOT_MASKS. Those entries are immortal, soPER_OBJECT_LAYOUTS_NONEMPTY— the emptiness proof that keeps
layout_forget_objectoff the allocation,death and relocation paths — could never go
falseagain.Symbolicated
layout_forget_objectself time:churn+for…ofTwo changes, both needed
gc::ImmortalLayoutScopearoundpopulate_global_this_builtins.Objects built inside it declare
GC_LAYOUT_UNKNOWN(the tag-checked payloadscan — the code's own fallback for the same situation, and the universally
safe state) instead of minting a mask nothing will ever remove.
Residue: 1113 entries → 0.
Deliberately not applied to typed-shape layouts: those describe raw-f64
slots, whose bit patterns can alias a heap pointer, and a conservative scan
would trace — and under the copying collector rewrite — a slot holding a
number.
An address filter, co-located with the flag in one thread-local.
Change 1 alone moved nothing measurable, which is the important finding:
ordinary runtime init still leaves one or two long-lived records, and for a
single global bit two entries are exactly as bad as 1113. A 4096-bit filter
turns "is either table empty?" into "can this address have an entry?", so
a nursery address the tables have never seen is proved absent in one
multiply and one load even while immortal records exist elsewhere.
All three arrangements were measured on the quiet mini:
churnpush_clstreeinterpchurn+for…oftree+for…ofDropping the flag loses: almost every workload is disarmed, and for those
the flag is one load where the filter is a multiply, a shift, a load and a
test (
push_clswent past budget). Two separate thread-locals cost a second_tlv_get_addron the workloads that ARE armed. One struct behind theexisting named hot slot gives both.
This is #7510's lesson repeating ("one immortal entry nullifies an is-empty
accelerator") — there it was a single interned keys array, here it is the
globalThisbootstrap at 1000× the scale.Tests
Five tests in
gc::tests::layout_trace::per_object_tables, written so none canpass vacuously:
globalThis.Arrayactually populated;cannot pass by the shape no longer reaching that branch at all;
scan;
armed — the exact condition under which the accelerator silently stopped
accelerating before, and which no existing test could observe.
PERRY_GC_DIAG=1now prints[gc-globalthis-bootstrap] elapsed_us=… per_object_slot_masks=… per_object_typed_layouts=…once per thread, so the residue is observable rather than inferred.
Measured (quiet mini, base
b9415d780, both arms built locally, interleaved best-of-5, exit-checked)churn+for…oftree+for…ofchurn(floor)tree(floor)Every protected bench stays inside budget (
churn0.422,churn_alloc0.375,push_cls0.368,push_num0.143,churn_read0.022,cycles0.194,deeplist0.245,tree1.640,tree_wide2.113,retain0.536,retain_wide1.092,fib400.393,interp1.922,shapes0.226,asyncpipe0.716). Outputs byte-identical to node with exit 0; canaryiso_missprintschecksum 437840 misses 0; clean underPERRY_GC_PROTECT_FROMSPACE=1 PERRY_GC_PROTECT_FROMSPACE_DEPTH=800andPERRY_GC_VERIFY_EVACUATION=1withretired_setcounts > 0 on the copyingbenches. Gap suite: no new failures — the 8 rows the gate reports reproduce
identically on clean
main.interp1.888 → 1.922 andiso_miss2.361 → 2.443 are the honest cost: bothare legitimately armed, so the filter never proves absence for them and they
pay the test without the benefit.
Not fixed here
The bootstrap's own fixed cost (
elapsed_us≈6100under load; ~4.4 ms on a quiethost) is untouched — it is ~3000
set_builtin_property_attrscalls, each aStringallocation plus a(usize, String)hash insert. Two of the three perinstalled method are the identical
name/lengthdescriptor on a builtinclosure that already self-identifies as one, which is the obvious next lever.
A second residue is per-collection: the bootstrap's ~1100 closures and ~3000
descriptors are immortal but are re-scanned by
scan_closure_dynamic_props_roots_mut/
scan_descriptor_roots_mut/prune_dead_descriptorson every cycle(~17 ms on
churn). Same shape as the latch — immortal data on a per-cyclepath — and worth its own change.
Summary by CodeRabbit
Performance
globalThisinitialization by avoiding unnecessary permanent layout records.Diagnostics
Bug Fixes
Tests