Merge train: #9870, #9872, #9876, #9879, #9880, #9882 - #9888
Merged
Conversation
(cherry picked from commit 63d0004)
(cherry picked from commit b13ab40)
(cherry picked from commit e44c369)
(cherry picked from commit 14041aa)
(cherry picked from commit 673e83b)
(cherry picked from commit 7d4865e)
(cherry picked from commit 8022034)
(cherry picked from commit d324285)
(cherry picked from commit 574e6fb)
`call_overridden_iterator_next` minted a fresh 4-byte "next" key string on
every built-in iterator step, purely to run a by-name prototype lookup that
concluded nothing was patched. The `ITERATOR_PROTOTYPE_PTR == 0` early-out
that was supposed to prevent this is dead after the first iterator any
program allocates: every iterator allocator calls `attach_iterator_prototype`
-> `ensure_iterator_prototypes`, which materializes the tower.
Adds `prototype_next_is_canonical`: the prototype's own `next` slot holds a
closure whose native entry is the canonical thunk, and no accessor descriptor
is recorded for "next". Both reads are non-allocating. Any other state falls
through to the by-name path, unchanged.
This is the third-ranked site by count in the 2026-09-06 claude-code
allocation census (~122,880 x 32 B per 400-character reply), which had
attributed it to `Intl.Segmenter` substring copying. Caller walk in the
shipped binary `cc_relink/cc_int_0905`:
js_for_of_next+0xd0
-> dispatch_array_iterator_method_inner+0x218 (bl call_overridden_iterator_next)
-> call_overridden_iterator_next+0x67c (bl js_string_from_bytes_with_capacity)
-> string_storage_alloc
Measured on a relinked claude-code binary carrying this fix plus a
measurement-only hit/miss counter. Before the fix every probe allocated, so
`hits + byname` is the pre-fix count and `byname` is what survives:
400-char reply, run A 144,189 probes byname 0
400-char reply, run B 144,303 probes byname 0
3300-char reply 887,076 probes byname 0
`byname = 0` on every one of the 173 per-minor reports across the three runs:
the proof answers 100 % of probes on a real program, which is what rules out
the one silent failure mode (the accessor half is a per-key Bloom bit, so a
colliding accessor on the prototype would disable the fast path with no test
failing).
`cargo test -p perry-runtime --release --lib -- --test-threads=1`: 3,171
passed, 0 failed. Four sabotage arms, each failing only its named assertion:
removing the fast path entirely reads exactly 32,000 bytes over 1,000 probes;
dropping only the accessor half fails only the accessor test; dropping only
the native-entry comparison fails only the replaced-`next` test.
(cherry picked from commit 8f4f87a)
An integration arm for the allocation-free proof: compiles
`test-files/test_gap_iterator_prototype_next_patch.ts` and byte-compares
stdout against node v26.5.1, captured 2026-09-06 on this box.
Three of the lines are the ones that can only pass if the proof is exactly
right:
F-bound-copy 100,200 a `bind` of the original has the SAME native entry as
the builtin thunk but a different `this`; a proof that
compared native entries without first reading the
prototype's own slot would print `1,2`.
G-accessor 1,2 true `defineProperty(proto,"next",{get})` leaves the old
closure in the data slot, so the own read alone still
sees the canonical closure — only the per-key accessor
Bloom bit makes the proof decline.
H true a deleted `next` must throw a TypeError, never fall
through to the builtin advance.
(cherry picked from commit e126360)
The allocation-free proof reads the prototype's own `next` slot as a RAW value before deciding anything, so a number, a string, `undefined`, `null` and a plain object each have to defeat it and throw a TypeError rather than be mistaken for the builtin closure. Node v26.5.1 throws for all five; pinned in the integration arm. (cherry picked from commit 90ebcf6)
…ialising it
Five `#[no_mangle]` entry points that let a compiled
`for (let {segment: O} of X.segment(q))` loop read what it needs from a cursor
over the input instead of building a record and a substring per grapheme:
`open`, `next`, `code_point_at`, `segment` (materialise-on-miss) and
`regexp_test`. The interface is `INTERFACE_segments_view.md` §9, agreed with
the compiler lane whose lowering is PR #9859.
Nothing here constructs a `Segments`: `open` takes the segmenter and the input.
That is why the view mode did not depend on the lazy-`Segments` change measured
and refuted separately — `build_segments` stays eager and simply stops being
reached for the loop that matters.
The cursor is an ordinary GC object whose slot 0 holds the input as a traced
value, so the collector rewrites it like any other field: no registered root, no
side table, no new scanner, and no new rooting rule for codegen. Every entry
point re-derives its `&str` at entry and drops it before returning; `next` and
`code_point_at` allocate nothing at all.
Three contracts that are easy to get subtly wrong, so each has a test:
* `open` declines with NO observable effect and in a fixed order — in
particular an input that is not already a string primitive is refused BEFORE
any coercion, because `build_segments` runs user `toString` and throws on a
Symbol, and the compiler evaluates `X.segment(q)` itself on a decline.
* `code_point_at`'s `k` is bounded by the SEGMENT, not the input: `k` past the
segment end is `undefined` even though the input has more code units there. A
view that clamped to the input would silently answer the next grapheme.
* `regexp_test` matches a bounded haystack whose bounds ARE the string's ends,
so `^`/`$`/lookbehind stay segment-local. It declines (three-valued
`undefined`) for a global or sticky regex, whose `test` is stateful in
`lastIndex`, and for a patched `RegExp.prototype.test`.
Tests: 8 unit tests including the falsifier — 200 `next` + `code_point_at`
steps move `arena_in_use_bytes` by ZERO with the minor-cycle count pinned — and
a walk compared against `graphemes(true)` on combining marks, a ZWJ sequence and
a regional-indicator pair. `cargo test -p perry-runtime --release --lib`: 3,179
passed, 0 failed.
Two sabotage arms, and one of them refused to fire, which is reported rather
than hidden: replacing the bounded haystack with a start offset FAILS
`regexp_test_matches_the_materialised_call_...`, so that contract is proven
load-bearing; storing the pre-allocation input value instead of re-reading the
rooted handle passes everything, including under `PERRY_GC_SCHEDULE_RATE=1`,
because `arena_alloc_gc` does not poll the collector — `gc_check_trigger()` runs
at a handful of explicit sites and arena allocation is not one of them. The
rooting stays as defensive practice; it is NOT demonstrated to be load-bearing,
and the interface says so.
Claude-Session: https://claude.ai/code/session_014knX724SYDogwzsXybCGxp
(cherry picked from commit 5a48920)
The compiler emits calls to `js_segments_view_*` only when the tier fires, so without a reference the bundle link's stub localization can drop them before the lowering that needs them is ever compiled. Same reason and same shape as `KEEP_JS_FOR_OF_NEXT` in `collection_iter_object.rs`. Claude-Session: https://claude.ai/code/session_014knX724SYDogwzsXybCGxp (cherry picked from commit fe0576a)
…ew-mode regex path #9870 inserted `regexp_test_str_bounded` between `js_regexp_test` and its own `#[cfg(feature = "regex-engine")]` + `#[no_mangle]`, so both attributes silently re-targeted onto the new function. Two consequences, neither visible to a workspace build (feature unification turns the engine on): `js_regexp_test` lost its gate and failed to compile without the engine, and a `pub(crate)` Rust fn picked up a `#[no_mangle]` it must not have. Attributes reattached to the function each belongs to. `js_segments_view_regexp_test` reaches two engine-gated helpers. It is `#[no_mangle]`, so it cannot itself be gated out — the symbol has to exist in every configuration or a binary emitting a call fails to link. Its regex-dependent path is gated instead, and the engine-off arm declines, which is the same contract its other declines already have. Also classifies #9870's eight new PERRY_SEGVIEW_DIAG counters as not_a_gc_pointer: plain AtomicU64 tallies written only via fetch_add.
|
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 (27)
📝 WalkthroughWalkthroughThis PR adds scheduled gate-failure issue tracking, an ChangesGate failure monitoring
Intl.Segmenter view mode
Live module bindings
Mock timer validation
Node and MIME runtime compatibility
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant segments_view
participant RegexEngine
Caller->>segments_view: open segmenter and input
segments_view-->>Caller: return traced cursor
Caller->>segments_view: advance or read code point
segments_view-->>Caller: return view result
Caller->>segments_view: test regexp against segment
segments_view->>RegexEngine: execute bounded test
RegexEngine-->>segments_view: return match result
segments_view-->>Caller: return boolean or undefined
sequenceDiagram
participant WorkflowRun
participant ObserverWorkflow
participant GitHubAPI
participant FailureIssue
WorkflowRun->>ObserverWorkflow: completion event
ObserverWorkflow->>GitHubAPI: fetch jobs and run history
GitHubAPI-->>ObserverWorkflow: failure data
ObserverWorkflow->>FailureIssue: create, update, reopen, or close
Possibly related PRs
Suggested labels: ✨ 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 |
This was referenced Sep 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Merge train: #9870, #9872, #9876, #9879, #9880, #9882.
A latent bug in #9870 that no workspace build could see
#9870 inserted
regexp_test_str_boundedbetweenjs_regexp_testand its own#[cfg(feature = "regex-engine")]+#[no_mangle], so both attributes silently re-targeted onto the new function.js_regexp_testlost its gate and no longer compiled without the engine, and apub(crate)Rust fn picked up a#[no_mangle]it must not have.cargo check --workspace --all-targetsis green on this, because feature unification turnsregex-engineon across the workspace and the gate never matters. Onlycargo test -p perry-stdlibin isolation fails. Attributes are reattached to the function each belongs to.Relatedly,
js_segments_view_regexp_testreaches two engine-gated helpers. It could not simply be gated with them: it is#[no_mangle], so the symbol must exist in every configuration or a binary that emits a call to it fails to link. Its regex-dependent path is gated instead and the engine-off arm declines — the same contract its other declines already have.#9870's eight new
PERRY_SEGVIEW_DIAGcounters are classifiednot_a_gc_pointer(plainAtomicU64, written only throughbump()'sfetch_add).Audit notes
ci: open issues for unattended scheduled gate failures #9880 is a
workflow_runobserver, which receives a write-capable token — the shape that usually goes wrong. It checks outref: mainrather than the triggering workflow's head, scopes permissions toactions: read/contents: read/issues: write, pins the checkout action by SHA, setspersist-credentials: false, and usescancel-in-progress: falsesomainruns queue rather than cancelling one another. The script passes argv as a list (noshell=True) and dedupes on a per-workflow title marker, PATCHing the existing issue instead of opening one per failure.feat(intl): Segments view mode — answer a grapheme loop without materialising a record or a substring #9870's segmenter cursor is a GC
ObjectHeader, not a Rust struct, andwith_inputre-derives its&strper call with a non-allocating callee — no borrowed heap slice outlives a collection point. Inert until feat(codegen): match the Intl.Segmenter for-of and answer it from the runtime view mode (default OFF) #9859 lands.fix(node:test): align mock timer validation #9872 and fix(node:v8): require new for class exports #9876 are parity fixes measured against the pinned Node 26 oracle (fix(node:test): align mock timer validation #9872 at 14/14).
fix(dynamic-import): preserve aliased live bindings #9879 correctly makes aliased dynamic imports LIVE, which is the semantics that matters. It implements that by turning namespace exports into accessor properties, so
Object.getOwnPropertyDescriptor(ns, "x")now reportsget/setwhere the spec (and Node) report a data descriptor withvalue/writable. The gap test covers liveness under GC stress but not descriptor shape. Landing it as a clear net improvement, with the narrower deviation tracked separately.Conflict resolutions
#9870's iterator-prototype commits conflicted with the fuller version of the same work that landed via #9848;
main's side kept, including its extra non-callable-nextcase.Validation
run_lint_gates: all 64 gates passed; 2 CI-only skippedSummary by CodeRabbit
New Features
Intl.Segmentergrapheme view mode for segment navigation, code-point lookup, segment retrieval, and bounded regular-expression testing.Bug Fixes
node:v8classes.essencesynchronized.Documentation