Skip to content

Commit 5dbe513

Browse files
author
Ralph Küpper
committed
perf(gc): don't mint per-object pointer masks for single-slot payloads
`interp.ts` spent ~19% of its runtime in `layout_forget_object`, plus ~6% in the hashbrown probe underneath it, against a `LAYOUT_SLOT_MASKS` that had grown past 400,000 live entries. Instrumented on the isolated FIB half, the `PER_OBJECT_LAYOUTS_NONEMPTY` fast path fired 52 times in 15,000,000 calls. The interpreter allocates `{ names: [p], vals: [a], parent }` per interpreted call, so `layout_note_slot`'s "first pointer into a POINTER_FREE object" arm minted 1.8M masks over payloads of exactly one slot. A mask over one slot can skip nothing -- the tracer tag-checks that slot either way -- but the entry it creates keeps the emptiness flag armed, which puts a two-map hash probe back on every allocation in the program for as long as it lives. Both mint sites now decline the mask below DEFAULT_MASK_MIN_SLOTS and use GC_LAYOUT_UNKNOWN, the tag-checked scan-all-slots state that is already the established fallback on this path. The tag check is exact here: neither site is reachable for an object with an intact typed descriptor, so no raw-f64 slot can be misread as a pointer. Quiet M1 mini, best-of-5, interleaved against the same binaries with the policy disabled: interp 1.894 -> 1.697, iso_miss 2.371 -> 2.157, new bench/mask_tax probe 0.1218 -> 0.1049 with its numeric-element control flat at 1.000. No regression on the rest of the 19-benchmark corpus, including tree, tree_wide, retain*, cycles and deeplist.
1 parent b9415d7 commit 5dbe513

5 files changed

Lines changed: 363 additions & 10 deletions

File tree

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
### GC: stop minting per-object pointer masks for single-slot payloads
2+
3+
`interp.ts` — the tree-walking interpreter that best resembles real software in
4+
the benchmark corpus — spent **~19% of its runtime in `layout_forget_object`**,
5+
plus another ~6% in the hashbrown probe underneath it. That is side-table
6+
bookkeeping, not user work, and by design it should have been ~zero: #7510's
7+
`PER_OBJECT_LAYOUTS_NONEMPTY` flag exists so that the allocation, store, death
8+
and relocation paths can skip both per-object layout maps whenever they are
9+
empty, which "on a monomorphic workload they are".
10+
11+
They were not. Instrumented on `iso_FIB.ts` (the isolated FIB half):
12+
13+
```
14+
forget_total=15,000,000 fast=52 slow=14,999,948
15+
residency: masks=313,875 -> 381,505 -> 400,430 (still climbing)
16+
```
17+
18+
The disarmed fast path fired **52 times in 15 million calls**. Every other call
19+
took two `RefCell` round-trips and two hashes against a 400k-entry, cache-cold
20+
map — once per allocation, program-wide.
21+
22+
**Cause.** `layout_note_slot`'s "first pointer stored into a `POINTER_FREE`
23+
object" arm minted a per-object entry in `LAYOUT_SLOT_MASKS`. The interpreter
24+
allocates `{ names: [p], vals: [a], parent }` per interpreted call, so it minted
25+
two masks per call — **1.8M of them**, each a mask over a payload of exactly
26+
**one slot**. A mask over one slot cannot skip anything: the tracer consults
27+
`layout_pointer_bearing_bits` on that slot either way, so the entry was the
28+
mask's entire contribution. The entries also outlive their arrays — they are
29+
only reclaimed when the recycled address is allocated over — so residency grew
30+
without bound, and a single live entry anywhere keeps the flag armed for every
31+
allocation in the program. This is #7510's "one immortal entry nullifies
32+
`is_empty()`" a second time, from the other direction.
33+
34+
**Fix.** Both mint sites (`layout_note_slot` and
35+
`layout_rebuild_from_slots_with_policy`) now decline the mask when the payload
36+
is below `DEFAULT_MASK_MIN_SLOTS` (2, i.e. single-slot payloads only) and use
37+
`GC_LAYOUT_UNKNOWN` — the tag-checked scan-all-slots state — instead. That state
38+
is already the established fallback on this exact path, and the tag check is
39+
exact here: neither site is reachable for an object with an intact typed
40+
descriptor, so there are no raw-f64 slots whose bits could be misread as a
41+
pointer. `PERRY_LAYOUT_MASK_MIN_SLOTS` overrides the threshold for bisection.
42+
43+
Two details worth keeping:
44+
45+
- An array reports its `length`, but **only for a store into an already-formed
46+
array**. Every append protocol notes the slot *before* bumping `length`, so
47+
mid-construction `length` is the pre-append value; judging on it stranded
48+
every incrementally built array — a `push` loop, a JSON parse — in the scan
49+
state regardless of final size. Capacity is not a substitute either:
50+
`MIN_ARRAY_CAPACITY` is 16, so a one-element literal reports 16 and the
51+
distinction disappears entirely.
52+
- An object reports a bound derived from `GcHeader::size`, not `field_count`,
53+
because `size` is maintained for every GC allocation whatever its
54+
type-specific header holds.
55+
56+
Both directions of error are *correct*, only differently priced: over-estimating
57+
mints a mask that was not needed (the old behaviour), and under-estimating
58+
routes the object to a scan that visits a superset of what the mask would have
59+
selected. Neither can hide a live child.
60+
61+
**Measured** (quiet M1 mini, best-of-5, interleaved against the same binaries
62+
with the policy disabled, outputs byte-identical to node and exit codes checked):
63+
64+
| bench | before | after |
65+
|---|--:|--:|
66+
| `interp` | 1.894 | **1.697** |
67+
| `iso_miss` | 2.371 | **2.157** |
68+
| `bench/mask_tax` (new probe) | 0.1218 | **0.1049** |
69+
| `bench/mask_tax_nopointer` (control) | 0.0929 | 0.0929 |
70+
71+
No regression anywhere on the 19-benchmark corpus, including the GC-heavy
72+
`tree`, `tree_wide`, `retain*`, `cycles` and `deeplist`. The correctness canary
73+
(`iso_miss` printing `checksum 437840 misses 0`) holds plain and under
74+
`PERRY_GC_PROTECT_FROMSPACE=1 PERRY_GC_PROTECT_FROMSPACE_DEPTH=800`,
75+
`PERRY_GC_VERIFY_EVACUATION=1` and `PERRY_GC_FORCE_EVACUATE=1`.
76+
77+
**New probe.** `gc-handoff/bench/mask_tax.ts` reduces the interpreter's
78+
environment chain to the shape that mints the masks, with
79+
`mask_tax_nopointer.ts` as a numeric-element control that holds flat at 1.000.
80+
The arrays have to genuinely escape: a first version kept them in a local,
81+
codegen scalar-replaced the array away, and the probe measured a 1.000 ratio
82+
while the bug was fully intact.
83+
84+
**Left on the table, deliberately.** Raising the threshold to 9 or above pays
85+
roughly twice as much (`interp` 1.619, `iso_miss` 2.046) with still no
86+
regression on the corpus, but 21 tests in this crate encode "a small mixed
87+
payload uses a mask" as a precondition (5 do at 2, 11 at 3, saturating at 21
88+
from 9). That is a contract change worth making deliberately rather than as a
89+
side effect of a perf patch.

crates/perry-runtime/src/gc/layout.rs

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -908,7 +908,13 @@ pub(crate) fn layout_note_slot(parent_user: usize, slot_index: usize, value_bits
908908
if (*header)._reserved & GC_LAYOUT_STATE_MASK != GC_LAYOUT_SIDE_MASK {
909909
set_layout_state(header, GC_LAYOUT_SIDE_MASK);
910910
}
911-
} else if (*header)._reserved & GC_LAYOUT_STATE_MASK == GC_LAYOUT_POINTER_FREE {
911+
} else if (*header)._reserved & GC_LAYOUT_STATE_MASK == GC_LAYOUT_POINTER_FREE
912+
&& !super::layout_tables::layout_prefers_scan_over_mask(
913+
header,
914+
parent_user,
915+
slot_index,
916+
)
917+
{
912918
let mut mask = LayoutSlotMask::Inline(0);
913919
mask.set_slot(slot_index);
914920
masks.insert(parent_user, mask);
@@ -1338,6 +1344,11 @@ pub(super) unsafe fn layout_rebuild_from_slots_with_policy(
13381344
if mask.is_empty() {
13391345
set_layout_state(header, GC_LAYOUT_POINTER_FREE);
13401346
slot_masks_remove(user_ptr as usize);
1347+
} else if slot_count < super::layout_tables::layout_mask_min_slots() {
1348+
// Too few slots for the mask to earn its side-table entry; the
1349+
// tag-checked scan is exact and costs the program nothing globally.
1350+
set_layout_state(header, GC_LAYOUT_UNKNOWN);
1351+
slot_masks_remove(user_ptr as usize);
13411352
} else {
13421353
set_layout_state(header, GC_LAYOUT_SIDE_MASK);
13431354
slot_masks_insert(user_ptr as usize, mask);

crates/perry-runtime/src/gc/layout_tables.rs

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
3232
use super::hot_tls::{hot_layout_slot_masks, hot_per_object_layouts_nonempty, hot_typed_layouts};
3333
use super::layout::{LayoutSlotMask, TypedLayoutDescriptor};
34+
use super::types::{GcHeader, GC_HEADER_SIZE, GC_TYPE_ARRAY, GC_TYPE_OBJECT};
3435
use std::cell::{Cell, RefCell};
3536

3637
thread_local! {
@@ -50,6 +51,71 @@ thread_local! {
5051
pub(in crate::gc) static PER_OBJECT_LAYOUTS_NONEMPTY: Cell<bool> = const { Cell::new(false) };
5152
}
5253

54+
/// Smallest payload slot count for which minting a **per-object pointer mask**
55+
/// is worth its side-table entry. Below it the object takes
56+
/// `GC_LAYOUT_UNKNOWN` — the tag-checked scan-all-slots state — instead.
57+
///
58+
/// The two sides are not symmetric. A mask's benefit is bounded by the object:
59+
/// it can skip at most `slots - pointers` tag checks per trace. Its cost is
60+
/// **program-global and unbounded** — one live entry arms
61+
/// [`PER_OBJECT_LAYOUTS_NONEMPTY`], which puts a two-map hash probe back on
62+
/// every allocation anywhere in the program for as long as that entry lives
63+
/// (see the module docs, and #7510's "one immortal entry nullifies
64+
/// `is_empty()`"). At the bottom of the range the asymmetry is total rather
65+
/// than merely lopsided: over a **single** slot a mask cannot skip anything at
66+
/// all, because the tracer consults `layout_pointer_bearing_bits` on that one
67+
/// slot either way, so the entry is the mask's entire contribution.
68+
///
69+
/// A tag check is exact at both mint sites: neither is reached for an object
70+
/// with an intact typed descriptor, so there are no raw-f64 slots whose bits a
71+
/// tag check could misread as a pointer. #7630 recorded the same conclusion for
72+
/// the materialiser cohort — "a pointer mask can never skip anything a tag
73+
/// check would not reject anyway ... the mask machinery buys nothing here".
74+
///
75+
/// `PERRY_LAYOUT_MASK_MIN_SLOTS` overrides it for bisection.
76+
#[inline(always)]
77+
pub(in crate::gc) fn layout_mask_min_slots() -> usize {
78+
use std::sync::atomic::{AtomicUsize, Ordering};
79+
/// `usize::MAX` = "not yet read from the environment".
80+
static N: AtomicUsize = AtomicUsize::new(usize::MAX);
81+
match N.load(Ordering::Relaxed) {
82+
usize::MAX => {
83+
let v = std::env::var("PERRY_LAYOUT_MASK_MIN_SLOTS")
84+
.ok()
85+
.and_then(|s| s.parse::<usize>().ok())
86+
.unwrap_or(DEFAULT_MASK_MIN_SLOTS);
87+
N.store(v, Ordering::Relaxed);
88+
v
89+
}
90+
v => v,
91+
}
92+
}
93+
94+
/// Only single-slot payloads take the scan. This is deliberately the
95+
/// *provable* end of the range: at one slot the mask demonstrably skips
96+
/// nothing, so no judgement about tracing cost is being made.
97+
///
98+
/// Measured on the 19-benchmark corpus (quiet M1 mini, best-of-5, interleaved
99+
/// against the same binaries with the policy disabled):
100+
///
101+
/// | bench | before | after |
102+
/// |---|--:|--:|
103+
/// | `interp` | 1.894 | **1.697** |
104+
/// | `iso_miss` | 2.371 | **2.157** |
105+
/// | `bench/mask_tax` | 0.1218 | **0.1049** |
106+
/// | `bench/mask_tax_nopointer` (control) | 0.0929 | 0.0929 |
107+
///
108+
/// Every other benchmark — including the GC-heavy `tree`, `tree_wide`,
109+
/// `retain*`, `cycles`, `deeplist` — is unchanged within noise.
110+
///
111+
/// Raising it pays roughly twice as much and costs test churn, both measured:
112+
/// `9` and above gives `interp` 1.619 / `iso_miss` 2.046 with still no
113+
/// regression on the corpus, but 21 tests in this crate encode "a small mixed
114+
/// payload uses a mask" as a precondition (5 do at `2`, 11 at `3`, saturating
115+
/// at 21 from `9`). That is a contract change worth making on purpose rather
116+
/// than as a side effect of a perf patch.
117+
pub(in crate::gc) const DEFAULT_MASK_MIN_SLOTS: usize = 2;
118+
53119
/// True when either per-object side table may hold an entry. `false` is a
54120
/// proof of emptiness (see [`PER_OBJECT_LAYOUTS_NONEMPTY`]); `true` is only a
55121
/// hint, so every caller still has to handle a miss.
@@ -227,3 +293,68 @@ pub(in crate::gc) fn layout_forget_object(user_ptr: usize) {
227293
pub(in crate::gc) fn test_per_object_tables_are_empty() -> bool {
228294
hot_layout_slot_masks().borrow().is_empty() && hot_typed_layouts().borrow().is_empty()
229295
}
296+
297+
/// An upper bound on the payload slots the tracer would enumerate for
298+
/// `user_ptr`, or `usize::MAX` when this module cannot cheaply tell.
299+
///
300+
/// Both directions of error are *correct*, only differently priced, which is
301+
/// what lets this be a bound rather than an exact count: over-estimating mints
302+
/// a mask that was not needed (the pre-existing behaviour), and
303+
/// under-estimating routes the object to `GC_LAYOUT_UNKNOWN`, where the tracer
304+
/// scans every slot and so visits a superset of what a mask would have
305+
/// selected. Neither can hide a live child.
306+
///
307+
/// An array reports its `length` — exactly the range the tracer walks, and so
308+
/// exactly the bound on what a mask could skip — but **only for a store into an
309+
/// already-formed array**. A store at the append position (`slot_index >=
310+
/// length`) reports `usize::MAX` instead, because every append protocol writes
311+
/// the element and notes the slot *before* bumping `length` (see
312+
/// [`layout_all_pointer_array_append`]): mid-construction `length` is the
313+
/// pre-append value, usually 0 or 1, and judging on it would strand every
314+
/// incrementally built array — a `push` loop, a JSON parse — in the scan state
315+
/// no matter how large it eventually grew. Capacity is not a substitute:
316+
/// `MIN_ARRAY_CAPACITY` is 16, so a one-element literal reports 16 and the
317+
/// distinction this is drawing disappears.
318+
///
319+
/// An object reports the bound derived from [`GcHeader::size`] rather than its
320+
/// `field_count`: `size` is maintained for every GC allocation whatever its
321+
/// type-specific header says, so this stays correct for a payload that is not a
322+
/// well-formed `ObjectHeader`, and it errs high — towards the old mask path.
323+
#[inline]
324+
pub(in crate::gc) unsafe fn layout_payload_slot_count(
325+
header: *const GcHeader,
326+
user_ptr: usize,
327+
slot_index: usize,
328+
) -> usize {
329+
match (*header).obj_type {
330+
GC_TYPE_ARRAY => {
331+
let arr = user_ptr as *const crate::array::ArrayHeader;
332+
let length = (*arr).length as usize;
333+
let capacity = (*arr).capacity as usize;
334+
if length > capacity || length > 16_000_000 || slot_index >= length {
335+
usize::MAX
336+
} else {
337+
length
338+
}
339+
}
340+
GC_TYPE_OBJECT => {
341+
let size = (*header).size as usize;
342+
match size.checked_sub(GC_HEADER_SIZE) {
343+
Some(payload) => payload / 8,
344+
None => usize::MAX,
345+
}
346+
}
347+
_ => usize::MAX,
348+
}
349+
}
350+
351+
/// True when `user_ptr` is small enough that a tag-checked scan of every slot
352+
/// beats a per-object pointer mask. See [`layout_mask_min_slots`].
353+
#[inline]
354+
pub(in crate::gc) unsafe fn layout_prefers_scan_over_mask(
355+
header: *const GcHeader,
356+
user_ptr: usize,
357+
slot_index: usize,
358+
) -> bool {
359+
layout_payload_slot_count(header, user_ptr, slot_index) < layout_mask_min_slots()
360+
}

crates/perry-runtime/src/gc/tests/layout_trace/array_layout.rs

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -52,20 +52,25 @@ fn test_layout_mask_overflow_fields_and_array_grow_transfer() {
5252
assert_ne!((*child_header).gc_flags & GC_FLAG_MARKED, 0);
5353
}
5454

55-
let arr = crate::array::js_array_alloc_with_length(1);
55+
// Two elements, not one: a mask over a single-slot payload can skip
56+
// nothing, so `layout_note_slot` now leaves such an array in the
57+
// tag-checked `GC_LAYOUT_UNKNOWN` state and mints no mask to grow or
58+
// transfer. Two slots is the smallest payload that still exercises the
59+
// grow/transfer path this test is about.
60+
let arr = crate::array::js_array_alloc_with_length(2);
5661
crate::array::js_array_set_f64(
5762
arr,
5863
0,
5964
f64::from_bits(STRING_TAG | (child as u64 & POINTER_MASK)),
6065
);
6166
let grown = crate::array::js_array_grow(arr, 128);
62-
assert_eq!(test_layout_pointer_slot_count(grown as usize, 1), Some(1));
67+
assert_eq!(test_layout_pointer_slot_count(grown as usize, 2), Some(1));
6368

64-
let moved = crate::array::js_array_alloc_with_length(1);
69+
let moved = crate::array::js_array_alloc_with_length(2);
6570
unsafe {
6671
layout_transfer(grown as *mut u8, moved as *mut u8);
6772
}
68-
assert_eq!(test_layout_pointer_slot_count(moved as usize, 1), Some(1));
73+
assert_eq!(test_layout_pointer_slot_count(moved as usize, 2), Some(1));
6974

7075
clear_marks();
7176
clear_mark_seeds();
@@ -213,7 +218,13 @@ fn test_array_mixed_bulk_producers_preserve_pointer_layout() {
213218
let set = crate::set::js_set_alloc(4);
214219
let set = crate::set::js_set_add(set, child_box);
215220
let set_arr = crate::set::js_set_to_array(set);
216-
assert_eq!(test_layout_pointer_slot_count(set_arr as usize, 1), Some(1));
221+
// A one-element result carries no mask: over a single slot a mask selects
222+
// exactly what the tracer's tag check already selects, so it is pure
223+
// side-table cost and `layout_note_slot` declines it. What this test is
224+
// actually about — that the bulk producer leaves a layout the tracer can
225+
// follow to the child — is asserted below, unchanged: one slot read, child
226+
// marked.
227+
assert_eq!(test_layout_pointer_slot_count(set_arr as usize, 1), None);
217228
assert_array_root_trace_reads(set_arr, 1);
218229
unsafe {
219230
assert_ne!((*child_header).gc_flags & GC_FLAG_MARKED, 0);
@@ -224,7 +235,11 @@ fn test_array_mixed_bulk_producers_preserve_pointer_layout() {
224235
let map = crate::map::js_map_alloc(4);
225236
let map = crate::map::js_map_set(map, 7.0, child_box);
226237
let entries = crate::map::js_map_entries(map);
227-
assert_eq!(test_layout_pointer_slot_count(entries as usize, 1), Some(1));
238+
// One entry, so the outer array is single-slot and carries no mask for the
239+
// same reason as the set above; the pair it holds is two slots and still
240+
// does. Both are traced either way, which is what the reads assertion and
241+
// the child's mark bit below check.
242+
assert_eq!(test_layout_pointer_slot_count(entries as usize, 1), None);
228243
let pair_box = crate::array::js_array_get_f64(entries, 0);
229244
let pair = (pair_box.to_bits() & POINTER_MASK) as *mut crate::array::ArrayHeader;
230245
assert_eq!(test_layout_pointer_slot_count(pair as usize, 2), Some(1));
@@ -235,14 +250,20 @@ fn test_array_mixed_bulk_producers_preserve_pointer_layout() {
235250
clear_marks();
236251
clear_mark_seeds();
237252

238-
let overwritten = crate::array::js_array_alloc_with_length(1);
253+
// Two slots, so this still goes through the mask: clearing the last
254+
// pointer empties it and restores `GC_LAYOUT_POINTER_FREE`, which is the
255+
// transition being asserted. A single-slot array never mints a mask now,
256+
// and `GC_LAYOUT_UNKNOWN` is one-way — such an array keeps being scanned
257+
// after the pointer is overwritten. That costs one tag check on one slot,
258+
// which is the whole reason the mask was not worth minting for it.
259+
let overwritten = crate::array::js_array_alloc_with_length(2);
239260
crate::array::js_array_set_f64(overwritten, 0, child_box);
240261
assert_eq!(
241-
test_layout_pointer_slot_count(overwritten as usize, 1),
262+
test_layout_pointer_slot_count(overwritten as usize, 2),
242263
Some(1)
243264
);
244265
crate::array::js_array_set_f64(overwritten, 0, 99.0);
245-
assert_numeric_array_trace_free(overwritten, 1);
266+
assert_numeric_array_trace_free(overwritten, 2);
246267

247268
clear_marks();
248269
clear_mark_seeds();

0 commit comments

Comments
 (0)