Merge train 220: node:http client pump, deferred conditional require, dyn_eval class expressions, inherited property reads, GC slot and ASCII perf, jsonwebtoken binding removal (v0.5.1598) - #10716
Merged
Conversation
…ib (tokio unification)
) The copying minor, the full mark and the remembered-set rebuild each enumerate child slots for every object they trace, including objects that have none to enumerate. Finding that out costs ~206 instructions per object: iterator construction (61), the descriptor body (127), the worklist push, and the drain entry with its cold header read. `gc_object_yields_no_child_slots` answers the question from the header word the caller has already loaded, so those objects are never pushed. Three of the four terms fold into one mask compare on `_reserved`; the term order is measured rather than chosen, and the comment says so. Measured with `perf stat -e instructions:u`, min-of-5, same SHA in both arms: gc3 -7.10%, w20000 -5.75%, w5000 -4.70%, leafarr -4.22%, w1000 -2.25% Call counts, per consumer: copying minor 2,800,337 -> 1,520,316 (-45.7%) full mark 2,400,630 -> 1,200,319 (-50.0%) remembered-set rebuild 980,690 -> 490,345 (-50.0%) Peak RSS on gc3 -4.4%, from the smaller worklist. Collection counts are identical on all seven fixtures, so this perturbs no pacing. Three fixtures regress: oldyoung +0.11%, dist16_ptr +0.12%, rec16_ptr +0.06%. This is structural, not noise. The predicate costs O(traced objects) while the win is O(qualifying objects), and oldyoung's pointer-free population is objects rather than arrays -- they pass the mask compare, fail the type test, and so pay both terms while qualifying for neither. In the full mark the skip is additionally gated on proxy tracing being inactive. A pointer-free payload is still handed to gc_observe_traced_value while a proxy is being traced, so skipping it there would collect a live proxy's target. The minor and the rebuild are unconditional; that asymmetry is deliberate.
…at memo probe concat_byte_parts (the s + t fast path for two statically-typed string operands) scanned both operands for ASCII-ness twice - once via bytes_all_ascii up front, again on the heap path via l_slice.is_ascii() && r_slice.is_ascii() - with the first scan's answer sitting unused in scope. A new sibling of str_bytes_from_jsvalue, str_bytes_ascii_from_jsvalue, computes the bit once and threads it through; bytes_all_ascii itself switches from a byte-at-a-time loop to <[u8]>::is_ascii() (word-at-a-time, total over arbitrary byte strings). An earlier version of this change also tried to read a heap string's ASCII-ness straight off its header (utf16_len == byte_len, free) instead of scanning at all. That is unsound and was caught in review before landing: Perry heap-string payloads are not guaranteed valid UTF-8 (WTF-8 lone surrogates, Buffer.toString of arbitrary bytes, FFI blobs - #6085), and a payload ending in a truncated multi-byte lead byte can coincide on utf16_len == byte_len without being ASCII (compute_utf16_len_wtf8 charges a truncated lead its full nominal unit count while the payload holds fewer bytes than that sequence declares - string/compare.rs's utf16_cmp_bytes doc names the identical hazard). The header check survives only as a negative filter (utf16_len != byte_len soundly proves non-ASCII, unconditionally, not just for well-formed input); utf16_len == byte_len is ambiguous and always falls back to a real scan. js_string_concat_value's memo-admission gate had the identical exposure independently and is fixed the same way. No TypeScript-reachable path that constructs such a payload was found: Buffer.toString (all seven encodings), TextDecoder.decode, and every bun:ffi string-returning path all validate via str::from_utf8/from_utf8_lossy (or are fed a Rust &str, valid by construction) before ever calling js_string_from_bytes. The fix stands regardless, since js_string_from_bytes is a pub extern "C" entry point whose own contract must hold for any bytes. Regression tests are therefore Rust-level against hand-built malformed StringHeaders (the same technique string/compare.rs's own corpus and tests_guard_page.rs already use) rather than a gap test - all three fail against the reverted code, confirmed by temporarily reintroducing it. The short-concat memo's probe also assembled both operands into a stack buffer just to hand the hash/lookup helpers one contiguous slice. FNV-1a is a streaming hash and the byte compare can run in two parts, so concat_memo_hash_parts / concat_memo_slot_and_tag_parts / concat_memo_lookup_parts replace the buffer with direct two-slice hashing and lookup; the single-slice js_string_concat_value memo probe now goes through the same two-slice primitives. The memo probe's own break-even hit rate (governed by the probe's hash/lookup/admit cost, not by the ASCII-determination fix) barely moved: ~54% before this fix, ~50.6% after, both measured by forcing the governor on/off via a temporary env knob (since removed). MEMO_MIN_HIT_SHIFT stays 1 (50%, still the closest power-of-two floor to either number) - it moved from the original 2 (25%, never measured) in this same change. Measured (differential instruction-count probe, bare-loop control flat in both arms, base and arm built in the same session): 640-distinct short concat 548 -> 403 instructions/concat (-26.5%), a 73-byte memo-ineligible concat 644 -> 471 (-26.8%), a 100%-memo-hit workload 319 -> 318 (unchanged, within noise). Covered by test-files/test_gap_string_concat_memo_ascii_header.ts (byte-for-byte against node) and GC stress on a concat-heavy fixture (117,923 copying minors, 14,739 retired from-space sets quarantined, no fault) confirming the memo's GC roots survive evacuation under the new two-slice storage.
…ng (#10437) Perry's CJS->ESM wrap turned every literal require('S') in a wrapped file into a hoisted static import, eager-initializing the target regardless of whether the surrounding control flow ever reaches the call. function_local_specs only kept a require() lazy when every call site sat inside a function body; a top-level if/for/while/switch/try/ &&/?: guard (including pg's own if (forceNative) { require('./native') }) still forced eager init. Broaden the classification to also cover a control-flow block (if/for/while/switch/catch/with/else/try/do/finally) and a braceless/operator equivalent (cond && require(...), cond ? require(...) : x, for (...) require(...) with no block) -- matching Node's actual 'loads only when control flow reaches it' semantics. An ordinary object literal, class body, or bare grouping block does not count (the common module.exports = { fs: require('fs') } barrel shape stays eager), and a process.platform === '<literal>' guard (node-pty's Windows/Unix terminal split) is exempted since the platform is a compile-time-known build target, not a runtime unknown. This was the sole remaining blocker compiling pg from source: pg crashed at init with Cannot find module 'pg-native' even though its guarding forceNative check was false. Fixes #10437.
… on a scalar-replaced object (#10689) `check_escapes_in_expr`'s `Expr::PropertyGet` arm treated every read on a scalar-replacement candidate as a plain field read, without checking that the class chain declares the key. Scalar replacement allocates a slot per declared field only, so `expr/property_get.rs`'s scalar arm found no slot and folded the read to the constant `undefined`. The effect was silent and order-dependent: const o = { a: 1 }; typeof o.toString // undefined, where node gives "function" o.toString() // correct — a fused call never consults the // elided object It reached `Object.prototype` members read as values (`toString`, `constructor`, `hasOwnProperty`), a class's own prototype method read as a value, and user-added `Object.prototype` properties. Anything that made the receiver escape — passing it to a function, storing it in an array — repaired it, which is what made the bug look like it depended on unrelated earlier statements. This is the READ half of the rule the write arms already apply: #9024 for `PropertySet`/`PutValueSet` and #9460 for `PropertyUpdate`, plus the sibling literal analysis in `escape_objects.rs`. Only the read arm was missing it. Reads of declared fields still take the no-heap scalar path, which is spec-correct because an own property shadows the chain and OrdinaryGet never reaches the prototype. Reads of undeclared keys now take the ordinary heap path. A fused method call is unaffected: its callee is handled in the `Expr::Call` arm and is no longer routed through `PropertyGet`, so `simple_scalar_method_summary` receivers stay scalar-replaced.
Fixes #10683. The hand-written native jsonwebtoken binding (crates/perry-ext-jsonwebtoken, plus a second duplicate implementation in crates/perry-stdlib/src/jsonwebtoken.rs exporting the same js_jwt_* symbols per #10678) has a live security defect: verify() returns null instead of throwing on every forgery case (tampered payload, wrong secret, alg:none, garbage token, tampered signature, expired token), so `try { jwt.verify(...) } catch { reject() }` never rejects a forgery. sign(..., { expiresIn: "1h" }) also silently drops the expiry (string coerces to NaN, and the runtime only writes `exp` when > 0.0). Removes both copies plus the dedicated codegen lowering path (lower_call/native/jsonwebtoken.rs's lower_jsonwebtoken_sign/_verify and its native_runtime_branch.rs dispatch), the decode-only NativeModSig row in native_table/utils_crypto.rs, the js_jwt_* FFI declarations in runtime_decls/stdlib_ffi/third_party.rs, the well_known_bindings.toml entry, the NATIVE_MODULES/manifest rows, the bundled-jsonwebtoken stdlib feature (re-wiring dep:rsa/dep:spki directly onto perry-stdlib's `crypto` feature, since webcrypto/key_object.rs and keys.rs need them independently of jsonwebtoken), and the Android stub exports. The real `jsonwebtoken` crates.io dependency stays — it is unrelated Rust tooling used by perry's own Apple code-signing (commands/run/resign.rs, commands/setup/common_apple.rs). Regenerated docs/api/perry.d.ts, docs/src/api/reference.md (--print-api-manifest) and docs/src/native-libraries/governance.md (binding_governance.py --table). Updated workspace-architecture.json (workspace_members 83->82, externalize 33->32) and scripts/string_payload_access_baseline.txt (perry-stdlib inline-offset sites 40->39, from the deleted stdlib file).
This was referenced Sep 19, 2026
|
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 (64)
✨ 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
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.
This train lands 7 PRs as v0.5.1598. Each source commit is verified to preserve its patch-id and authorship.
#10466) — rebuilds the client-pump stdlib fornode:http.#10437) — defers conditional CommonJSrequire()init.#10661) — supports class expressions indyn_eval new.#10689) — an inherited property read no longer falls through.#10683) — removes the jsonwebtoken native binding.Two PRs were pulled from this train, each for a defect a diff-scoped gate could not see
#10699 broke
native_fluent_chain_still_dispatches_through_native_methodsincrates/perry-hir/tests/fluent_chain_lowering.rs— a suite its own diff never touches. The mergedriver and CI's
e2e-scopedboth derive which integration suites to run from the diff, soneither would have run it; only the sweep tier's
cargo test --workspacewould have, hours afterthe merge and attributed to a window rather than a PR. Confirmed by running it directly: 2 passed /
1 failed with the PR, 3 passed / 0 failed without it. Its author has since repaired it at source.
#10668 took
crates/perry-ext-http/src/response_headers.rsfrom 1 to 6 unrooted-localshapes —
REGRESSION: 563 findings exceeds baseline 561, againstmain's 559. The cause was asingle
*mut ArrayHeaderheld acrossalloc_stringcalls inside a loop and reused afterwards:CLAUDE.md's root-store-dominance violation exactly, on a handle-backed surface. Now fixed, and the
fix found a second pre-existing instance of the same shape in the neighbouring set-cookie builder.
A gate detail worth recording from that second one.
unrooted_local_shape.py --no-raise-vs "$BASE_SHA"returned green on the same tree, in the samerun_lint_gates.shrun, where theabsolute
--checkreported the regression. The variant whose entire purpose is catching a riseagainst the merge base did not catch one. Filed as #10713; the absolute
--checkis the onlyform this lane now trusts.
Validation
Validated head
e48eae9954. Five-package release build pinned and hash-verified, re-verifiedafter the gap run (
artifacts_match_pin_after_gap=True).lint1 of 83 — only the public-benchmark freshness step known-red onmain, read from thelog rather than inferred from the exit code.
test_gap_*count, none vacuous:
Issues closed by this train
A merge train closes its source PRs rather than merging them, so the
Fixes #Nkeywords in thosePR bodies never evaluate. They are carried here, on the PR that actually merges, so they fire:
Fixes #10466
Fixes #10437
Fixes #10661
Fixes #10689
Fixes #10683