chore: merge train 221 (v0.5.1599) - #10729
Merged
Merged
Conversation
…ched via require()
A CommonJS-wrapped module runs its whole body inside the wrap's IIFE, so
every top-level const -- including const { AsyncResource } =
require("node:async_hooks") -- is a genuine local. Class-heritage
resolution's locally_shadowed check (perry-hir/src/lower_decl/class_decl.rs)
could not distinguish that from a real user shadow (const EventEmitter =
MyOwnClass), so it always fell back to the dynamic extends_expr /
js_fetch_or_value_super dispatch and lost the native base's install +
argument forwarding.
For bases whose runtime value is a genuine ES class (AsyncResource,
AsyncLocalStorage), that dynamic dispatch calls the value without new and
throws. For bases backed by an old-style function (EventEmitter, node:stream
classes) it happens to complete, but through a far more expensive indirect
path.
Record require()-destructured bindings' provenance (local name -> export
key) unconditionally in var_decl_sources.rs, regardless of the #8342
CJS-wrapper gate that skips the full native-module-alias registration for
the same binding, and consult it from both class-heritage arms
(declaration and expression) so a local is only treated as shadowing when
it did NOT come from a require() of the real native module.
crates/perry-hir/src/lower/context.rs was exactly at the 2000-line file cap;
the new field's init line tipped it over. Split LoweringContext::new /
with_class_id_start[_salted] into a new sibling file (pure relocation, no
logic change).
Rebasing #10636 onto current main pushed both files 2-31 lines over the file-size gate (tests.rs 2000->2002, class_decl.rs 1976->2007, from main's own growth plus this PR's small additions). Pure relocation, no logic changes: - tests.rs: extract the #8882 hoisted-sibling-in-a-later-closure test into its own tests/hoisted_sibling_in_later_closure.rs, matching the existing one-test-per-file convention already used for its neighbors. - class_decl.rs: extract lower_class_from_ast (class EXPRESSION lowering) into its own class_decl/from_ast.rs sibling module, matching the existing class_heritage.rs / member_registration.rs split.
cargo fmt
`js_string_code_point_at` walked the WTF-8 payload from byte 0 on every call for any string that is not pure ASCII, so a sequential scan over such a string was O(n^2). #10055 reported exactly this pathology; #10067 moved `charCodeAt` and bracket indexing onto a lazy sparse index with a cursor and left `codePointAt` on the old walk. The accessors disagreed on the same string. 200,000 indexed reads of `typescript@5.9.3`'s `lib.dom.d.ts` (1,874,815 chars, 45 of them non-ASCII): charCodeAt 2 ms codePointAt 13,049 ms Scaling over a string with a single `é`, time quadrupling per doubling: n=5,000 8 ms n=20,000 128 ms n=10,000 32 ms n=40,000 518 ms `codePointAt` is defined on code units, so no bespoke decoding is needed: route it through `utf16_unit_at` and apply the spec algorithm — read the unit, and only when it is a leading surrogate with a trailing surrogate after it combine the pair. That keeps the bounded WTF-8 stepping #6085 requires, so a payload ending in a truncated multi-byte lead still decodes from what is present instead of over-reading; the existing guard-page test `code_point_at_does_not_read_past_payload` covers that and still passes. After, same hardware, against Node v26.5.1: 200k reads, non-ASCII 13,049 ms -> 2 ms (node 1 ms) n=40,000 scan 518 ms -> 1 ms (node 0 ms) tsc --noEmit demo.ts 658.31 s -> 85.04 s user (node 0.80 s) The tsc figure is the motivating case: `sample` attributed ~85% of that run to `js_string_code_point_at` and ~12% to `copy_utf16_range` underneath it, because TypeScript's scanner calls `codePointAt` per character and `lib.dom.d.ts` — the largest file it loads — carries those 45 non-ASCII characters. 7.7x on the real workload, and the remaining gap is no longer string indexing. Values are unchanged: `"a\u{1F600}b"` still yields 97, 128512, 56832, 98 at indices 0-3 (astral code point at the pair start, bare trailing surrogate on the low half), `"ab".codePointAt(5)` is still undefined, all byte-identical to Node. Tests: two added next to the index they exercise — one walks every index of a long non-ASCII string containing a surrogate pair and compares against an independent UTF-16 expansion, one asserts forward and backward traversal agree so the cursor cannot serve a stale answer on a backward seek.
…0685) `copy_utf16_range` resolved its start boundary with `advance(bytes, Boundary::default(), start)` — a walk from byte 0 on every call. Slicing a non-ASCII string at increasing offsets, which is what every tokenizer does, was therefore O(n^2): n=20,000 33 ms n=80,000 404 ms n=40,000 102 ms n=160,000 1,662 ms ASCII controls are flat at every size, so this is the fast-path loss rather than the copy itself. This is the same pathology as #10055: #10067 moved `charCodeAt` and bracket indexing onto the lazy index, #10656 moved `codePointAt`, and this was the accessor still left on the walk. `Index::seek` is factored out of `unit_at` so `unit_at` and the new `boundary_at` share one implementation and one cursor rather than drifting apart — which is how the other two were missed. `boundary_at` returns `None` for short payloads and `start == 0`, where the caller's own walk is already cheap and a four-entry cache should not be disturbed, so those keep their existing behaviour exactly. After, against Node v26.5.1: substring scan, n=160,000 1,662 ms -> 1 ms (node 0 ms) tsc --noEmit demo.ts 85.04 s -> 7.76 s user (node 0.80 s) 11x on real tsc, and 85x cumulative against the 658.31 s this started at. `sample` had attributed ~97% of the remaining run to `copy_utf16_range` (50,627 of ~51,700 leaf samples; next symbol 162), because TypeScript's scanner extracts every token with `substring` and `lib.dom.d.ts` carries 45 non-ASCII characters in 1.87 MB. Correctness is unchanged, including the cases a wrong boundary would corrupt rather than merely slow: hashing every substring of a string containing astral characters (all i,j pairs, including ones that split a surrogate pair) gives 1234090636 on both Perry and Node, split-pair slices still yield the lone halves "\ud83d" / "\ude00", and a random-access-order slice hash matches. Tests: `boundary_at_matches_a_walk_from_zero` compares the indexed lookup against `advance` from zero at every index of a string containing both a two-byte scalar and a surrogate pair; `boundary_at_is_order_independent` asserts forward and backward traversal agree so the cursor cannot serve a stale boundary after a backward seek. 158 `string::` tests pass.
…th (#10694) `js_array_get_f64`, `js_array_set_f64`, `js_array_set_f64_extend` and the iteration-exotic helpers probed the buffer and typed-array registries on every element access. A `GC_TYPE_ARRAY` header can never be a registered buffer or typed array — every registration carries its own GC object type — and `array/header.rs`'s `receiver_may_be_registered_exotic` exists to say so in one already-warm header byte read plus an integer compare. Thirteen call sites in `array/iter_methods.rs` already used that gate. None in `array/indexing.rs` did. Measured with the runtime's own `PERRY_BUFFER_DIAG` on `tsc --noEmit demo.ts` (a two-line input): before: probes=79,691,777 admits=26,198,956 true_positives=90 after: probes=39,845,889 admits=21,646,032 true_positives=90 79.7M probes to answer a question about a set that never holds more than 9 buffers, and is answered yes 90 times. Honest perf note: this is not measurable on tsc wall-clock. `is_registered_buffer_slow` was 2.8% of leaf samples, so halving its calls predicts ~1.4%; five interleaved rounds give a median of -1.2% with overlapping ranges (A 6.85-8.02s, B 6.76-7.86s), i.e. below the noise floor at that sample size. The change earns its place on correctness and consistency — it removes provably-wasted work and makes the indexing path match the gate every iteration helper already uses — not on a demonstrated speedup. Buffer-heavy workloads should benefit more; tsc is not one. Gating the four remaining `||`-shaped probe sites in the same file changed the probe count by zero, so the other ~39.8M originate outside `array/indexing.rs` and are still unattributed. #10694 also records that the diagnostic sizes a 1024-bit/3-hash Bloom at 0.0% false-positive on this workload, which would make each surviving probe ~free regardless of caller. Verified: 443 existing tests pass (360 `array::`, 52 `buffer::`, 31 `typedarray::`), and a differential test against Node covering every typed array kind, Uint8Array wrapping (300 -> 44, -1 -> 255), Uint8ClampedArray clamping, f32 precision, Buffer, subarray aliasing and an Array subclass is byte-for-byte identical.
…10688) `UTF16_INDEX_CACHE` held `CACHE_ENTRIES = 4` indexes and evicted round-robin. A program interleaving indexed access across five or more non-ASCII strings evicted the entry it was about to need on every access and rebuilt it from scratch, forever. It was a step function, not a gradual decay: K interleaved before after 1 67 ns 67 ns 4 50 ns 67 ns 5 81,525 ns 67 ns 8 80,972 ns 67 ns 12 81,228 ns 67 ns 1,217x at K>=5, and flat everywhere. An ASCII control stays at 25-39 ns in both, which isolates the cause to the index rather than to string count. Capacity was the defect, so there is no capacity: the cache becomes an owner-keyed map and entries live until their string dies. The lifetime machinery already existed — `prune_dead_utf16_indexes` is driven by the collector — so this reuses it rather than inventing ownership. Raising `CACHE_ENTRIES` would only move the wall to K+1. I first tried changing the table's *shape* instead (sparse run-boundary syncs with affine spans, prototyped at 108x less index memory); it made this cliff 23% WORSE, 81,525 -> 100,287 ns, because K>=5 is a pure *rebuilding* workload and a run table builds more slowly than it queries. That experiment is written up on #10688 and is what established that the fix had to be "stop rebuilding". `scan_utf16_index_roots_mut` now drains, lets the visitor rewrite the owner identities the map is keyed by, and reinserts — the keys are exactly what the collector relocates, so they must be rehashed rather than mutated in place. Memory: peak RSS on `tsc --noEmit demo.ts` is 606.7 MB against 613.4 MB for the same binary without this change, i.e. slightly lower rather than higher. Entries are unbounded between collections by construction, which is a real change in character even though it does not cost anything measurable here. Tests: 158 `string::` tests pass single-threaded. The former `cache_eviction_is_bounded_and_short_strings_do_not_evict_sources` asserted `len() <= CACHE_ENTRIES`, which is now false by design, so it is replaced by `indexes_survive_any_number_of_interleaved_strings` — 16 strings, four times the old capacity, asserting every index survives, still answers correctly on a second pass, and is reclaimed by the prune hook. It keeps that test's other, capacity-independent invariant: a one-character string from `char_at` must not disturb its source's index. `gc::tests::copy_slot_decode::sabotaged_remembering_arm_is_refused_by_the_coverage_cross_check` fails identically with and without this change (same panic site, `copy_slot_decode.rs:135`), verified by running it alone against both trees; it is pre-existing and unrelated.
The loop that collects a global replace's matches polled the GC safepoint once per match. That poll costs about 436 instructions -- it evaluates the whole budgeted trigger ladder, which has no cheap "nothing is due" precheck -- and on this loop it enables no collection at all: matches are written into a native span buffer, so the loop creates nothing traced. Measured rather than argued. Nulling this poll entirely moves peak RSS on an allocating replace at n=1,000,000 by +0.0% median over nine interleaved rounds, 4 of 9 rounds in each direction. The same change applied to `Pieces::finish`, which does produce garbage, moved that figure +13.2% with 8 of 9 rounds against -- so this is a controlled contrast between an exposed and an unexposed site, not an assumption that polls are cheap to drop. Instructions, both arms from one commit, release, min of repeated rounds: replace, string template 27,837,069,685 -> 26,852,985,515 -3.5% replace, callback, ASCII 51,289,880,556 -> 50,427,663,998 -1.7% replace, callback, Unicode 61,277,245,689 -> 60,415,684,343 -1.4% replace1m (both, to n=1,000,000) 275,177,993,181 -> 270,666,099,260 -1.6% Peak RSS, nine interleaved rounds on replace1m: median -0.5%, mean +0.4%, 4 of 9 rounds higher. Answers are identical to Node 26.5.1 and to the previous build on every probe. Worst-case work between executed polls does not grow. Every search this loop performs goes through `find_near`, which either polls unconditionally (the owned path and any lent fallback) or ticks `PRE_SEARCH_POLL_TICK` and polls on one search in 64 (#10494). That tick advances once per search, which is once per iteration of this loop, so the two strides run in parallel on the same unit rather than composing: the bound stays 64 searches either way. The stride value matches `PRE_SEARCH_POLL_STRIDE` because they must count the same unit, not because 64 is derived. It is a chosen margin in #10494 -- the evidence there argues for removing the poll, not for any particular stride -- and nothing here depends on it being the right number, only on not exceeding the value already bounding this path.
…omplete/rawHeaders
…calls build_raw_headers_array (res.rawHeaders, #10467) held its result array's raw pointer in a plain local across alloc_string/js_array_push calls that can allocate and therefore collect, moving the array out from under it -- the unrooted-local-shape ratchet caught this going from 1 to 6 findings in this file. Root arr through TransientRootScope::root_nanbox and re-derive via .get() after every allocating call instead of reusing the pre-call copy, matching the pattern already used elsewhere in this crate. Also fixes the same pre-existing shape in build_response_headers_object's set-cookie array builder (unrelated to this PR's diff), extracted into its own top-level build_set_cookie_array so the rooting lines stay short enough that rustfmt doesn't wrap the let binding across lines -- which had been hiding it from the scanner's line-oriented detection. unrooted_local_shape.py --check: 558 (was 561 pre-PR; response_headers.rs per-file ceiling drops from 1 to 0).
…wering native_fluent_chain_still_dispatches_through_native_methods asserted the pre-fix, spelling-based, no-import native dispatch that this PR's own detect_native_instance_expr change deliberately eliminates. With no import at all, `new Decimal(1)` (or Command/LRUCache/Big/BigNumber) now correctly falls through to an unresolved-global reference -- matching Node's ReferenceError on a genuinely undefined global -- instead of silently reaching the native handle by name. The test predates this change and was never updated for it, so it went red on this same commit without this PR's diff touching that file: only the sweep's `cargo test --workspace` would have caught it, hours later and attributed to a time window rather than this PR. Removed with the rationale recorded inline, matching the identical resolution three PRs stacked on this branch (#10704, #10708, #10712) each carried independently -- landing it here so none of them has to repeat it. crates/perry-hir/tests/fluent_chain_lowering.rs now runs 2/2; the crate's full test suite (`cargo test -p perry-hir --tests`) is green.
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (58)
✨ 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 19, 2026
Closed
This was referenced Sep 19, 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
2 participants
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 221 — six PRs validated together as one tree, released as v0.5.1599.
Trains land as their own PR, which means the source PRs are closed, not merged, and their close-keywords never fire. Every issue they resolve is therefore listed here.
Contents
fix(codegen): forward implicit-ctor args to a native basesuper()reached viarequire()fix(ext-net):net.Socketsurface cluster —prependListener,on()chaining,pipe(), stream statefix(string): makecodePointAtandslicelinear on non-ASCII strings — native tsc 658s → 7.8sperf(regex): stride the replace collection loop's safepoint pollfix(http): clientrawHeaders/httpVersion*/complete,'upgrade'event, request-levelcreateConnectionfix(hir): resolve native-instance chain detection by import provenance, not spellingValidation
Assembled on
4715bc2fa1and proven before validating: every PR fully represented (no dropped commits, checked by patch-id and by subject+author, not by a pathspec diff), source heads asserted unchanged since assembly.Green:
fmt,file_size,raw_handle(self-test, ceilings, and vs-main),gc_runtime_root_holders,check_test_registration,addr_class_inventory,cargo check --workspace --all-targetsunder-D warnings, the release build of all five pinned artifacts, and theperry-codegen/perry-stdlib/perry-hir/perry-transformunit suites.Two results that need stating rather than burying:
cargo test --release -p perry-runtimereported two failures, both pre-existing onmainand both structurally incapable of passing under a release profile.gc::tests::heap_generation::a_free_or_move_outside_every_scope_is_caught_in_debug_buildsasserts thatdebug_assert_heap_change_open()panics, but that function is#[cfg(debug_assertions)]; andgc::tests::copy_slot_decode::sabotaged_remembering_arm_is_refused_by_the_coverage_cross_checksays so in its own doc comment ("In the debug buildcargo testruns…"). Neither is cfg-gated. CI'scargo-testjob builds debug and never sees them. Not this train's defect; both are gated with#[cfg_attr(not(debug_assertions), ignore)]in train 222.The
lintstep was killed (SIGTERM) after 4 of its 6 derived compile commands and re-run standalone to completion. It is recordedexpect=Nonebecause the public-baseline step is known-red onmain, which meant a killed lint was indistinguishable from a passed one. The validation driver now parses the script's own banner for the derived count and asserts the executed count matches — verified discriminating against the original log.Issues resolved
Closes #10623
Closes #10441
Closes #10442
Closes #10444
Closes #10465
Closes #10656
Closes #10685
Closes #10467
Closes #10468
Closes #10469
Closes #10439