merge train: land #10096, #10108, #10111, and #10112 - #10121
Merged
Conversation
added 9 commits
September 12, 2026 09:35
BUFFER_LIKE_ADDR_FILTER ends every measured claude-code run with all 1,024 bits set and rejects nothing, while costing three hash rounds and up to three dependent loads on every one of ~26.6 M admitted probes per run. Its adoption note asked the capacity question and answered it from a 400-character reply: 213 cumulative registrations, live_max 201, true positives 0.207% of admits, predicting a 10.0% false-positive rate. An ordinary command (startup, two real Read calls, streamed reply) falsifies both premises. The population is 15x larger - 3,232 cumulative admissions and 1,618 live against 1,024 bits - so the filter saturates, and every rejection comes from the window in front of it at 15.51%, not the 25.94% the note quotes. And 88.87% of admitted probes find a real registered buffer, against 0.207% before, so a filter cannot remove work the registry genuinely has to do. Six interleaved pairs, one binary and one environment variable: minimum command CPU 1.22 -> 1.19 s (-2.46%), medians 1.28 -> 1.20 s, paired median -3.60%, faster in five pairs and tied in the sixth, peak RSS maxima 664.5 -> 635.2 MiB. Node anchor in the same session 0.41/0.42 s. The min/max window stays and keeps doing all of the rejecting, including the debug-build machine-check that re-derives every rejection from the authoritative tables. Deleting a negative accelerator cannot produce a wrong answer. (cherry picked from commit b13d382)
…er-byte view lookups (#10088) collect_buffer_set_bytes paid a view-registry lookup per byte for a Buffer source, and a per-element dispatch/coercion call per byte for a TypedArray source, materializing the result into an intermediate Vec<u8> before copying it into the target a second time. For a Buffer source, or a same-element-width (1-byte) TypedArray source (Int8Array/Uint8Array/Uint8ClampedArray), resolve the source's raw byte span once via the existing view::resolve_data_ptr / typedarray::data_ptr resolvers and copy it straight into the target with ptr::copy (a memmove, so a source/destination overlap through a shared backing buffer stays correct). Array/Object sources and wider/BigInt TypedArray sources are unchanged. Conflict resolved on the train: this branch is based on train161's `main`, and train162's #10071 (zero-copy Buffer/Uint8Array subarrays) has since deleted `view::propagate_written_range_from_receiver` outright — views now share their backing storage, so a write through the target's data pointer is already visible to every view of it and there is nothing to propagate. The bulk-copy fast path and its `ptr::copy` overlap handling are kept verbatim; the two propagate calls are dropped because the function no longer exists.
(cherry picked from commit c9fb412)
The primary (case-insensitive) collation pass built two fresh lowercased
`String`s with `str::to_lowercase` and compared those, so every comparison
paid two heap allocations and two full Unicode case-mapping passes over both
operands to answer a question usually decided by the first character — and a
sort pays that O(n log n) times. Walk the two lowercased scalar streams in
lockstep instead, stopping at the first difference, with a byte-level loop
for the leading all-ASCII run.
On the non-ASCII path `locale_compare_canonical` also stops materializing two
NFC `String`s per comparison: an `is_nfc_quick` check (a table lookup per
scalar, no allocation) skips the rewrite for text that is already NFC, which
covers precomposed letters, CJK and emoji.
The ordering is deliberately unchanged. U+03A3 is the one scalar whose
lowercase mapping is context-dependent (final sigma ς vs medial σ); the walk
reports it rather than guessing and falls back to `str::to_lowercase`, which
implements the Final_Sigma rule.
Tests pin both halves of that claim:
`matches_the_allocating_reference_{on_every_pair,on_random_strings}` are
differential against the exact formulation this replaces, over a corpus
spanning ASCII, Latin-1 accented letters in both spellings, CJK, emoji,
combining marks, the two special case mappings and WTF-8 lone surrogates,
and assert the corpus reaches all three arms;
`locale_compare_is_a_strict_weak_ordering` proves reflexivity, antisymmetry
and transitivity over every triple, so `Array.prototype.sort` stays
well-defined; `canonical_equivalents_stay_equal` and
`documented_guarantees_hold` cover canonical equivalence, case-only
differences, empty/prefix/long-common-prefix pairs and the documented
divergence from ICU.
Also make the limitation public and accurate. `docs/typescript-parity-gaps.md`
listed `localeCompare()` and `toLocaleLowerCase()`/`toLocaleUpperCase()` as
"Missing (needs Intl)"; all three are implemented. The new note and the
rewritten `js_string_locale_compare` doc comment state what the ordering
actually guarantees — canonical equivalence, case-insensitive code point
order, a lowercase-first case tiebreak — and what it does not: no collation
weights, no locale tailoring, and an order that differs from Node for
accented letters, symbols and emoji, by design rather than by omission.
Refs #10094
(cherry picked from commit c8efcde)
(cherry picked from commit 759302d)
The corpus forged them with `from_utf8_unchecked`, because a lone surrogate has no valid `&str` spelling and `locale_compare_default` takes `&str`. `chars()` over such a slice yields a value that is not a valid `char`, and std's UB precondition check catches that in a debug build: CI's `cargo-test` job (debug profile) aborted with SIGABRT instead of reporting an ordering. A `--release` run does not enable the check, which is why this was green locally. That is a property of the runtime's WTF-8-as-`&str` view (`string_as_str`), unchanged by this PR and identical on both sides of the differential, so the entries could only ever have proven the checker works. The sound byte-level lone-surrogate coverage stays where its helper takes `&[u8]`: `utf16_cmp_ascii_fast_path_tests::lone_surrogates_fall_back_to_byte_order`. The corpus doc comment now says so. Verified on the debug profile CI actually uses: `cargo test -p perry-runtime` → 3606 passed, 0 failed, 4 ignored. (cherry picked from commit 6e0b8de)
`[x, y] = [y, x]` and `const [a, b] = pair(i)` lowered through the full spec
iterator protocol — `GetIterator`, one `iteratorNextResult` per element (each
allocating a `{ value, done }` result object), two property reads off that
result, and `IteratorClose` — none of which is observable for an array whose
iteration protocol is the pristine builtin. #10086 measured a flat 76x Node for
the swap and 78x for the destructured return.
The lowering now emits both arms and branches on the same runtime guard `for…of`
over a proven array uses (`Expr::ArrayIterationPatched`). A spread-free array
literal written in place has its elements spilled into temps before the guard
and read straight from them, so the array is never built; a source whose static
type proves a plain Array is read by index with `length` re-read per element,
exactly as `IteratorStep` does.
The branch is per element, not around the whole pattern: a pattern DECLARES
bindings, so lowering it twice would give each binding two `LocalId`s, and both
arms have to keep the spec's interleaving (`let [a = f(), b] = src` evaluates
`f()` between producing element 0 and element 1).
A rest element, a nested pattern, an empty pattern, a generator / Set / Map /
string source and anything without a static array proof keep the unguarded
iterator lowering, statement-for-statement identical to before.
The guard itself had a hole, which this closes: a replaced
`%ArrayIteratorPrototype%.next` is detected per `.next()` call, so an arm that
never calls `.next()` could not see it — `for…of` over a proven array has
iterated unpatched elements since #7760. The exported byte (renamed
`PERRY_ARRAY_ITERATION_NOT_PRISTINE`) is now also set when the array-iterator
prototype object escapes to user code through `Object.getPrototypeOf` /
`Reflect.getPrototypeOf`, the only way to name it in order to patch it. Setting
it on escape rather than on the write is deliberate: the object is ordinary, so
a precise hook would have to cover every mutation funnel and missing one fails
silently toward a wrong answer. `ARRAY_PROTO_ITERATOR_MODIFIED` keeps its
original narrow meaning, so the spread and `js_get_iterator` paths are unchanged.
(cherry picked from commit 9ad1974)
The PR changed crates/ without one, which the changeset gate requires. Body summarised from the commit message's own measurements.
This was referenced Sep 12, 2026
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (25)
📝 WalkthroughWalkthroughThe pull request adds guarded fast paths for array destructuring and buffer copying, removes the buffer address filter, and reduces allocations in locale comparison. It also adds tests, changelog entries, documentation updates, and increments the workspace version. ChangesArray destructuring fast path
Buffer copy and address probing
Locale comparison allocation reduction
Release metadata
Estimated code review effort: 5 (Critical) | ~90 minutes Change: Other Sequence Diagram(s)sequenceDiagram
participant TypeScript
participant DestructuringHIR
participant PerryRuntime
participant ArraySource
TypeScript->>DestructuringHIR: lower array destructuring
DestructuringHIR->>PerryRuntime: read PERRY_ARRAY_ITERATION_NOT_PRISTINE
PerryRuntime-->>ArraySource: return iteration guard state
ArraySource->>ArraySource: read indexed elements or initialize iterator
ArraySource-->>TypeScript: bind destructured values
Possibly related PRs
✨ 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 12, 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
1 participant
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 landing #10096, #10108, #10111 and #10112 on top of
maine8f912392(train163).The individual PRs merge as their own branch, so GitHub's close keywords do not
fire — each original is closed with a pointer comment after this lands, and the
issues it names are swept by hand.
b13d3824d56eb7eba6,c9fb41291c8efcde5a,759302d21,6e0b8dee69ad197404No stacks; no shared commits between any open pair; nothing already upstream.
The finding: #10096 predates train162's buffer rework
#10096 branched from train161's
main(c58c1fae5). Train162's #10071(zero-copy
Buffer/Uint8Arraysubarrays) has since deletedview::propagate_written_range_from_receiveroutright — views now sharetheir backing storage, so a write through the target's data pointer is already
visible to every view of it and there is nothing to propagate. Both arms of
#10096's new
matchcalled that function, so taking the hunk verbatim would nothave compiled.
Resolved by keeping #10096's actual contribution — the
bulk_copy_source_ptrfast path and its deliberate
ptr::copy(memmove, becausebuf.set(buf.subarray(2))can genuinely overlap) — and dropping the two propagate calls. The overlap
handling matters more after #10071, not less, since views really do alias now.
Verified every other symbol the PR depends on still exists on current
main:view::resolve_data_ptr(now a thinbuffer_datawrapper, which is exactly whata bulk copy wants),
typedarray::data_ptr,buffer_data_mut,collect_buffer_set_bytes, and the three 1-byteKIND_*constants.Audit notes
#10111's third commit removes lone surrogates from the locale-compare
corpus. That reads like a weakened test, so I checked the replacement rather
than the justification:
utf16_cmp_ascii_fast_path_tests::lone_surrogates_fall_back_to_byte_orderdoes exist, its helper takes
&[u8], and it asserts U+D800/U+DC00 orderingagainst each other, against ASCII, and against empty. It is a relocation to a
sound helper, not a deletion. The corpus entries were forging surrogates with
from_utf8_uncheckedfor a&str-taking comparator, which trips std's UBprecondition check — that is why CI's debug-profile
cargo-testjob aborted withSIGABRT while release runs were green.
#10108 deletes a saturated negative accelerator, which is the safe direction:
the
RegistryAddrWindowmin/max bound stays and keeps doing all of therejecting, including the debug-build machine check that re-derives every
rejection from the authoritative tables. The
gc_runtime_root_holdersgate isgreen after the static's removal, so it left no stale entry behind.
Maintainer fix commits
changelog: add fragment for #10108— that PR changedcrates/without afragment, which
check_changeset_fragment.shrequires. Body summarised fromthe commit message's own measurements.
chore: bump workspace version to 0.5.1538— none of the four PRs bumpedit, which is the maintainer's job at merge time;
Cargo.lockregenerated withit.
Validation
6389 tests, zero failures. Exit codes are captured from each command itself, not
from a pipeline.
Only one lint gate fails — public benchmark evidence freshness, genuinely red
on
mainsince 2026-07-29 and scoped out by #9969. The two API-doc gates thatnormally also fail are a
CARGO_TARGET_DIRartifact and pass oncetarget/release/perryis symlinked before the run; worth doing rather thanexplaining away, because without the symlink the shell truncates the redirect
target before exec'ing the missing binary and leaves
docs/src/api/reference.mdat 0 bytes.Each suite was checked for having actually exercised its subject, not merely for
not throwing: 65 locale/buffer tests in the runtime log including
utf16_cmp_ascii_fast_path_tests::lone_surrogates_fall_back_to_byte_order(thecoverage #10111 relies on when it drops the corpus entries), and 7 destructuring
tests in the HIR log for #10112.
The two new gap tests were run against the pinned Node 26.5.1, with the
static archives rebuilt first
(
-p perry -p perry-runtime-static -p perry-stdlib-static) and their mtimesconfirmed newer than the last commit:
Validated at head
b4d2491941a9d84bbc387088ffa8d6e4ba281dc9.Summary by CodeRabbit
New Features
Uint8Arrayand Buffer bulk copies are faster, including safe handling of overlapping ranges.Bug Fixes
Performance
localeCompareperformance by reducing temporary allocations.Documentation