perf: inline number-to-string, fix own-override flag arming for Map/Set, regex scratch GC pressure, faster randomUUID - #11643
proggeramlug wants to merge 9 commits into
Conversation
#10762: String(n), `${n}`, n.toString() and "" + n on a number operand build a small integer's SSO text inline at the call site (fixed-point digit split, exact for every value under 100000), with the runtime call on a cold arm. `const s = String(n)` / `${n}` / "" + n (a + with a string literal) now record a runtime-derived String proof, so s.charCodeAt and the other string lowerings no longer fall to the generic method site, and the inline charCodeAt reads an ASCII SSO receiver's byte from the value instead of materializing a heap copy. #10697: populating globalThis installed builtins onto intrinsics through the exotic-store gauntlet and armed PERRY_OWN_NAMED_PROP_INSTALLED, which sent every proven Map/Set/Date builtin call through the ~600-instruction hasOwn predicate. The runtime's own builtin definitions now arm it only for Map/Set/Date (or unreadable) owners; the universal dispatcher reads a separate flag every install still arms. Small-map lookups also answer a bit-identical key of any type from the inlined hot lane. #11549: the lent regex scratch cell grows past 32 registers, and operation-scoped regex Buffers are accounted as transient external bytes, so a large pattern in a loop no longer feeds released-bytes pressure into full collections. The scavenge nursery now powers on at a quarter of its base and climbs back on survivor influx, which keeps peak RSS flat now that the phantom fulls are gone; the #8122 census seeds at half the power-on cap. #10523: the UUID formatter writes through a constant position table and the stdlib copies its bytes without re-validating them as UTF-8. No version bump.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review. 📝 WalkthroughWalkthroughThe pull request updates numeric string conversion, small-Map lookup, builtin install tracking, regex scratch and garbage collection, and UUID formatting. It adds supporting tests, documentation, and changelog entries. ChangesInline number-to-string and character access
Small-Map lookup
Builtin own-override tracking
Regex scratch and garbage collection
UUID formatting
Priority: ⬇️ Low Estimated code review effort: 4 (Complex) | ~60 minutes Change: Refactor Suggested reviewers: Merge Risk: ⚪ Minimal · up to The selected changes are mergeable after normal checks; no unresolved issue is established. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to Several shared behaviors change, but the examined controls remain in place and no exploitable regression was established. Broader integration behavior remains unverified. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 69.51% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 82 functions across 30 files. (1 skipped: 1 unsupported.) ✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
🛠️ Fix failing CI checks 💡
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 |
… user Without the feature the regex module is compiled out and both functions were dead code, which the -D warnings workspace check rejects. Re-pins PASS1_MARKED's gc/policy.rs source hash.
|
No fix exists to port. The artifact has to be regenerated with Generated by Claude Code |
…r-453kqt # Conflicts: # scripts/gc_runtime_root_holders.json
…r-453kqt # Conflicts: # scripts/gc_runtime_root_holders.json
…r-453kqt # Conflicts: # scripts/gc_runtime_root_holders.json
|
Cause: a class static setter receives the class object instead of the assigned value. For example, Fix: none that I've verified yet. #11667 reworks static storage, but I haven't confirmed that it fixes the setter argument. Once a fix lands on Generated by Claude Code |
…r-453kqt # Conflicts: # scripts/gc_runtime_root_holders.json
|
Proposed patch for --- a/scripts/ci_e2e_scope.py
+++ b/scripts/ci_e2e_scope.py
@@ _CODEGEN_SUITES
"typed_array_rmw_8692",
+ "typed_array_update_lowering",
"typed_shape_declared_at_allocation",Once this lands on Generated by Claude Code |
Resolve scripts/gc_runtime_root_holders.json by taking main's PASS1_MARKED text, re-appending this branch's #11549 audit note, and pinning gc/policy.rs to the merged file (it differs from main only by this branch's audited hunks). Port own_override_builtin_install_tests to main's #11654 closure ABI: js_closure_alloc now takes a static JsFunctionInfo, and bodies take the #11637 receiver parameter.
|
This PR doesn't touch either file or the ratchet. Proposed patch for - "_hot_declarations": 528,
+ "_hot_declarations": 521,
- "crates/perry-runtime/src/async_hooks.rs": 7,
+ "crates/perry-runtime/src/async_hooks.rs": 6,
- "crates/perry-runtime/src/node_stream_constructors.rs": 3,
+ "crates/perry-runtime/src/node_stream_constructors.rs": 2,Once this lands on Generated by Claude Code |
|
This PR doesn't cause it; the drift is on
I haven't found a fix on Generated by Claude Code |
…r-453kqt # Conflicts: # scripts/gc_runtime_root_holders.json
|
On
All three look like fallout from Generated by Claude Code |
This branch has not been deployed
2 participants
Summary
Performance fixes for four issues. No version bump, and no changes to any JS-callable native function's signature. Every measured workload's output is byte-identical to Node 26.5.1.
Changes
#10762: number-to-string
String(n),`${n}`,n.toString()and"" + non a numeric operand now build a small integer's SSO text inline at the call site. The runtime call is kept on a cold arm. The digit split is fixed-point and exact for every value under 100000 (expr/number_to_string_inline.rs).const s = String(n),`${n}`and"" + n(a+with a string-literal operand) now record a runtime-derivedStringproof. Before this,s.charCodeAt(i)on such a local fell to the generic method site.charCodeAtnow reads an ASCII SSO receiver's byte directly from the value. Before, it materialized a heap copy through the intern table (~175 instructions).String(n)/`${n}`consumed bycharCodeAtgo from 449/462 to 169 instr/op; a negative integer from 644 to 199.#10697: string-keyed Map
globalThispopulation armedPERRY_OWN_NAMED_PROP_INSTALLED, because installing statics on%TypedArray%,Array.prototype.constructorandNumber.parseFloatgoes through the exotic-store gauntlet. After that, every provenMap/Set/Datebuiltin call paid ~600 instructions ofhasOwn.m.get(k)from 947 to 206.#11549: regex scratch counted as GC pressure
perex_memory::Bufferbytes are counted as transient, via the newgc_note_external_transient_alloc/_free. They no longer feed the released-bytes term that scheduled full collections.#10523: randomUUID
getrandomcalls for 120k UUIDs.Four
changelog.d/11643-*fragments have the details and measurements.Related issue
Refs #10762, #10697, #11549, #10523
crypto.randomUUID()is 17× slower than Node even without the per-read closure (onegetrandomsyscall per UUID, no entropy cache) #10523: its syscall target was already met on main.node:cryptowall time was still ~2.5× Node on a loaded host before this PR's formatter change.Test plan
Regression tests fail without their fix; each was sabotage-checked by reverting the fix locally:
a_search_over_thirty_two_registers_borrows_the_lent_scratch: 64/64 searches took the owned path before, 0 now.regex_scratch_buffers_do_not_count_as_released_external_pressure: 8,192 bytes of released pressure before, 0 now.the_nursery_starts_at_a_floor_and_follows_survivor_influxa_builtin_install_on_an_intrinsic_does_not_arm_the_guard: 272 arms before, 0 now; installs onto Maps still arm.a_small_map_answers_an_identical_key_without_the_cold_path: 8/8 lookups went to the cold path before, 0 now.number_to_string_inline_tests(3 tests)hyphenated_layout_is_exactGap tests added:
test_gap_10762_inline_number_to_string,test_gap_10697_own_override_after_global_population,test_gap_11549_regex_large_register_scratch.Suites run locally, with branch results compared against main:
RUST_TEST_THREADS=1 cargo test -p perry-runtime --lib: 4709 passed, 0 failed.cargo test -p perry-codegen --lib: 1790 passed.perry-uuid: 4 passed.test_gap_10623_implicit_ctor_native_super,test_gap_6558_webassembly_graceful_fail) pass through the harness with a build of the pushed commit.check_file_size,addr_class_inventory,gc_runtime_root_holders(4 new holder verdicts, and thePASS1_MARKEDpin re-audited forgc/policy.rs),check_gc_doc_claims,local_binding_type_audit,registry_lifetime_check,runtime_abi_check --check-native, and the others in thelintjob.cargo fmt --checkis clean.RUSTFLAGS=-D warningscheck (not enough disk here). The changed crates' lib and test builds introduced no new warnings.run-extended-tests/ gc-ratchet. The nursery starting cap is a default-pacing change. I checked it on dotenv, moment, a keep-one-in-ten retaining loop (within ±1.5% of main's total instructions at 2M–4M iterations, 22–27% below it at 0.5–1M) and an allocation-heavy string loop (−10%), but not on the ratchet corpus.Not addressed:
PERRY_GEN_GC=0. This is pre-existing and deserves its own issue."" + x, wherexis declarednumberbut holds an object with bothvalueOfandtoString, prints thetoStringresult where Node usesvalueOf. The behavior is identical on main.🤖 Generated with Claude Code
https://claude.ai/code/session_01Vagu5YeePM8twRt4LkpaZ3
Summary by CodeRabbit
Performance Improvements
Bug Fixes