perf(codegen, runtime): stop recording parameter-guard visits that can never be consulted

[proggeramlug]

Refs #8202.

js_param_type_guard runs on every unproven call into a guarded ordinary-parameter clone. #8201 moved the scalar descriptors to the typed-abi leaf guards; what stayed on the interpretive validator is the structural half, and #8202 priced its fixed per-call component: descriptor re-parse, a ~1 KB GuardState zero-init, and a linear seen_or_insert scan per node visited.

This removes two of those three.

The visited set was unconditional

Every container the walk touched went into the set — per array element. Validating p: { toks: Token[], pos: number } on every peek(p) therefore recorded one entry per token, and not one of them could ever be consulted: Token lies on no descriptor cycle and is reachable by exactly one path, so a second arrival at the same (address, node) pair is impossible.

The set is load-bearing for exactly two facts, and both are properties of the immutable compiler-emitted graph rather than of the value:

• termination — a value cycle (env.parent === env) can only walk forever through a node that reaches itself;
• no re-walk blowup — a node the traversal can enter twice with the same address must memoize, or a shared graph re-walks exponentially.

So the compiler decides it. visit_tracking_bits runs Tarjan over the graph it just built and propagates a saturating "ways in" count from the root, then sets the high bit of the op byte on exactly the container nodes that need recording. js_param_type_guard masks the op byte and reads the bit.

On interp.ts that is: peek(p: Parser) — 6 nodes, 0 tracked, was one record per token; asNum(v: Value) — 123 nodes, 15 tracked, exactly the recursive Node/Env cluster, 108 nodes stop recording. Descriptor length is unchanged (the bit rides in a byte that only ever held ops 0–16).

The magic goes PGT1 → PGT2, so a mismatched compiler/runtime pair fails closed on the magic — guard returns 0, caller takes the generic function — instead of reading a v1 blob as one that opts out of tracking everywhere.

GuardState zeroed 1 KB of stack per call

inline_visited is now MaybeUninit. Only [..inline_visited_len] is ever read, and after the change above most guarded calls never write a slot at all.

Measured

Instructions retired (/usr/bin/time -l), best-of-3, both arms built from their own tree with the same -p perry -p perry-runtime-static -p perry-stdlib-static, PERRY_RUNTIME_DIR pinned per arm, all 19 corpus stdouts byte-compared and exit-checked. iso_miss keeps misses 0.

Exactly 2 of the 19 rows emit a js_param_type_guard call site — interp and iso_miss, two sites each (asNum, peek). Both improve:

row	instructions	peak RSS
interp	13.814 B → 13.589 B (−1.63%)	−0.06%
iso_miss	16.489 B → 16.264 B (−1.36%)	+0.13%

Isolating the validator's own cost, by differencing against the same runtime archive (PGT1 blob + PGT2 runtime = every guard fails, so binary-layout effects cancel):

row	validator cost, main	with this change	delta	share of program
interp	1.671 B	1.422 B	−14.9%	12.05% → 10.45%
iso_miss	1.611 B	1.400 B	−13.1%	9.76% → 8.60%

The other 17 rows are not attributable, in either direction

The two arms' libperry_runtime.a differ in exactly two functions out of 11,185 in the crate's codegen unit — js_param_type_guard (808 → 316 bytes) and GuardState::matches (+28) — every other function byte-identical. The 17 rows that emit no guard call site execute neither, so their movement (pipeline −3.9%, retain_wide1 +0.6%, retain1 +0.3%, deeplist −0.5%, retain −0.3%, the remaining 12 within ±0.1%) is address-layout noise, not effect. Two main builds from identical source came out byte-identical (archive and perry binary alike), and repeat runs of one binary spread ~0.1%, so the build itself is deterministic and pipeline's ±4% is what an address-hash-sensitive program does when the heap moves. I am claiming none of it.

Tests

a_tree_shaped_descriptor_records_no_visits, a_container_on_a_cycle_records_its_visits, a_shared_container_records_its_visits (codegen, through the real encoder); a_tracked_node_terminates_on_a_cyclic_value — which also asserts the untracked form of the same descriptor conservatively returns 0 rather than hanging — an_untracked_shared_node_decides_the_same_way, and the_previous_descriptor_format_is_refused (runtime).

cargo test --release -p perry-runtime --lib -p perry-codegen green.

What this does NOT fix

#8202's premise was that the fixed per-call overhead dominates. It does not: it is ~15% of the validator's cost, and the validator is ~12% of interp. The structural walk is the other ~10.5pp, and the measurement in the issue thread shows the specialization it gates is worth ~0.2%. That is a policy question for #8094/#8079, not a per-call-overhead one — see the issue comment.

Summary by CodeRabbit

• Bug Fixes

  • Improved parameter validation for cyclic and shared data structures.
  • Added safeguards to limit repeated traversal and prevent excessive validation work.
  • Invalid or outdated guard descriptors now fail closed instead of being accepted.

• Performance

  • Reduced runtime tracking overhead by enabling visit tracking only where necessary.
  • Added bounded handling for deeply repeated or duplicated values.

[coderabbitai]

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 70befde1-3584-436e-ac93-b6059ebe03f5

📥 Commits

Reviewing files that changed from the base of the PR and between 09bbf03 and 3f8da1c.

📒 Files selected for processing (3)

• changelog.d/8238-param-guard-visit-tracking.md
• crates/perry-codegen/src/codegen/param_guard.rs
• crates/perry-runtime/src/param_type_guard.rs

---

📝 Walkthrough

Walkthrough

The compiler now marks only cyclic or multiply reachable descriptor containers for visit tracking. The runtime uses PGT2 metadata, lazy visit storage, and a cumulative traversal limit. Tests cover cycles, shared values, duplicated values, and rejection of PGT1 descriptors.

Changes

Parameter-guard visit tracking

Layer / File(s)	Summary
Compiler descriptor analysis and encoding crates/perry-codegen/src/codegen/param_guard.rs	The compiler detects cyclic and multiply reachable containers, sets OP_TRACK_VISIT during descriptor serialization, preserves scalar classification, and tests tracking decisions.
Runtime tracked traversal crates/perry-runtime/src/param_type_guard.rs	The runtime decodes PGT2 descriptors, memoizes visits only for marked array, tuple, object, map, and set nodes, and enforces MAX_VISITS.
Visit state and validation coverage crates/perry-runtime/src/param_type_guard.rs, changelog.d/8238-param-guard-visit-tracking.md	Visit slots use lazy initialization. Tests cover cyclic termination, shared-value revalidation, bounded duplicated-value traversal, and PGT1 rejection. The changelog documents the format and behavior changes.

Estimated code review effort: 4 (Complex) | ~45 minutes

Possibly related issues

• PerryTS/perry issue 8202 — The changes modify GuardState visit tracking and descriptor traversal to address the parameter-guard overhead described in the issue.

Possibly related PRs

• PerryTS/perry#8094 — This PR extends the parameter-guard descriptor format introduced there with cyclic and shared-node visit tracking.
• PerryTS/perry#8201 — Both changes modify parameter-descriptor classification and encoding, including scalar handling of opcode metadata.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch perf/8202-param-guard-overhead

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[proggeramlug]

Validation

• cargo test --release -p perry-runtime --lib -p perry-codegen --lib — green. Every pre-existing guard test still passes alongside the new ones: scalar_descriptor_rep_classifies_exactly_the_leaf_guard_ops, recursive_alias_serializes_as_a_finite_graph, a_class_descriptor_carries_its_class_id_and_its_declared_fields, and the four ordinary_param_guard_tests.
• Gap corpus A/B, Perry vs Perry. All 37 test_gap_*.ts that declare a structural parameter type — the only shapes that can reach the validator — compiled under both arms and byte-compared on stdout, stderr and exit code: 37 identical, 0 differing, 0 compile failures. Seven of them genuinely exercise the subject rather than merely not crashing, 30 guard call sites in total: test_gap_specabi_ordinary_param_guards (7), test_gap_7890_declared_array_receiver_element_read (7), test_gap_declared_field_type_refine_guarded (5), test_gap_repsel_element_shape_loop_clone (4), test_gap_7891_string_receiver_numeric_string_key (3), test_gap_repsel_element_shape_param_binding (2), test_gap_gc_alloc_point_no_move (2).
• 19-program perf corpus: all stdouts byte-compared, iso_miss keeps misses 0.
• cargo fmt --all -- --check, scripts/check_file_size.sh, addr_class_inventory.py, gc_runtime_root_holders.py, check_thread_locals.py — all clean.

Not run here: the full 569-test gap suite against node, and crates/perry-codegen/tests/native_proof_regressions.rs. The dev host is saturated (load average 110+ from unrelated jobs) and the gap runner was getting ~0.5 s of CPU per 10 minutes, so I stopped it rather than add to the contention; the 37-test structural subset above is the part of it that can reach this code.

Why the behavioural surface is narrow

The bit only controls memoization, so it cannot make the guard accept something main rejects. Over-tracking reproduces today's behaviour exactly. Under-tracking can only cost a memo hit — the walk then depth-caps and returns 0, the caller takes the generic function, and the observable result is unchanged. The magic bump is symmetric, so a mismatched compiler/runtime pair also just routes everything generic; that is the configuration the interp/iso_miss decomposition above measures, and it prints the same checksums.