perf(codegen): decide scalar parameter descriptors with the typed-abi leaf guards - #8201
Conversation
… leaf guards (#8079) The public spec wrapper validated EVERY ordinary boxed parameter through the interpretive js_param_type_guard, whose fixed per-call cost (descriptor parse plus a 768-byte GuardState init) is ~450 instructions even for a bare `number`. Every unproven direct call pays it — including a Tier-B guarded clone's own recursion, which re-enters through the wrapper (#8169). Measured at bfb0707, best-of-5 instructions retired: the validator accounted for 33.8% of tree, 19.6% of interp, 17.0% of tree_wide and 16.3% of iso_miss. A single-node scalar descriptor is now decided by the existing typed-abi leaf guard with the exact same predicate: OP_NUMBER = js_typed_f64_arg_guard (is_number || is_int32) OP_INT32 — the validator already calls js_typed_i32_arg_guard OP_BOOLEAN = js_typed_i1_arg_guard (TAG_TRUE | TAG_FALSE) OP_STRING = js_typed_string_arg_guard (is_any_string) Predicate equality is the soundness argument: routing (clone vs generic fallback) is bit-for-bit the decision the validator would have made, so no annotation lie can reach the clone that could not before. Structural descriptors — unions, objects, arrays, literals — keep the interpretive call, and their rodata blobs; scalar ones no longer emit rodata. The layout check in scalar_descriptor_rep is exhaustive, so a future descriptor-format change falls back to the validator instead of misreading bytes. Measured (19-program corpus, quiet-host instructions retired, best-of-5, all stdout byte-exact, peak RSS unchanged on every row): tree 15.82B -> 10.79B (-31.8%) tree_wide 31.59B -> 26.55B (-15.9%) interp 15.05B -> 13.80B (-8.3%) iso_miss 17.70B -> 16.47B (-6.9%) every other row within noise The interp/iso_miss residual is the structural share (asNum's Value union, peek's Parser object) plus the Tier-B self-recursion wrapper round-trip, which is #8169. Validation: cargo test -p perry-codegen --no-fail-fast reproduces exactly the 9 pre-existing failures by name (baseline verified on pristine bfb0707 in the same target dir); perry-runtime --lib 2499/0/4; perry --bin perry 987/0. fmt, file-size, addr-class, binding-type, raw-handle, shape-census, class-id, gc knob/wiring and workspace-architecture gates all pass.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughScalar ordinary-parameter guards now use typed ABI leaf guards for supported scalar descriptors. Non-scalar descriptors retain ChangesScalar guard specialization
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: ✨ 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 |
Mini cross-check (canonical interleaved sweep, run7)Instrumented instructions retired on the quiet-mini protocol reproduce the PR's local numbers exactly; instructions are the contention-immune metric, and the wall-time column is under a +4.9% mean ambient slowdown (another session's
tree's wall gain exceeding its instruction gain (−46% vs −32%) is the 768-byte Notes:
Validation is complete from my side: corpus byte-exact at every arm, codegen suite reproduces exactly the 9 pre-existing failures by name, runtime/bin suites green, all script gates pass. |
…ject (#8242) `js_param_type_guard` re-derived three per-object facts for every descriptor field it looked up, and re-derived all of them again for every arm of a union — which tries its arms against the SAME value. None of that decided anything. - The accessor probe ran per field. `get_accessor_descriptor` per field per call costs a UTF-8 validation plus a key hash before its own #6759 prefilter can say no; `owner_may_have_descriptor_entries` answers "does this object own ANY accessor" from one meta word, and it is false for every ordinary object. Equivalent, not merely conservative: summary false implies `accessor_key_bits == 0`, so the per-key bit test could only have been false too, and a non-meta-capable owner still reports true and keeps the per-field probe. - The `keys_array` header was re-validated per field, so a two-field object paid for `try_read_gc_header` plus the whole length/capacity/ size arithmetic twice. `object_keys` resolves it once per object. - A union re-ran `plain_object` per arm, including the ShapeId probe that #8125 calls the object model's hottest lookup. A one-entry cache keyed on the NaN-box bits serves the retries; validation runs no JavaScript and allocates nothing, so nothing between two arms can move or mutate the object. Also hoists `table_end` into `Descriptor`, which `node` recomputed from `node_count` on every node visit. Measured on the 19-program corpus (instructions retired, best-of-5, interleaved, stdout/stderr byte-exact, peak RSS unchanged on every row): interp -1.51%, iso_miss -1.24%, all 17 other rows within 0.2%. This is a partial fix for #8202: a SKIPVAL arm prices the whole validator at -13.36% / -11.04%, so the residual reproduces, but its two headline mechanisms do not exist. The "768-byte GuardState init" is never paid (LLVM already elides the zero-init; the function opens `sub sp, sp, #0x470` plus a single `str xzr, [sp]`), and `Descriptor::parse` ablates to -0.26%. The remaining ~11pp is the interpretive walk itself, spread thin with no dominant term. Refs #8202, #8201, #8169. Co-authored-by: Ralph Küpper <ralph@skelpo.com>
What this is
The #8079 re-measurement (details in the issue comment to follow), and the fix for the mechanism it actually found: the public spec wrapper decides scalar ordinary-parameter descriptors with the existing typed-abi leaf guards instead of the interpretive
js_param_type_guard.Why
js_param_type_guardcosts ~450 instructions per call before it checks anything —Descriptor::parseplus a 768-byteGuardStateinit — and every unproven direct call through the public wrapper pays it, including a Tier-B guarded clone's own recursion (#8169). Atbfb0707bethat interpretive overhead measured as:(SKIPVAL diagnostic arm: descriptor validation replaced by
true, routing unchanged, all stdout byte-exact.)Soundness
A single-node scalar descriptor's predicate is bit-for-bit an existing leaf guard's predicate:
OP_NUMBER=js_typed_f64_arg_guard=is_number || is_int32OP_INT32— the validator already delegates tojs_typed_i32_arg_guardOP_BOOLEAN=js_typed_i1_arg_guard=TAG_TRUE | TAG_FALSEOP_STRING=js_typed_string_arg_guard=is_any_stringRouting (clone vs generic fallback) is therefore exactly the decision the validator would have made — no annotation lie can reach the clone that could not before. Structural descriptors (unions, objects, arrays, literals) keep the interpretive call and their rodata.
scalar_descriptor_repchecks the descriptor layout exhaustively, so a future format change falls back to the validator rather than misreading bytes.Measured (19-program corpus, instructions retired, best-of-5, byte-exact stdout, peak RSS unchanged on every row)
bfb0707beThe interp/iso_miss residual is the structural share (
asNum'sValueunion,peek'sParserobject) plus the Tier-B self-recursion wrapper round-trip — that half is #8169, deliberately not touched here.Validation
cargo test -p perry-codegen --no-fail-fast: exactly the 9 pre-existing failures by name (baseline re-run on pristinebfb0707bein the same target dir); the two ordinary-param-guard assertions that tracked the old wrapper shape now assert the stronger one (no interpretive call for scalars, leaf guards present,$genericfallback intact).scalar_descriptor_rep_classifies_exactly_the_leaf_guard_opsbuilds descriptors through the real encoder — a layout change goes red instead of silently misclassifying.cargo test -p perry-runtime --lib2499/0/4 ignored;cargo test -p perry --bin perry987/0.global_this_webassembly.rsdrift, not this PR — left for its owner), shape-descriptor census, class-id, GC knob drift, GC gate wiring, workspace architecture,gc_root_dominance_check.py --audit-poll-reach.expected/at every arm.Refs #8079. The wrapper-recursion half is #8169.
Summary by CodeRabbit
Performance
Bug Fixes
Tests