merge train: land #10075 and #10077 - #10113
Conversation
(cherry picked from commit 0c5348c)
(cherry picked from commit 8d66646)
(cherry picked from commit 6183863)
Conflicts resolved on the train (#10072/#10074 landed first): - array/concat_reverse.rs, array/sort.rs: kept main's newer structure -- #10072's three-arm linear append in js_array_concat, and the publish_sorted_values / with_sorted_indices refactor -- then applied this change's actual intent to it, replacing every open-coded `header + size_of::<ArrayHeader>()` element base with `crate::array::array_elements_ptr`. That includes four sites in sort.rs that postdate this branch's base and so were never converted here. - array/push_pop.rs: union -- this change's offset-aware accessor plus main's TAG_HOLE -> undefined conversion, which spread needs and this branch predates. - scripts/gc_runtime_root_holders.json: merged structurally, not textually. The PASS1_MARKED re-audit prose is base + main's #10055/#10054 paragraphs + this change's #10060 paragraph, and each `window.sources` pin is taken from the side that actually moved it (gc/census.rs from here, gc/mod.rs from main) so neither is reverted to the base hash. - test-parity/gc_repsel_corpus.txt: both new entries kept.
(cherry picked from commit 1e79ee9)
(cherry picked from commit b27feab)
(cherry picked from commit 176e5ab)
#10077 gives a dense array a queue front offset: `capacity` counts the slots remaining AFTER the front, so logical element zero is no longer the end of `ArrayHeader`. That PR converted every open-coded `header + size_of::<ArrayHeader>()` element base in its own tree — but two places on current `main` postdate its branch point and were never converted: - `json_tape/cached_read.rs` (from #10064, landed in train161) open-codes the base on the hot materialized-array read. A materialized JSON array is an ordinary `GC_TYPE_ARRAY` that user code can `shift()`, so after this train that read would return the wrong element. Routed through `crate::array::array_elements_ptr`. - `array/sort.rs` gained four more open-coded bases with the `publish_sorted_values` / `apply_sorted_indices` refactor. Converted with the conflict resolution in the pick itself; only the rustfmt reflow lands here. Also converts the six raw-handle debt sites #10075's two new test modules introduced. `--no-raise-vs <merge-base>` refuses both a ceiling on a module that was absent at the base (`string/slice_tests.rs`) and a per-module raise (`gc/tests/runtime_roots/string_slice.rs`, 1 -> 3), so the sites are converted rather than recorded: - `gc/tests/runtime_roots/string_slice.rs`: the two post-collection re-reads now run the collection inside `across_mut` / `across_const`, and the kept-slice assertions inside `with_const_ptr`. That also retires the module's one pre-existing site, so its ceiling line is DELETED — the deletion is the receipt — and the recorded total falls 945 -> 944. - `string/slice_tests.rs`: the three argument-position reads become `with_const_ptr`, which is the documented shape for a self-rooting entry point such as `js_string_slice`.
|
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 (179)
📝 WalkthroughWalkthroughThis pull request adds dense ChangesArray shift queue and storage
String slicing and suffix cursors
Benchmark and release support
Estimated code review effort: 5 (Critical) | ~90 minutes Change: Bug fix · Severity of issue fixed: Medium 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 |
Merge train landing #10075 and #10077 on top of
maindc0d876fe(train162). Both were
CONFLICTINGagainst currentmain.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.
0c5348ce5,8d66646ff,6183863dda7c59a837,1e79ee9e4,b27feab6f,176e5ab86The finding: #10077's front offset vs. code that postdates its branch point
#10077 gives a dense array a queue front offset —
capacitycounts theslots remaining AFTER the front, so logical element zero is no longer the end of
ArrayHeader, and every open-codedheader + size_of::<ArrayHeader>()elementbase becomes wrong for an array that has been
shift()ed. The PR converts allof them in its own tree, in both the runtime (
array::storage::array_elements_ptr)and codegen (
LlBlock::array_elements_addr, 36 call sites across 20 files).mainhas moved since that branch point and grew new ones the PR never saw. Iswept the merged tree for every element-base idiom and converted the two that
remained:
json_tape/cached_read.rs— the hot materialized-array read landed byperf(json): read traced materialized arrays without redundant resolution #10064 in train161. A materialized JSON array is an ordinary
GC_TYPE_ARRAYthat user code can
shift(), so without this the read returns the wrongelement. This is a genuine cross-PR interaction, not a style fix.
array/sort.rs— four more bases from thepublish_sorted_values/apply_sorted_indicesrefactor.The only
size_of::<ArrayHeader>()uses left in the tree are allocation-sizevalidations (
param_type_guard.rs) and GC payload arithmetic, not elementbases; codegen has no remaining literal
+8on an array pointer.Conflict resolutions
array/concat_reverse.rs,array/sort.rs— keptmain's newerstructure (fix(runtime): make spread push scale linearly #10072's three-arm linear append in
js_array_concat, and thesort refactor), then applied fix(runtime): make dense Array.shift queue drains linear #10077's intent to it. Taking "theirs" would have
reverted fix(runtime): make spread push scale linearly #10072 back to the
ptr::copy_nonoverlapping+rebuild_array_layout_exactbulk path that madepush(...chunk)quadratic.array/push_pop.rs— union: fix(runtime): make dense Array.shift queue drains linear #10077's offset-aware accessor plusmain'sTAG_HOLE→undefinedconversion, which spread needs and fix(runtime): make dense Array.shift queue drains linear #10077 predates.scripts/gc_runtime_root_holders.json— merged structurally, nottextually. Each
window.sourcespin is taken from the side that actuallymoved it (
gc/census.rsfrom fix(runtime): make dense Array.shift queue drains linear #10077,gc/mod.rsfrommain); a line unionwould have restored
gc/mod.rsto the stale base hash. ThePASS1_MARKEDprose is base +
main's perf(string): non-ASCII charCodeAt and bracket scans become quadratic #10055/perf(string): trim scans and copies the entire interior for constant-size edge whitespace #10054 re-audits + fix(runtime): make dense Array.shift queue drains linear #10077's perf(runtime): Array.shift queue drain reaches 722x Node at 10k elements and times out at 100k #10060 re-audit,so no changed pin loses the justification it is required to carry.
test-parity/gc_repsel_corpus.txt— both new entries kept.string/mod.rs(fix(string): preserve UTF-16 slices and avoid quadratic suffix parsing #10075) — module-declaration list, sorted union.Maintainer fix commits
fix(runtime): make the JSON cached read honour the array front offset—the two conversions above, plus the six raw-handle debt sites fix(string): preserve UTF-16 slices and avoid quadratic suffix parsing #10075's new
test modules introduced.
--no-raise-vs <merge-base>refuses both a ceilingon a module absent at the base (
string/slice_tests.rs) and a per-moduleraise (
gc/tests/runtime_roots/string_slice.rs, 1 → 3), so they areconverted: the post-collection re-reads now run the collection inside
across_mut/across_const, and the argument-position reads becomewith_const_ptr. That also retires the module's one pre-existing site, so itsceiling line is deleted and the recorded total falls 945 → 944
(
--no-raise-vs origin/main: "none raised").chore: bump workspace version to 0.5.1537— neither PR bumped it, whichis the maintainer's job at merge time;
Cargo.lockregenerated with it.Validation
6375 tests, zero failures. Exit codes are captured from each command itself, not
from a pipeline.
Only one lint gate fails, not the usual three. The two API-doc gates are
normally written off as a
CARGO_TARGET_DIRartifact, becausescripts/regen_api_docs.shbuilds withCARGO_TARGET_DIRbut then reads ahardcoded
$ROOT/target/release/perry. Creating that symlink BEFORE the runmakes them genuinely pass with zero drift. Worth knowing: they are not merely
failing, they are destructive — the shell truncates the redirect target
before exec'ing the missing binary, so a run without the symlink leaves
docs/src/api/reference.mdat 0 bytes, a 4229-line deletion sitting in theworktree. The remaining failure, public benchmark evidence freshness, is
genuinely red on
mainsince 2026-07-29 and is scoped out by #9969.The four new gap tests this train adds were run directly 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:
The two GC fixtures are registered in
test-parity/gc_repsel_corpus.txt, sogc-stress and gc-moving-witnesses will execute them rather than leaving them
dark on disk.
Each suite was also checked for having actually run its subject: 10 tests from
the new
array::shift_queue_tests,string::slice_tests,gc::tests::copying::shift_queueandgc::tests::runtime_roots::string_slicemodules appear in the runtime log.
Validated at head
2aa97f529d45ab7041f5f3fc58f03e9ce45091f3.Summary by CodeRabbit
Bug Fixes
Array.prototype.shift()behavior for dense queues, aliases, garbage collection, growth, and observable edge cases.slice(),substring(), andsubstr()handling at UTF-16 and surrogate boundaries, including lone surrogates and Unicode characters.Performance
Tests