Skip to content

Commit ffb765b

Browse files
committed
Amortize arena free-run ownership checks
1 parent 5471488 commit ffb765b

5 files changed

Lines changed: 323 additions & 29 deletions

File tree

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
# Packed arena range preflight
2+
3+
## Motivation
4+
5+
The pushed typed-sweep amortization checkpoint `54714887` reduced repeated
6+
quarantine visits and global certificates, but a fresh loop-only hardware
7+
profile still measured 405.96 ns per `closures_upval` operation. The remaining
8+
free-run reconstruction path was the clearest isolated target:
9+
10+
```text
11+
arena_clear_extent_range 51.42 ns/op
12+
arena_set_free_run 32.02 ns/op
13+
lj_arena_scan_free_runs 13.95 ns/op
14+
total 97.39 ns/op
15+
```
16+
17+
The first two functions repeatedly loaded root, recovery, destructor, READY,
18+
and lifetime state for every cell in a large free span. The same metadata is
19+
already stored in packed atomic planes, so this was observation overhead rather
20+
than useful semantic work.
21+
22+
## Exact packed proof
23+
24+
`arena_range_ownership_preflight()` now walks only the bitmap words intersected
25+
by the requested range. Existing exact converters map every nonzero root,
26+
recovery, and destructor state and every non-FREE lifetime nibble back to one
27+
bit per arena cell. Each word is intersected with a partial-range mask, so cells
28+
before or after the requested span cannot veto it.
29+
30+
This is only an amortized form of the previous acquire observations. It does
31+
not authorize mutation, replace the owner/open-or-closed generation protocol,
32+
or remove any later validation. `arena_set_free_run()` still completes its
33+
entire first preflight before changing metadata, scrubs READY/cdata/destructor
34+
coverage, and calls the stricter `arena_clear_extent_range()` preflight again
35+
before removing interior boundaries.
36+
37+
The two historical policies remain distinct:
38+
39+
- `arena_set_free_run()` treats recovery ownership as an ordinary fail-closed
40+
veto and returns without mutation;
41+
- `arena_clear_extent_range()` retains its corruption assertion and abort when
42+
recovery is the first cell-ordered blocker.
43+
44+
The cold fatal path also preserves the old diagnostic order inside a packed
45+
word. An ordinary blocker in a lower cell returns first; recovery at the same
46+
or an earlier cell remains fatal. Zero-length free-run publication is now
47+
rejected explicitly before end arithmetic or structural mutation. Other
48+
invalid bounds are rejected before an array access.
49+
50+
READY deliberately differs between the two callers. An old typed READY bit
51+
vetoes direct extent removal, while free-run publication is allowed to scrub
52+
READY after lifetime FREE and zero destructor identity. Its second interior
53+
preflight then observes the scrubbed plane exactly as before.
54+
55+
## Race and ordering argument
56+
57+
The packed loads use the same acquire order class as the former per-cell
58+
accessors. Release publication of `block[]` changes and the atomic relaxed AND
59+
on concurrently markable `mark[]` are unchanged. A racing allocation,
60+
recovery, root, or remote-free publication is still governed by its lifetime
61+
claim and the surrounding arena generation. Grouping reads by word does not
62+
make a stale snapshot commit authority; later validation and the exact
63+
open/commit generation continue to reject a conflicting publisher.
64+
65+
For a valid nonzero range, the new predicate is logically identical to the old
66+
one:
67+
68+
- every root/recovery pair and lifetime nibble is converted exactly;
69+
- every destructor plane is ORed into cell geometry;
70+
- READY is included only for the caller which historically required it; and
71+
- plain arenas continue to omit the traversable-only lifetime plane.
72+
73+
An independent concurrency/source audit found no unsafe-positive race or
74+
memory-order weakening after the recovery-policy, zero-length, and cold-order
75+
hardening.
76+
77+
## Deterministic coverage
78+
79+
An `LJ_ARENA_TEST_HELPERS` wrapper calls the actual static free-run function;
80+
it is absent from production builds and changes no public ABI. The focused
81+
arena-sweep fixture uses one 20-cell range spanning both packed lifetime
82+
positions 15/16 and bitmap positions 31/32. Whole-arena snapshots prove that:
83+
84+
- zero-length and invalid-bound calls fail without mutation;
85+
- root, recovery-PENDING, destructor, and non-FREE lifetime blockers each fail
86+
without changing any arena byte;
87+
- recovery remains a nonfatal veto on the set-free path;
88+
- blockers immediately outside the partial range do not leak through its
89+
masks; and
90+
- a valid run scrubs READY/cdata, removes all interior block/mark boundaries,
91+
and publishes exactly one mark-only free-run start.
92+
93+
The fixture compiles with `-Wall -Wextra -Werror` and passed.
94+
95+
## Validation
96+
97+
All requested forced-clean gates passed:
98+
99+
- `m2_arena_sweep`, before and after the direct packed-boundary fixture;
100+
- `m2_arena_gcsweep`;
101+
- `m3_gc2_recovery` in normal and assertions/GC2-paranoia builds; and
102+
- `m3_gc2_paranoia`, including all C oracles, 509/509 JIT tests, and 387/387
103+
no-JIT tests.
104+
105+
Every configuration-changing run restored the default build. The only known
106+
compiler diagnostic was the pre-existing GCC inlining warning around
107+
`gc2_root_rescan_later`/`la_load32_acq`.
108+
109+
## Performance
110+
111+
Five fresh independent processes with the valid stopped-GC wrapper measured:
112+
113+
```text
114+
current active: 319.30, 317.84, 318.53, 320.70, 323.47; median 319.30 ns/op
115+
current stopped: 94.83, 94.02, 95.16, 94.28, 95.99; median 94.83 ns/op
116+
stock active: 37.51, 38.45, 36.98, 39.65, 38.28; median 38.28 ns/op
117+
stock stopped: 18.35, 21.46, 21.04, 20.61, 21.70; median 21.04 ns/op
118+
```
119+
120+
The final zero-length/diagnostic hardening was performance-neutral in a
121+
three-process confirmation: 319.44 ns/op active and 94.14 ns/op stopped.
122+
123+
Against pushed `54714887` (389.19 active, 113.31 stopped), the packed preflight
124+
improves active time by 17.96%, stopped time by 16.31%, and active-minus-stopped
125+
cost by 18.63%. It is 34.68% faster than the earlier 488.80 ns/op typed-body
126+
checkpoint. The main five-process medians remain 8.34x stock active and 4.51x
127+
stock stopped, so b1.2.0 is still performance-blocked.
128+
129+
The next profile-directed tranche is an independently revalidated terminal
130+
FREE/FREEING word fast path in quarantine bitmap readiness/application,
131+
followed by all-free live-cell counting/adoption if the new profile still
132+
justifies it.

src/lj_arena.c

Lines changed: 72 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -3629,28 +3629,70 @@ static uint64_t arena_range_mask(uint32_t lo, uint32_t nbits)
36293629
(((uint64_t)1 << nbits) - 1u) << lo;
36303630
}
36313631

3632-
static int arena_clear_extent_range(GCArena *a, uint32_t start, uint32_t len)
3632+
/* Prove that every cell in a structural-reuse range is free of persistent
3633+
** ownership metadata. The packed-to-block converters are exact: any nonzero
3634+
** root/recovery/dtor state and every non-FREE lifetime nibble maps to its
3635+
** allocation cell. The surrounding owner-open/closed generation protocol is
3636+
** unchanged; this summary merely groups the former acquire observations and
3637+
** neither authorizes mutation nor replaces any later validation.
3638+
**
3639+
** READY is authoritative for arena_clear_extent_range(), where an old typed
3640+
** publication must veto boundary removal. arena_set_free_run() deliberately
3641+
** omits it: that path has already reached lifetime FREE with no destructor
3642+
** identity and defensively scrubs READY before its second, stricter interior
3643+
** validation through arena_clear_extent_range(). */
3644+
static LJ_AINLINE int arena_range_ownership_preflight(GCArena *a,
3645+
uint32_t start, uint32_t len, int require_ready_clear,
3646+
int abort_on_recovery)
36333647
{
36343648
uint32_t pos, end;
3649+
int lifetime_managed;
36353650
if (start >= LJ_ARENA_CELLS || len > LJ_ARENA_CELLS - start)
36363651
return 0;
36373652
end = start + len;
3638-
/* Keep the exact per-cell ownership precondition of arena_set_extent().
3639-
** Validate the complete range before changing either bitmap, so a failed
3640-
** side-plane check leaves every old boundary intact. */
3641-
for (pos = start; pos < end; pos++) {
3642-
uint32_t recovery = lj_arena_recovery_state_acq(a, pos);
3643-
lj_assertX(recovery == LJ_ARENA_RECOVERY_IDLE,
3644-
"arena extent reuse crossed recovery ownership");
3645-
if (recovery != LJ_ARENA_RECOVERY_IDLE)
3646-
abort();
3647-
if (lj_arena_root_state_acq(a, pos) != LJ_ARENA_ROOT_NONE ||
3648-
lj_arena_dtor_kind_acq(a, pos) != LJ_ARENA_DTOR_NONE ||
3649-
lj_arena_ready_get(a, pos) ||
3650-
(arena_lifetime_managed(a) &&
3651-
lj_arena_lifetime_state_acq(a, pos) != LJ_ARENA_LIFETIME_FREE))
3653+
lifetime_managed = arena_lifetime_managed(a);
3654+
for (pos = start; pos < end; ) {
3655+
uint32_t wi = pos >> 6;
3656+
uint32_t lo = pos & 63u;
3657+
uint32_t take = end - pos;
3658+
uint32_t room = 64u - lo;
3659+
uint64_t mask, recovery, blockers;
3660+
if (take > room)
3661+
take = room;
3662+
mask = arena_range_mask(lo, take);
3663+
recovery = arena_recovery_block_bits(a, wi) & mask;
3664+
blockers = arena_root_block_bits(a, wi) |
3665+
arena_dtor_block_bits(a, wi);
3666+
if (lifetime_managed)
3667+
blockers |= arena_lifetime_block_bits(a, wi);
3668+
if (require_ready_clear)
3669+
blockers |= la_load64_acq(&a->ready[wi]);
3670+
blockers &= mask;
3671+
if (recovery != 0 && abort_on_recovery) {
3672+
uint64_t first_recovery = recovery & (0u - recovery);
3673+
/* Preserve the old cell-order diagnostic policy on the cold corrupt
3674+
** path: an ordinary blocker in an earlier cell returns fail-closed;
3675+
** recovery at the same or an earlier cell remains a fatal invariant. */
3676+
if ((blockers & (first_recovery - 1u)) == 0) {
3677+
lj_assertX(0, "arena extent reuse crossed recovery ownership");
3678+
abort();
3679+
}
3680+
}
3681+
if (recovery != 0 || blockers != 0)
36523682
return 0;
3683+
pos += take;
36533684
}
3685+
return 1;
3686+
}
3687+
3688+
static int arena_clear_extent_range(GCArena *a, uint32_t start, uint32_t len)
3689+
{
3690+
uint32_t pos, end;
3691+
/* Validate the complete range before changing either bitmap, so a failed
3692+
** side-plane check leaves every old boundary intact. */
3693+
if (!arena_range_ownership_preflight(a, start, len, 1, 1))
3694+
return 0;
3695+
end = start + len;
36543696
/* The complete preflight proves this is already opaque reusable storage:
36553697
** READY and dtor are zero, every lifetime lane is FREE, and no root/recovery
36563698
** owner exists. Remove old structural boundaries with release stores;
@@ -3751,19 +3793,15 @@ static int arena_set_alloc(GCArena *a, uint32_t cell, uint32_t ncells,
37513793

37523794
static int arena_set_free_run(GCArena *a, uint32_t start, uint32_t len)
37533795
{
3754-
uint32_t i, pos = start, end = start + len;
3755-
if (arena_lifetime_managed(a) &&
3756-
lj_arena_lifetime_state_acq(a, start) != LJ_ARENA_LIFETIME_FREE)
3796+
uint32_t pos, end;
3797+
/* This is the complete pre-mutation ownership proof. READY is intentionally
3798+
** scrubbed below; arena_clear_extent_range() then repeats the stricter
3799+
** ownership validation over every interior cell before boundary removal. */
3800+
if (len == 0 ||
3801+
!arena_range_ownership_preflight(a, start, len, 0, 0))
37573802
return 0;
3758-
for (i = 0; i < len; i++)
3759-
if (lj_arena_root_state_acq(a, start + i) != LJ_ARENA_ROOT_NONE ||
3760-
lj_arena_recovery_state_acq(a, start + i) !=
3761-
LJ_ARENA_RECOVERY_IDLE ||
3762-
lj_arena_dtor_kind_acq(a, start + i) != LJ_ARENA_DTOR_NONE ||
3763-
(i != 0 && arena_lifetime_managed(a) &&
3764-
lj_arena_lifetime_state_acq(a, start + i) !=
3765-
LJ_ARENA_LIFETIME_FREE))
3766-
return 0;
3803+
pos = start;
3804+
end = start + len;
37673805
while (pos < end) {
37683806
uint32_t wi = pos >> 6;
37693807
uint32_t lo = pos & 63u;
@@ -3783,6 +3821,13 @@ static int arena_set_free_run(GCArena *a, uint32_t start, uint32_t len)
37833821
return 1;
37843822
}
37853823

3824+
#if defined(LJ_ARENA_TEST_HELPERS)
3825+
int lj_arena_test_set_free_run(GCArena *a, uint32_t start, uint32_t len)
3826+
{
3827+
return arena_set_free_run(a, start, len);
3828+
}
3829+
#endif
3830+
37863831
static int arena_link_run_head(TGAlloc *alloc, GCArena *a, uint32_t start,
37873832
uint32_t len)
37883833
{

src/lj_arena.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -729,6 +729,8 @@ LJ_FUNC uint32_t lj_arena_test_remote_drain_paused(void);
729729
LJ_FUNC void lj_arena_test_remote_stats_reset(void);
730730
LJ_FUNC uint64_t lj_arena_test_remote_fast_skips(void);
731731
LJ_FUNC uint64_t lj_arena_test_remote_arena_probes(void);
732+
LJ_FUNC int lj_arena_test_set_free_run(GCArena *a, uint32_t start,
733+
uint32_t len);
732734
#endif
733735
#if defined(LJ_ARENA_TEST_HELPERS) || defined(LJ_GC2_TEST_HELPERS)
734736
LJ_FUNC void lj_arena_test_lifetime_pause(int enabled);

tests/suites/m2_arena.lua

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -119,8 +119,12 @@ return function(add)
119119
name = "m2_arena_sweep",
120120
description = "owner-local arena sweep scaffold C fixture",
121121
run = function(t)
122-
run_standalone_fixture(t, t:tmp("lj_t_arena_sweep"),
123-
"t-arena-sweep.c")
122+
compile_and_run_sources(t, t:tmp("lj_t_arena_sweep"),
123+
arena_sources(t, "t-arena-sweep.c"), {
124+
cflags = "-DLUAJIT_SECURITY_PRNG=0 -DLJ_ARENA_TEST_HELPERS",
125+
link_luajit = false,
126+
libs = {}
127+
})
124128
end
125129
})
126130

0 commit comments

Comments
 (0)