perf(repsel): stop charging Perry's own cjs_wrap preamble to the Ptr<Shape> report (#7152)

[proggeramlug]

Closes the __cjs_module half of #7152.

--opt-report's Ptr<Shape> section stops charging Perry's own CommonJS
wrapper to the user's code. Over 195 real __esModule dependency modules
from real-apps/scriptc/node_modules (every third by sorted path, compiled
standalone, --opt-report=json --no-link, 195/195 in both arms), 31 % of all
Ptr<Shape> candidates — 379 of 1231 — are two statements the cjs_wrap
template emits itself.

denial bucket	base	after
rule 2 — bare reference	140	4
rule 1 — unbound alloc, constructor argument	196	8
rule 5 — module barrier	187	132
rule 1 — unbound alloc, all positions	885	759
total ptr-shape candidates	1231	914
selected / consumed	3 / 11	3 / 11

Named, not inferred: 136 of the 140 rule-2 bare references were the local
__cjs_module, as were 55 of the rule-5 denials, and 188 of the 194
constructor-argument object literal { ... } rows were the {} inside
const __cjs_module = { exports: {} }. Zero __cjs_module rows remain in any
bucket. The two arms compile the same 195 modules and fail on the same 2
(rollup/dist/shared/index.js, @vitest/utils/dist/chunk-_commonjsHelpers.js,
identical in both — pre-existing).

1. The shape, and why the reference is NOT exemptable

cjs_wrap's preamble (wrap.rs, the cjs_preamble literal) emits into every
wrapped module:

const __cjs_module = { exports: {} };   // rule-1 seed; inner {} = an unbound alloc
var module = __cjs_module;              // ← THE rule-2 "bare reference"
var exports = __cjs_module.exports;
…
require.main = module;

Confirmed in HIR (--print-hir on a wrapped CJS dependency):

Let { id: 12, name: "__cjs_module", mutable: false,
      init: Some(New { class_name: "__AnonShape_5d9c…",
                       args: [New { class_name: "__AnonShape_a298…", args: [] }] }) }
Let { id:  6, name: "module", mutable: true, init: Some(LocalGet(12)) }
…
Expr(PutValueSet { target: LocalGet(10 /*require*/), key: "main", value: LocalGet(6) })

__cjs_module is a Stmt::Let of an object literal, so rule 1 seeds it; the
next statement aliases it into a var, so rule 2 denies it as a bare
reference.

It cannot be exempted the way #7139 exempted the defineProperty sites.
module is a reassignable var that the preamble itself stores into
require.main, and that CommonJS bodies write module.exports = X through —
real packages also read module.id / .filename / .loaded / .parent, any
of which transitions the shape. The record genuinely and permanently escapes;
no narrowing of rule 2 could promote it, and a guard-free fixed-offset load on
it would be wrong.

What is wrong is that it was ever a candidate. Promotion would buy nothing
even if it were provable — a one-field record, read three times at module init,
loop depth 0, no numeric field. So this suppresses it at the seed rather than
exempting it at the rule, which is what #7152 itself proposed ("if it is not, it
should not be a candidate").

2. The recogniser (collectors/cjs_scaffolding.rs, extending #7139's)

A region is a CommonJS preamble when its top level carries all of:

• R1 a mutable: false Stmt::Let named __cjs_module, initialized by an
  Expr::New of an __AnonShape_… class;
• R2 whose args are exactly one argument-less __AnonShape_… allocation —
  the template's { exports: {} };
• R3 uniquely in the region (two ⇒ ambiguous ⇒ recognise neither);
• R4 a mutable: true Stmt::Let named module whose init is a bare
  Expr::LocalGet of that record.

Soundness, per conjunct

This is a candidate SUPPRESSION, not a proof relaxation — the opposite
direction from #7139. Dropping a candidate can only remove facts, never add
one, so it cannot make codegen unsound. The whole obligation is to show it
never removes a fact that would otherwise exist.

R4 discharges that outright, and is the only conjunct that carries soundness
weight. It is not a heuristic about the template — it is the denial:
Stmt::Let { mutable: true, init: Some(LocalGet(record)) } takes UseWalk's
plain-local branch, walks the init under the default escape context, hits the
Expr::LocalGet arm and calls disq(record, ESC_BARE_REFERENCE) on every
path; mutable: true is what keeps the alias pre-pass from tracking it
instead. A region satisfying R4 therefore cannot promote its record, so
removing it from the candidate set leaves the returned HashMap bit-identical.

R1-R3 are hygiene, not soundness. They pin recognition to the template's
exact literal so a user binding that merely shares the name keeps its
candidacy. If any of them drifts the candidate simply reappears in the report —
the failure direction is more candidates, never a lost promotion.

The claim is also tested rather than asserted:
a_var_alias_denies_a_local_that_otherwise_promotes builds a Point local
that IS promoted, inserts exactly R4's statement, and shows the promotion
disappear.

3. Why the whole preamble, not just the record

--opt-report dedups alloc-site rows per function on
(module, function, name, position, analysis, outcome, rule), and every object
literal renders as object literal { ... }. So each wrapped module contributes
exactly one alloc-site row, and which context it reports is whichever
preamble literal the walk reaches first. Suppressing only the record would have
moved the row from constructor argument to statement and changed the total
by zero.

So the recognised region also stops walking the four allocating preamble
statements behind the record: the two defineProperty sites #7139 already
recognises (reusing CjsScaffolding::exempts_shape_barrier verbatim, so
the two exemptions cannot disagree about what scaffolding is) plus
require.cache = {} and require.extensions = { … }. That half is report-only
in the strongest sense — unbound_new_sites is called exclusively under
opt_report::enabled().

The same dedup is why statement rises 273 → 320 and call argument 175 →
187: with the scaffolding row gone, 62 real user allocations that were being
masked by it become visible. Removing 379 scaffolding rows made 62 real ones
appear; net 317.

4. Evidence

Codegen-neutral, measured on emitted IR. Object-file hashes are not
usable here and saying so is part of the result: the same compiler run twice on
the same input differs by ~2 KB, because the --no-link temp output directory
(perry-objs-<pid>-<ts>) lands in the Mach-O debug records. So the comparison
is on --trace llvm output over 49 dependency modules, each with a
same-compiler control run, normalising exactly one pre-existing
nondeterminism (HashMap-ordered js_register_function_name string constants —
16 of ~12 900 lines, itself inside the CJS preamble) and nothing else.

Behaviour, against the pinned oracle. A CJS dependency fixture
(__esModule, class with methods, record literal, 1000-iteration loop) built as
a compilePackages dependency: both arms produce byte-identical program output,
byte-exact against Node 26.5.1. Anti-vacuity on the same fixture — the
report goes 5 candidates / 2 denied → 3 candidates / 0 denied, with the
same three selections and the same consumption sites.

Sabotage matrix — 16 rows, control green in all 17 runs.

sabotage	red set
S1 R1 record mutable: false	a_mutable_record_binding_is_not_recognised
S2 R1 record binding name	a_differently_named_record_binding_is_not_recognised
S3 R1 outer is an object literal	a_record_of_a_declared_class_is_not_recognised
S4 R2 exactly one field	a_record_literal_with_a_second_field_is_not_recognised
S5 R2 inner literal is empty	a_record_whose_exports_literal_is_not_empty_is_not_recognised
S6 R3 exactly one record per region	two_record_bindings_in_one_region_are_ambiguous
S7 R4 the alias must exist	without_the_module_alias_the_record_is_not_recognised, a_const_module_alias_does_not_satisfy_r4, a_module_alias_of_another_local_does_not_satisfy_r4, an_unrecognised_region_suppresses_nothing
S8 R4 the alias is mutable: true	a_const_module_alias_does_not_satisfy_r4
S9 R4 the alias is of THIS record	a_module_alias_of_another_local_does_not_satisfy_r4
S10 the two require literal keys	user_allocations_are_never_suppressed
S12 the candidate suppression	the_record_is_not_a_ptr_shape_candidate
S13 the alloc-site skip	the_report_walk_drops_the_scaffolding_allocations_only
T1 template: record name	canary: the_cjs_preamble_is_still_recognised_as_scaffolding_allocation
T2 template: the var module alias	canary (same)
T3 template: the { exports: {} } literal	canary (same)
T4 template: require.cache	canary (same)

The first pass had four GREEN holes, and all four were fixed in the code, not
in the tests. The binding name and the record-uniqueness count each had two
enforcement points that masked one another — either could be deleted with every
test still green — so each conjunct now has exactly one. And "is a record
recognised?" was a removable if self.record.is_none() { return false; } that
no sabotage could kill, because Default's scaffolding sets were empty anyway;
the record and its bindings now live in one Option<(u32, CjsScaffolding)>, so
"unrecognised but still suppressing" has no representation and the losing
mode does not compile. That row is gone from the matrix rather than passing
vacuously.

Template canary (cjs_wrap/preamble_canary_tests.rs, extending #7139's):
wrap → parse → lower → perry_codegen::cjs_preamble_census, with one
anti-vacuity assertion per conjunct so a template edit names the conjunct it
broke, an exact expected count of recognised preamble statements, and a
negative control that a module which was never cjs_wrapped recognises
nothing.

Gates: cargo test -p perry-codegen --lib 472 passed (30 new);
cargo test -p perry --bin perry preamble_canary 4 passed;
compiler_output_regression.py census --gate green with no floor moved;
cargo fmt --all -- --check clean; scripts/check_file_size.sh adds no file to
the over-2000 list (ptr_shape.rs was pushed to 2005 by this change and
brought back to 1991 by collapsing the two early-bail arms).

5. What this does NOT do — stated plainly

Conversion to selected/consumed is ZERO, and that is by construction: R4
asserts every suppressed value was already denied. selected and consumed
are 3 and 11 in both arms, on every module. This PR moves no code, retires no
guard, and makes nothing faster. It removes 379 rows of Perry's own scaffolding
from a report whose readings scheduled #7149.

The dependency-JS wall is still rule 1 — allocations never bound to a local
— but its size needs restating rather than subtracting. In this corpus rule 1
goes 885 → 759 (188 scaffolding rows out, 62 masked user rows in). #7152's
506 is a different 180-module sample; by the same per-module rate roughly 180
of those 506 are this same literal, but the residual is not 506 − 180
because of the unmasking above. Re-derive it against this compiler rather than
subtracting.

Unmeasured / caveats, in full:

• gc_repsel_matrix.sh --arms all --pressure 8 was NOT run to completion.
  It was started and killed during its compile phase: a sibling agent (repsel Phase 5a: route the class-id dispatch tower to __pshape under an inline keys check (batch.ts: 4 guard calls + 3 by-name field calls per call) #7142)
  was running the identical 21-arm matrix concurrently and free disk on the
  host had fallen from 52 GB to 13 GB, below the campaign's 15 GB floor. I
  aborted mine so theirs could finish. The substitute evidence is §4's IR
  comparison, which for a change that emits no code is the stronger statement:
  the matrix asks "does behaviour survive 21 GC arms", the IR diff answers
  "there is no behaviour to survive them, the emitted code is identical". Worth
  re-running before merge if a reviewer wants the arm-liveness columns.
• Byte-for-byte object neutrality is not claimed — it is not measurable
  across processes on this host (see §4). IR neutrality is.
• 2 of the 49 IR-compared modules were excluded, not passed. Their
  same-compiler control run differs: perry_method_<Class>__<name> call
  emission (and the matching class-id compare constants) is HashMap-ordered, so
  a module with the same method name on several classes — vite's
  module-runner.js (EvaluatedModules.clear / HMRClient.clear), rollup's
  shared/index.js — reorders between runs of the same binary. That is a
  pre-existing compiler nondeterminism in the codegen: object emission is nondeterministic on Linux — the LLVM temp module name embeds pid + nanotime #7131 family, not introduced
  here; the harness reports those cells as control-failed rather than
  counting them as identical, and no verdict is claimed for them.
• The corpus is 197 selected paths of which 195 compile; it is not perf(repsel): open the array-element escape for Ptr<Shape> (#7034 §3) #7149's
  180-module set (that tree has drifted), so base-arm counts here are not
  directly comparable to the numbers in repsel: dependency JS is walled by rule 1 (unbound allocations), not containment — 506 of 746 candidates #7152's table. Both arms in this PR
  were measured on the same list with the same harness.
• Both arms of the table above are pinned at df7214b0d — the commit this
  branch was cut from — differing only by this patch. The branch has since been
  rebased onto 5f5006aa6; a spot re-measurement on the rebased HEAD keeps the
  scaffolding result exactly (rule-2 bare reference 4, zero
  __cjs_module rows in any bucket) but moves the other buckets, because an
  unrelated main change made rollup/dist/shared/index.js — a large module
  that failed to compile in both original arms — compile, adding ~109 denials
  of its own. That is a different baseline, not a different result, and
  mixing the two would be exactly the kind of A/B this campaign keeps warning
  about. The pinned pair is the measurement; the re-run is a liveness check on
  it.
• The recogniser is region-scoped and top-level-only, matching the template. A
  future cjs_wrap that emits the preamble inside a conditional would stop
  being recognised — degrading to today's behaviour, and the canary would go
  red.

[coderabbitai]

Warning

Review limit reached

@proggeramlug, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 5 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: ad543f6f-4b6b-4446-8d6b-2ea13a492cb6

📥 Commits

Reviewing files that changed from the base of the PR and between 5f5006a and 698c604.

📒 Files selected for processing (7)

• changelog.d/7171-cjs-preamble-ptr-shape-report-noise.md
• crates/perry-codegen/src/collectors/cjs_scaffolding.rs
• crates/perry-codegen/src/collectors/mod.rs
• crates/perry-codegen/src/collectors/ptr_shape.rs
• crates/perry-codegen/src/collectors/ptr_shape_report.rs
• crates/perry-codegen/src/lib.rs
• crates/perry/src/commands/compile/cjs_wrap/preamble_canary_tests.rs

---

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.}