Merge train: #9808, #9818, #9819, #9820, #9822, #9823, #9826 - #9866
Conversation
… advertises a proof `transfer_element_shape` runs for every relocated array and already decides `had_bit` from header words it has read anyway — then took the side table's `RefCell` and hashed both addresses regardless, for two removes that remove nothing whenever the source proved nothing and the destination advertises nothing. It now returns before the table in that case. The gate cannot be `!had_bit` alone: a destination still advertising a proof describes storage the move has just replaced, so that case keeps the full fail-closed path. Both halves are pinned by a new test, which fails on its named assertion if the gate is widened. Leaving a record behind at an address whose bit is clear is not a new state: the bit is the sole authority for a read, `establish` draws identities from `ELEMENT_SHAPE_PROOF_SEQ` rather than from the record at the address, and `prune_dead_element_shape_owners` drops it on the next collection — the same guarantees a fail-closed transfer already depended on. Claude-Session: https://claude.ai/code/session_014UZWia6L37DpA93VLtNK9m
…has a key to filter
`js_for_in_keys_value` maintained a `HashSet<String>` of every own name at
every prototype level so that a name owned closer to the receiver hides the
same name further along the chain (ECMA-262 14.7.5, 12.6.4-2). It built that
set unconditionally: at every level it materialised a SECOND key array (all
own names, including non-enumerable ones) on top of the enumerable one, and
turned every name at every level into a heap `String` so it could be hashed
into the set.
The set can only ever filter a level >= 1, and a level that contributes no
enumerable keys of its own never consults it. So the set is now built on
demand, at the moment a level >= 1 actually has an enumerable key, from
exactly the levels already walked — which is the same content the eager
version held at that point, so the emitted key sequence is unchanged.
Measured with the new `PERRY_ENUM_DIAG`, one 400-character reply through the
compiled claude-code TUI, one binary and one environment variable apart:
eager (today) deferred
for-in calls 17,281 17,266
key arrays 69,124 34,532 4.00 -> 2.00 per call
String allocs 159,947 0
seen.insert 159,947 0 (SipHash of the whole key)
keys emitted 11,342 11,246
emitted at proto level >=1 0 0
shadow set built - 0 times
**Not one key in 17,281 `for-in` loops came from a prototype level**, so the
159,947 `String` allocations and 159,947 hash inserts filtered nothing at all.
Half the key arrays go with them: the all-own-names array is materialised only
once the set is live.
Those `String`s are 1.91 MB in total, which is why no allocation-byte ranking
found this — the cost is 160k mallocs, memcpys, hashes and frees, not the
bytes. Collection schedule is unchanged as predicted for a category this small
(41 vs 43 copying minors, 46 vs 48 budgeted full-cycle steps).
`VisitedLevels` keeps the walked levels inline (8 against a measured 2.00 per
call) so the rebuild's bookkeeping does not reintroduce one allocation per
`for-in` in place of the ones removed.
`PERRY_FORIN_LAZY_SHADOW=0` restores the eager path, so both live in one
binary and the A/B above is one environment variable.
Three tests, each verified to fail under sabotage: deleting the deferred build
fails two of them by name, and dropping the spill fails the third. The third
had to be rewritten to do so — its first version put the shadowing property on
every level, so the leaf still shadowed the name and deleting the spill changed
nothing.
Claude-Session: https://claude.ai/code/session_014UZWia6L37DpA93VLtNK9m
The first version of `only_a_spilled_level_shadows_the_root...` gave every level the shadowing property, including the leaf. Deleting the spill arm left it passing, because the leaf's own copy shadowed the root's on its own: the assertion was true regardless of what the spill did. The doc comment now carries that reasoning, and the general rule behind it, so the test cannot be 'simplified' back into one that cannot fail. Claude-Session: https://claude.ai/code/session_014UZWia6L37DpA93VLtNK9m
`js_regexp_new` materialized the canonical flags twice on every construction — a Rust `String` from `validate_and_canonicalize_flags`, and a fresh GC `StringHeader` for `flags_ptr` — and a JS regex literal constructs a fresh object every time it is evaluated. `PERRY_REGEX_DIAG` counts 161,897 constructions per 400-character claude-code reply: ~5.2 MB of identical one- and two-byte GC strings, ~44 MB on a 3300-character reply, ~1.4 M allocations. There are eight legal flags and each may appear once, so the canonical form is at most eight ASCII bytes and now lives inline in `CanonicalFlags`. JS strings are immutable and have no identity semantics, so when the caller's flags text already IS the canonical text — a literal, whose flags the author wrote in spec order — the header shares the caller's string instead of duplicating it. Nothing downstream depends on the pointer being fresh: `flags_ptr`-keyed lookups read it through `string_as_str` and compare content. GC safety: the comparison and the root are taken BEFORE the validation block, because `raw_flags_str` borrows the caller's GC string and that block can allocate — the same hazard the ★ note on `pattern_root` describes, and the same one #7341 fixed for the freshly-allocated flags string. The existing re-read from `flags_root` after `gc_malloc` covers both arms unchanged. Below the campaign's ~10 % line at ~2-3 % of arena traffic per turn, so the cc rig is expected to read flat; the counter is the proof, not the benchmark. Claude-Session: https://claude.ai/code/session_014UZWia6L37DpA93VLtNK9m
Rename only — the fragment was written before the PR number was known. Claude-Session: https://claude.ai/code/session_014UZWia6L37DpA93VLtNK9m
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (47)
📝 WalkthroughWalkthroughThe pull request updates documentation-test requirements, GC evacuation verification, primitive string property access, enumeration behavior, regex flag allocation, diagnostics, and codegen evidence records. It adds targeted runtime, harness, and regression tests. ChangesDocumentation test requirements
Garbage-collection verification
Primitive string property reads
Enumeration and diagnostics
Regular-expression flag allocation
Codegen mechanism evidence
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant DocExample
participant DocTests
participant Compiler
DocExample->>DocTests: provide requires: auto-optimize
DocTests->>Compiler: compile with auto-optimize enabled
Compiler-->>DocTests: pass or compile_fail
DocTests-->>DocExample: include result in CI report
sequenceDiagram
participant GCPhase
participant EvacuationVerifier
participant RootScanner
GCPhase->>EvacuationVerifier: select verification mode
RootScanner->>EvacuationVerifier: inspect forwarded value
EvacuationVerifier-->>RootScanner: accept retained alias or reject stale hop
✨ 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 |
Merge train: #9808, #9818, #9819, #9820, #9822, #9823, #9826 — seven PRs, several of which conflicted against work that landed earlier today.
Conflict resolutions
NEVER_MATCH_PATTERNand the cache-key type, because train Merge train: #9794, #9780, #9774, #9814, #9756, #9796, #9775 (all previously conflicting) #9863 split those intoregex/program_key.rs. Took HEAD so they are not duplicated; verified the sibling still defines them exactly once.string_payload_access_baseline.txt, a generated ratchet baseline. Regenerated rather than merged.gc_runtime_root_holders.json. Merging it as an entry-list union silently restored that branch's olderPASS1_MARKEDwindow pins overmain's current ones. Union is right for holder entries and wrong for a pin, which has exactly one correct value per file; all three were recomputed from the tree. fix(gc): allow retained array-growth aliases during copying verification #9822 does not modify any window file, so the audits already recorded on that entry still stand.Gate work
tls-budgetbreak that train Merge train: #9794, #9780, #9774, #9814, #9756, #9796, #9775 (all previously conflicting) #9863 itself shipped:gc/diag_sites.rscarried a rawthread_local!. Converted, socheck_thread_localsis green again.ENUM_DIAGclassified — perf(enum): for-in builds its shadow set only when a prototype level has a key to filter #9823's for-in diagnostics are twoInstants and u64 counters, no addresses.enumeration.rscrossed the 2000-line gate at 2247, so its test module moved toenumeration_tests.rsandjs_object_entries_shapetoentries_shape.rs.enumeration.rs4→3 and 3→2, one of each accounted toentries_shape.rs, counts verified against the relocation so the totals are unchanged.Validation
64/64 lint gates;
perry-runtime,perry-codegen,perry-hir,perry-stdlib,perry-transform— all green, run serially (RUST_TEST_THREADS=1), 0 failures.Summary by CodeRabbit
Bug Fixes
Performance
for-inenumeration.Documentation