-
-
Notifications
You must be signed in to change notification settings - Fork 161
feat(cli): --opt-report — surface which values could not be statically typed, why, and whether the developer can fix it #7037
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,137 @@ | ||
| // Pattern: batch-process a stream of records — map to reshaped objects, | ||
| // sort by a derived key, group/reduce into summary rows. Models the | ||
| // object/property-heavy middle of a request handler, an ETL step, or a | ||
| // report builder: allocation-heavy, property-access-dominated, and every | ||
| // record *escapes* its producing scope (pushed, returned, or passed to a | ||
| // callback). | ||
| // | ||
| // This is the workload #7034 measured `Ptr<Shape>` promotion against and | ||
| // found **zero** promoted locals. It is committed here so that | ||
| // `perry <this file> --opt-report` reproduces that finding in one command | ||
| // and names the rule that denied each candidate. Keep the escape shapes | ||
| // below intact — they are the point of the fixture, not incidental style. | ||
|
|
||
| const N = 40_000; | ||
|
|
||
| class Row { | ||
| id: number; | ||
| bucket: string; | ||
| weight: number; | ||
| score: number; | ||
|
|
||
| constructor(id: number, bucket: string, weight: number) { | ||
| this.id = id; | ||
| this.bucket = bucket; | ||
| this.weight = weight; | ||
| this.score = 0; | ||
| } | ||
|
|
||
| rescore(factor: number): number { | ||
| this.score = this.weight * factor + this.id % 7; | ||
| return this.score; | ||
| } | ||
| } | ||
|
|
||
| interface Shaped { | ||
| key: string; | ||
| value: number; | ||
| tag: string; | ||
| } | ||
|
|
||
| interface Summary { | ||
| bucket: string; | ||
| count: number; | ||
| total: number; | ||
| peak: number; | ||
| } | ||
|
|
||
| const BUCKETS = ["alpha", "beta", "gamma", "delta"]; | ||
|
|
||
| // Producer: every `new Row(...)` escapes into the array (rule 2 — the | ||
| // local is used as a call argument). | ||
| function buildRows(n: number): Row[] { | ||
| const rows: Row[] = []; | ||
| for (let i = 0; i < n; i++) { | ||
| const row = new Row(i, BUCKETS[i % 4], (i % 97) * 0.5); | ||
| rows.push(row); | ||
| } | ||
| return rows; | ||
| } | ||
|
|
||
| // Reshape: the `.map(x => ({...}))` idiom — the record is produced in a | ||
| // closure body and returned, never bound to a local at all. | ||
| function shape(rows: Row[]): Shaped[] { | ||
| return rows.map((r) => ({ | ||
| key: r.bucket + ":" + r.id, | ||
| value: r.rescore(1.5), | ||
| tag: r.id % 2 === 0 ? "even" : "odd", | ||
| })); | ||
| } | ||
|
|
||
| // Sort by a derived key — the comparator is a closure body invoked once | ||
| // per comparison, with no loop of its own. | ||
| function ranked(items: Shaped[]): Shaped[] { | ||
| const copy = items.slice(); | ||
| copy.sort((a, b) => { | ||
| if (a.value !== b.value) { | ||
| return b.value - a.value; | ||
| } | ||
| return a.key < b.key ? -1 : a.key > b.key ? 1 : 0; | ||
| }); | ||
| return copy; | ||
| } | ||
|
|
||
| // Reduce into per-bucket summaries — the accumulator object is returned | ||
| // from the reducer (rule 2 — return disqualifies). | ||
| function summarize(rows: Row[]): Summary[] { | ||
| const byBucket: Summary[] = []; | ||
| for (let b = 0; b < BUCKETS.length; b++) { | ||
| const s: Summary = { | ||
| bucket: BUCKETS[b], | ||
| count: 0, | ||
| total: 0, | ||
| peak: 0, | ||
| }; | ||
| byBucket.push(s); | ||
| } | ||
| rows.reduce((acc: Summary[], r: Row) => { | ||
| const idx = BUCKETS.indexOf(r.bucket); | ||
| const slot = acc[idx]; | ||
| slot.count = slot.count + 1; | ||
| slot.total = slot.total + r.score; | ||
| if (r.score > slot.peak) { | ||
| slot.peak = r.score; | ||
| } | ||
| return acc; | ||
| }, byBucket); | ||
| return byBucket; | ||
| } | ||
|
|
||
| // Fold the summaries into one totals record and hand it back — the single | ||
| // most common record-producing idiom in real TypeScript (#7034 §4), and | ||
| // another rule-2 denial: the accumulator is returned. | ||
| function totalsRow(summaries: Summary[]): Row { | ||
| const acc = new Row(0, "acc", 0); | ||
| for (let i = 0; i < summaries.length; i++) { | ||
| acc.weight = acc.weight + summaries[i].total; | ||
| acc.score = acc.score + summaries[i].peak; | ||
| } | ||
| return acc; | ||
| } | ||
|
|
||
| const rows = buildRows(N); | ||
| const shaped = shape(rows); | ||
| const top = ranked(shaped); | ||
| const summaries = summarize(rows); | ||
|
|
||
| let checksum = 0; | ||
| for (let i = 0; i < 16; i++) { | ||
| checksum = checksum + top[i].value; | ||
| } | ||
| for (let i = 0; i < summaries.length; i++) { | ||
| checksum = checksum + summaries[i].count + summaries[i].peak; | ||
| } | ||
| const totals = totalsRow(summaries); | ||
| checksum = checksum + totals.weight + totals.score; | ||
|
|
||
| console.log("checksum: " + checksum.toFixed(4)); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| **`--opt-report`: see which values Perry could not statically type, and why (#6952).** Perry's speed comes from proving static types and selecting unboxed representations; when a proof fails the value stays NaN-boxed and the fast paths silently do not fire. Until now the only way to find out *which* values failed was to read LLVM IR — which is how #7034 discovered, by accident, that `Ptr<Shape>` promotes **zero** locals on the object-heavy workload that motivates it. `perry compile app.ts --opt-report` now answers that in one command: per value it reports the position (local / param / return / allocation site), the representation it got, the collector rule that denied it in that collector's own numbering, an actionability tier (fixable in your source / inherently polymorphic / Perry limitation, with the tracking issue), and a static hotness proxy. Wins are reported alongside the misses so the ratio is visible. `--opt-report=json` emits a stable schema (`schema_version: 1`) that CI can diff between builds to catch a representation silently regressing to zero. | ||
|
|
||
| Covers `Ptr<Shape>` locals and allocation sites (all five rules plus class admission, with the escape *kind* discriminated — reassignment, closure capture, call argument, return and container element are five different fixes), canonical i32/u32/Str locals, and the specialized-ABI entry decision, which is the only place params and returns carry a representation today. Allocation sites that are never bound to a local — the `.map(x => ({...}))` idiom that provenance structurally cannot see, and the majority of records in real code — are reported too. Hotness is reported as loop depth **and** iterating-builtin-callback status in separate columns, never collapsed: #7034 measured that 208 of 247 guard sites in the motivating program sit in callback bodies with zero loop depth, so a loop-nesting proxy alone ranks exactly the hot sites last. | ||
|
|
||
| Off by default and observational only: emitted LLVM IR is byte-identical with the flag on and off (verified on two programs), so it is deliberately not part of the object-cache key. Like `--trace llvm` it disables build/object cache reuse for its own run, because a cache hit skips codegen and there would be nothing to report. Values are identified by function and binding name — HIR keeps names through lowering but drops source positions; the `LocalId → Span` side-table that would add `file:line` and source snippets is #7036. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -592,6 +592,10 @@ pub(super) fn compile_function( | |
| .collect() | ||
| }) | ||
| .unwrap_or_default(); | ||
| // `--opt-report` (#6952): attribute every representation decision the | ||
| // collectors below make to this function. No-op when the report is off. | ||
| let _opt_report_scope = | ||
| crate::opt_report::enter_region(&f.name, crate::opt_report::RegionKind::Function); | ||
| let native_facts = crate::collectors::collect_native_region_fact_graph_with_spec_lens( | ||
| &f.body, | ||
| &f.params, | ||
|
|
@@ -609,6 +613,24 @@ pub(super) fn compile_function( | |
| ); | ||
|
|
||
| if let Some(plan) = spec_entry { | ||
| // `--opt-report` (#6952): the spec-ABI win, recorded at the same site | ||
| // as the PERRY_REPSEL_DEBUG line so the two cannot diverge. | ||
| if crate::opt_report::enabled() { | ||
| crate::opt_report::select( | ||
| crate::opt_report::Position::Param, | ||
| "(parameters + return)", | ||
| None, | ||
| crate::opt_report::Analysis::SpecAbi, | ||
| &plan | ||
| .reps | ||
| .iter() | ||
| .map(|r| r.label().to_string()) | ||
| .collect::<Vec<_>>() | ||
| .join(","), | ||
| 0, | ||
| Some(format!("specialized entry for {}", f.name)), | ||
| ); | ||
| } | ||
|
Comment on lines
+616
to
+633
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win Do not report an un-specialized return as selected.
🤖 Prompt for AI Agents |
||
| if std::env::var("PERRY_REPSEL_DEBUG").as_deref() == Ok("1") { | ||
| eprintln!( | ||
| "repsel: spec entry '{}' tuple=[{}] [{}]", | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -58,6 +58,7 @@ mod opts; | |
| mod spec_abi; | ||
| mod string_pool; | ||
| mod typed_abi; | ||
| mod typed_abi_opt_report; | ||
|
|
||
| pub(crate) use closure::emit_typed_string_capture_guard; | ||
| pub use helpers::resolve_target_triple; | ||
|
|
@@ -127,6 +128,33 @@ fn record_typed_clone_rejection( | |
| if !should_record_typed_clone_rejection(reason) { | ||
| return; | ||
| } | ||
| let source_function = source_function.into(); | ||
| // `--opt-report` (#6952): surface the specialized-ABI (RFC Phase 2) | ||
| // decision, which is the only place params and returns get a | ||
| // representation today. The `typed_*_clone_decision` consumers are the | ||
| // older per-type clone mechanism and would report the same function up | ||
| // to four times, so they stay out of the report and keep going to the | ||
| // native-reps artifact only. | ||
| if consumer == "spec_abi_entry_decision" && crate::opt_report::enabled() { | ||
| let (why, tier, issue) = reason.opt_report_reason(); | ||
| crate::opt_report::deny_named( | ||
| &source_function, | ||
| crate::opt_report::RegionKind::Function, | ||
| crate::opt_report::Denial { | ||
| position: crate::opt_report::Position::Param, | ||
| name: "(parameters + return)", | ||
| local_id: None, | ||
| analysis: crate::opt_report::Analysis::SpecAbi, | ||
| rule: reason.as_str(), | ||
| reason: why, | ||
| tier, | ||
| issue, | ||
| loop_depth: 0, | ||
| detail: None, | ||
| byte_offset: None, | ||
| }, | ||
| ); | ||
| } | ||
| records.push(crate::native_value::typed_clone_rejection_record( | ||
| source_function, | ||
| consumer, | ||
|
|
@@ -163,6 +191,15 @@ pub fn compile_module(hir: &HirModule, opts: CompileOptions) -> Result<Vec<u8>> | |
| // set-per-module discipline as the outline gate above. | ||
| helpers::set_jscvt_for_target(&triple); | ||
|
|
||
| // `--opt-report` (#6952): mark the closures that are iterating-builtin | ||
| // callbacks before any region is lowered, so their denials carry the | ||
| // per-element hotness column. No-op when the report is off. | ||
| crate::opt_report::scan_module(hir); | ||
| // Module-wide fallback attribution scope. Per-region scopes nest inside | ||
| // it and restore it on drop, so decisions taken outside any region (the | ||
| // specialized-ABI entry decision) still know their module. | ||
| let _opt_report_module_scope = crate::opt_report::enter_module(&hir.name); | ||
|
|
||
|
Comment on lines
+194
to
+202
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win Scan callbacks in every lowered class region.
🤖 Prompt for AI Agents |
||
| let mut llmod = LlModule::new_with_fp_flags(&triple, fp_flags); | ||
| // Null guard global: a zeroed i32 used as a safe dereference target | ||
| // when a NaN-unboxed pointer is null/invalid. Prevents segfaults from | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,98 @@ | ||
| //! `--opt-report` (#6952) rendering for specialized-ABI rejections. | ||
| //! | ||
| //! Split out of `typed_abi.rs` for the 2000-line file-size gate. This is an | ||
| //! inherent impl on `TypedCloneRejectionReason`, so it can live in any module | ||
| //! of the crate; the variant list itself stays next to the decision logic. | ||
|
|
||
| use super::typed_abi::TypedCloneRejectionReason; | ||
|
|
||
| impl TypedCloneRejectionReason { | ||
| /// `--opt-report` (#6952) rendering of a **specialized-ABI entry** | ||
| /// rejection: a human expansion of the variant plus who can act on it. | ||
| /// | ||
| /// Extends this existing vocabulary rather than paralleling it — the | ||
| /// variant IS the rule, so the report cites `as_str()` as the rule name | ||
| /// and this as the explanation. Only the `Spec*` and shared-precondition | ||
| /// variants that the spec-ABI decision can actually produce are given | ||
| /// bespoke text; the rest fall through to a generic line. | ||
| pub(crate) fn opt_report_reason( | ||
| self, | ||
| ) -> (&'static str, crate::opt_report::Tier, Option<&'static str>) { | ||
| use crate::opt_report::Tier; | ||
| match self { | ||
| Self::SpecTupleUnproven => ( | ||
| "no call site proved a representation tuple for this \ | ||
| function's parameters, so every call uses the boxed ABI and \ | ||
| re-unboxes on entry. Either the arguments are genuinely \ | ||
| polymorphic, or their types are outside what the specialized \ | ||
| ABI covers today (i32/u32, f64, bool, string, typed-array \ | ||
| pointer) — object, array and union parameters are not \ | ||
| covered yet, so a correct annotation does not always help.", | ||
| Tier::CompilerLimitation, | ||
| None, | ||
| ), | ||
| Self::SpecBudgetExceeded => ( | ||
| "the module-wide specialized-entry budget \ | ||
| (PERRY_SPECIALIZED_ABI_MAX) was exhausted before this \ | ||
| function's turn; it routes to the boxed entry.", | ||
| Tier::CompilerLimitation, | ||
| None, | ||
| ), | ||
| Self::SpecTypedCloneOverlap => ( | ||
| "the function already carries a typed_abi clone family, which \ | ||
| is mutually exclusive with a specialized entry in this phase.", | ||
| Tier::CompilerLimitation, | ||
| None, | ||
| ), | ||
| Self::SpecReadsDynamicThis => ( | ||
| "the callee reads dynamic `this`, so direct call sites must \ | ||
| juggle the implicit-this slot and cannot take the raw ABI.", | ||
| Tier::CompilerLimitation, | ||
| None, | ||
| ), | ||
| Self::AsyncOrGenerator => ( | ||
| "async and generator bodies route their locals through shared \ | ||
| cells (the async-to-generator transform), which the \ | ||
| specialized ABI must not touch.", | ||
| Tier::CompilerLimitation, | ||
| None, | ||
| ), | ||
| Self::I64Specialized => ( | ||
| "the function is already i64-specialized, which takes \ | ||
| precedence over a representation-tuple entry.", | ||
| Tier::CompilerLimitation, | ||
| None, | ||
| ), | ||
| Self::Captures | Self::CapturesThis | Self::CapturesNewTarget => ( | ||
| "the function captures from an enclosing scope; captures are \ | ||
| passed through the closure environment, not the raw ABI.", | ||
| Tier::Fixable, | ||
| None, | ||
| ), | ||
| Self::ParamDefault => ( | ||
| "a parameter has a default value, so the callee cannot assume \ | ||
| a fixed argument count on the raw entry.", | ||
| Tier::Fixable, | ||
| None, | ||
| ), | ||
| Self::RestParam => ( | ||
| "a rest parameter makes the argument count dynamic, so there \ | ||
| is no fixed representation tuple.", | ||
| Tier::Fixable, | ||
| None, | ||
| ), | ||
| Self::ArgumentsObject => ( | ||
| "the body reads `arguments`, which requires the boxed \ | ||
| argument vector.", | ||
| Tier::Fixable, | ||
| None, | ||
| ), | ||
| _ => ( | ||
| "the specialized-ABI precondition named by the rule did not \ | ||
| hold; the function keeps the boxed entry ABI.", | ||
| Tier::CompilerLimitation, | ||
| None, | ||
| ), | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Avoid report-name allocations while opt-report is disabled.
The report helpers no-op when disabled, but their arguments are evaluated first. This unconditionally clones/formats names during ordinary compilation, violating the requirement for no reporting cost when disabled.
crates/perry-codegen/src/codegen/closure.rs#L734-L741: only clone or formatopt_report_nameafter checkingopt_report::enabled().crates/perry-codegen/src/codegen/method.rs#L373-L377: defer the instance-methodformat!until reporting is enabled.crates/perry-codegen/src/codegen/method.rs#L1417-L1421: defer the static-methodformat!until reporting is enabled.📍 Affects 2 files
crates/perry-codegen/src/codegen/closure.rs#L734-L741(this comment)crates/perry-codegen/src/codegen/method.rs#L373-L377crates/perry-codegen/src/codegen/method.rs#L1417-L1421🤖 Prompt for AI Agents