perf: inline number strings, streamline Map guards and UUID formatting - #11643
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 |
proggeramlug
added
the
run-extended-tests
…643-remove-held-regex-20261002 # Conflicts: # scripts/gc_runtime_root_holders.json
proggeramlug
changed the title
2 participants
Current head
65cbfce71e8b4d26c781f8e04ac5d09150550433incorporates the landed main CI repairs: the diagnostic census has a five-minute step limit, and full-tier require fixtures prepare coherent providers before their bounded execution. The merge changes only five workflow/protocol-test/changelog files; every Rust source, manifest, lockfile and Cargo configuration is byte-identical to the validatedf57f5d90984cfe6068e709e8d07e6ddebadf8f06head. All eleven removed regex/nursery paths remain byte-identical to current main. At the refreshed head, all eleven provider protocol tests, eighteen GC wiring controls and nine wired gates pass; whitespace checks pass.The previous native root job 110697725348 succeeded in both required checks and its diagnostic census, but the census consumed 12m56s and the whole job hit its 90-minute limit during Post Cache cargo. Its cancelled whole result is not approval. The new head needs actual successful whole shadow and native root jobs and remaining CI acceptance. Product/unit/Node/main-stdlib comparisons below were obtained at the earlier scoped head; source identity is established above, but no new-head whole CI result is claimed.
This PR now contains three scoped improvements:
charCodeAt, retaining the original runtime fallback for values outside the proven fast path (perf: number-to-string is 6.6x-15.9x bun, and four spellings of it differ by 2.6x inside perry while bun spans 30-34 instructions #10762).crypto.randomUUID()is 17× slower than Node even without the per-read closure (onegetrandomsyscall per UUID, no entropy cache) #10523).The held regex scratch/external-byte accounting and adaptive nursery changes have been removed. All eleven affected regex, collector-policy, test, documentation and changelog paths are byte-identical to main
47a0c7bf1795bc5e1f85d6c96c0f52037ff1972b. The root-holder inventory preserves every main entry and source pin and adds only three scalar/test holder entries. No RSS-versus-compute tradeoff is included in this scoped change.Validation at
f57f5d90984cfe6068e709e8d07e6ddebadf8f06:The full serial stdlib suite has 243 passes and one pre-existing failure in
runtime_thread_exit_tests::symbols_tests::thread_exit_releases_the_threads_dom_exceptionsatsymbols_tests.rs:484(a dead thread's DOMException address outlived its heap). An independent whole-suite run at current main47a0c7bf1795bc5e1f85d6c96c0f52037ff1972b, using the same locked release command, package and environment, produced exactly the same 243 passes and single failed test/assertion; the test source is byte-identical. No additional stdlib failures were introduced.Full local script lint finished: 110 of 111 checks passed; only the known Public benchmark evidence freshness check failed. The compile tier and two CI-only commands were explicitly skipped; the locked release product build and targeted codegen units above were run separately. The owner authorized removing the held bundle and landing the valid scoped fixes; the bundle-related hold can now be lifted for this published head. Fresh CI remains required.
The instruction counts in the retained changesets are the original author's historical measurements; they have not been remeasured at this revised head. CI approval requires new-head checks, including successful whole
gc-root-dominanceandgc-root-dominance-statepointsjobs. The old skipped jobs provide no approval for this head.Historical CI triage at
f57f5d90984cfe6068e709e8d07e6ddebadf8f06: native-backend job 110697759307 completed with 1,844 passes, five failures and one ignored test. All five failed property-read IR assertions match the independently completed main job 110249849088, including the previously reviewed inherited-to-generic test-name rename; the test source is byte-identical to the previously reviewed integration head. Lint job 110697808524 failed only Public benchmark evidence freshness. These are specific main-side failure attributions, not green CI results. The previous shadow job passed; the previous native whole job later cancelled during cache saving. Neither history replaces new-head CI approval.Current-head GC update: both whole jobs completed successfully at
65cbfce71e8b4d26c781f8e04ac5d09150550433: shadow root dominance and native statepoints root dominance. All executed steps in both jobs succeeded. Remaining CI failures and pending jobs still require acceptance; this does not claim overall CI is green.