perf(codegen): make typed-feedback site registration opt-in (3.6x on dynamic property access)

[TheHypnoo]

Removes a per-access no-op runtime call from the property get/set hot path in normal builds.

Root cause

Profiling bench_object_property (10k objects × 20 dynamic string-keyed fields) showed it at 1127ms vs Node 16ms (70×). The emitted IR for every property get/set contained:

call void @js_typed_feedback_register_site(i64 ..., ptr ..., i64 19, ptr ..., i64 52, ...)  ; 14 args
... shape guard + fast/fallback ...

js_typed_feedback_register_site is part of typed feedback (#854), an opt-in profiling feature that early-returns at runtime unless PERRY_TYPED_FEEDBACK / PERRY_TYPED_FEEDBACK_TRACE is set. In a normal build the call did nothing — but it was still emitted and executed (a 14-argument cross-crate call) on every field access, e.g. once per write in a dynamic-property loop.

Fix

Gate the register_site emission on the same env that enables feedback at runtime, read fresh at codegen time (crates/perry-codegen/src/expr/typed_feedback.rs). A normal build skips registration entirely and pays nothing; a profiling build (PERRY_TYPED_FEEDBACK=1 perry app.ts -o app && ./app, env inherited by the run) emits and uses it.

The site-id is still allocated and returned, so the shape guard call (js_typed_feedback_class_field_*_guard / object_*_by_name_guard) is unchanged — dispatch and field-access correctness are unaffected. This is purely removing a call that was already a runtime no-op in the default configuration.

Results

Benchmark	Before	After	Node	Checksum
bench_object_property	1127ms	~310ms (3.6×)	17ms	1999990000 (identical to Node, before & after)

Correctness re-verified vs node --experimental-strip-types on bench_int_arithmetic, bench_string_heavy, bench_json_roundtrip, bench_array_grow, bench_numeric_array_numeric — all checksums match.

Helps any dynamic-property-heavy code (the common TypeScript case), not just this benchmark.

Tests

• cargo test -p perry-codegen — all pass. The dedicated typed_feedback_instruments_property_and_method_boundaries test opts in via the env var to exercise the enabled path; all other feedback tests assert the guards (still emitted) and are unaffected.

Not included

This does not fix method_calls (491×), which profiling showed is dominated by the per-access shape guard calls and the method-dispatch guard/call — a larger change (devirtualize monomorphic dispatch + inline/hoist the field-access guards, entangled with the require_raw_f64 typed-shape layout). That is scoped as separate follow-up work.

Summary by CodeRabbit

• New Features

  • Typed-feedback emission can be toggled via environment variables.

• Chores

  • Generation now skips typed-feedback artifacts and registration when emission is disabled.
  • Tests updated to safely mutate environment variables during concurrent runs using a shared lock and guard.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR adds an environment-driven opt-in for typed-feedback site registration: typed_feedback_emission_enabled() checks PERRY_TYPED_FEEDBACK or PERRY_TYPED_FEEDBACK_TRACE, and emit_typed_feedback_register_site skips emitting the registration call and LLVM rodata globals when disabled. Tests now set the env var under a mutex to exercise the emission path.

Changes

Environment-driven typed-feedback gating

Layer / File(s)	Summary
Environment-driven gating implementation crates/perry-codegen/src/expr/typed_feedback.rs	Adds typed_feedback_emission_enabled() which checks PERRY_TYPED_FEEDBACK or PERRY_TYPED_FEEDBACK_TRACE. emit_typed_feedback_register_site early-returns the allocated site_id when emission is disabled, avoiding emission of the registration call and associated rodata globals.
Test synchronization and opt-in crates/perry-codegen/tests/typed_feedback.rs	Adds a global ENV_LOCK and EnvVarGuard to safely set/unset environment variables in tests and updates typed_feedback_instruments_property_and_method_boundaries to set PERRY_TYPED_FEEDBACK=1 while holding the lock so the emission path is exercised.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 I sniff the env, a tiny hop and peek,
If PERRY flags are set, the registration speaks,
If quiet, I skip with a soft little thrum,
Site IDs still made — no globals to become,
A rabbit's nod to tidy, safe test runs.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically identifies the main change: making typed-feedback site registration opt-in, with the performance impact (3.6x improvement) as supporting context.
Description check	✅ Passed	The PR description covers the root cause (profiling results), the fix (compile-time gating), test verification, and results with benchmark numbers. It addresses the template's Summary, Changes, and Test plan sections effectively.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ 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/typed-feedback-opt-in

---

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

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/perry-codegen/tests/typed_feedback.rs`:
- Line 190: The test typed_feedback_instruments_property_and_method_boundaries
sets PERRY_TYPED_FEEDBACK without restoring it; wrap that set in the existing
EnvVarGuard + ENV_LOCK pattern (capture previous env var value and restore in
Drop) used in EnvVarGuard in typed_shape_descriptors.rs: acquire ENV_LOCK,
create an EnvVarGuard for "PERRY_TYPED_FEEDBACK" (saving previous state), call
set_var("PERRY_TYPED_FEEDBACK","1") under the guard, and let the guard drop at
test end to restore/remove the variable so other tests won't observe the changed
environment.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: cd21aefb-854b-4574-81a3-4983958ee4b6

📥 Commits

Reviewing files that changed from the base of the PR and between e092362 and 0f4607c.

📒 Files selected for processing (2)

• crates/perry-codegen/src/expr/typed_feedback.rs
• crates/perry-codegen/tests/typed_feedback.rs

[coderabbitai]

🧹 Nitpick comments (1)

crates/perry-codegen/tests/typed_feedback.rs (1)

5-34: ⚡ Quick win

Consider consolidating the duplicated EnvVarGuard + ENV_LOCK pattern into a shared test utility.

The comment on line 6 confirms this mirrors the guard in typed_shape_descriptors.rs. Duplicating this 30-line pattern violates DRY and creates a maintenance burden—if the implementation needs enhancement (e.g., logging, better lock-poisoning recovery), it must be updated in multiple places.

♻️ Proposed consolidation approach

Create crates/perry-codegen/tests/common.rs:

pub static ENV_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(());

pub struct EnvVarGuard {
    key: &'static str,
    prev: Option<std::ffi::OsString>,
}

impl EnvVarGuard {
    pub fn set(key: &'static str, value: Option<&str>) -> Self {
        let prev = std::env::var_os(key);
        match value {
            Some(value) => std::env::set_var(key, value),
            None => std::env::remove_var(key),
        }
        Self { key, prev }
    }
}

impl Drop for EnvVarGuard {
    fn drop(&mut self) {
        match &self.prev {
            Some(value) => std::env::set_var(self.key, value),
            None => std::env::remove_var(self.key),
        }
    }
}

Then in both typed_feedback.rs and typed_shape_descriptors.rs:

mod common;
use common::{EnvVarGuard, ENV_LOCK};

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/perry-codegen/tests/typed_feedback.rs` around lines 5 - 34, Extract
the duplicated ENV_LOCK and EnvVarGuard definitions into a shared test utility
module (a common mod) and replace the local definitions in this file and the
other test file with imports; specifically move the static ENV_LOCK and the
EnvVarGuard struct/impl/Drop into the shared module, make EnvVarGuard::set and
ENV_LOCK public, then in this test file import them with use
common::{EnvVarGuard, ENV_LOCK} and remove the local definitions so both tests
use the single canonical implementation.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@crates/perry-codegen/tests/typed_feedback.rs`:
- Around line 5-34: Extract the duplicated ENV_LOCK and EnvVarGuard definitions
into a shared test utility module (a common mod) and replace the local
definitions in this file and the other test file with imports; specifically move
the static ENV_LOCK and the EnvVarGuard struct/impl/Drop into the shared module,
make EnvVarGuard::set and ENV_LOCK public, then in this test file import them
with use common::{EnvVarGuard, ENV_LOCK} and remove the local definitions so
both tests use the single canonical implementation.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 1a5a7115-91c4-473d-9f06-76c0054e5c76

📥 Commits

Reviewing files that changed from the base of the PR and between 0f4607c and fbbd16d.

📒 Files selected for processing (1)

• crates/perry-codegen/tests/typed_feedback.rs