perf(codegen): skip canonical-shape own-method scans (#8406) (#8505)

Co-authored-by: Ralph Küpper <ralph@skelpo.com>

Author: proggeramlug

changelog.d/8406-shape-header-reads.md

@@ -0,0 +1 @@
+Optimize polymorphic instance-method dispatch by using the receiver's compiler-published class and ShapeId pair to prove that canonical instances have no own-method override. Eligible call sites now read the object header once and bypass `js_object_get_own_field_or_undef`'s keys-array scan, while declared/computed fields, dynamic parent chains, mutated shapes, non-instance receivers, and wide dispatch towers retain the existing guarded fallback. The class id obtained by the shape probe is reused by the bounded dispatch tower. This cuts the `shapes` corpus row's retired instructions by 9.14% and CPU cycles by 9.93%, with every corpus program still byte-exact; focused integration coverage keeps both declared function fields and post-construction method overrides on the correct path (#8406).

crates/perry-codegen/src/collectors/proven_this_routing_tests.rs

@@ -800,35 +800,44 @@ fn tower_route_is_guarded_by_the_class_shape_id() {
             panic!("nothing conditionally branches to {clone_block} — the clone is reached unguarded:\n{ir}")
         });
 
-    // 1. the class ShapeId global is loaded once at function entry …
-    let global_load = ir
-        .lines()
-        .find(|l| l.contains("= load i32, ptr @perry_class_shape_id_"))
-        .unwrap_or_else(|| panic!("the class ShapeId is never read:\n{ir}"));
-    let global_reg = global_load.trim().split(' ').next().expect("ssa name");
+    // 1. a class ShapeId global is loaded at function entry …
     // 2. … and parked in an entry slot …
-    let store = ir
+    // 3. … which THIS guard block reloads …
+    // 4. … and compares against the receiver's live ShapeId.
+    //
+    // There may be another hoisted load of the same global for an earlier
+    // dynamic-dispatch shape probe (#8406), so follow each candidate's
+    // dataflow into this guard instead of assuming the first load owns it.
+    let (slot, expected) = ir
         .lines()
-        .find(|l| l.contains(&format!("store i32 {}, ptr ", global_reg)))
-        .unwrap_or_else(|| panic!("the hoisted ShapeId is never stored:\n{ir}"));
-    let slot = store.rsplit(' ').next().expect("slot name");
+        .filter(|line| line.contains("= load i32, ptr @perry_class_shape_id_"))
+        .find_map(|global_load| {
+            let global_reg = global_load.trim().split(' ').next()?;
+            let store = ir
+                .lines()
+                .find(|line| line.contains(&format!("store i32 {global_reg}, ptr ")))?;
+            let slot = store.rsplit(' ').next()?;
+            let expected = guard_body.iter().find_map(|line| {
+                let line = line.trim();
+                line.ends_with(&format!("load i32, ptr {slot}"))
+                    .then(|| line.split(' ').next().map(str::to_string))
+                    .flatten()
+            })?;
+            guard_body
+                .iter()
+                .any(|line| line.contains("icmp eq i32") && line.contains(&expected))
+                .then(|| (slot.to_string(), expected))
+        })
+        .unwrap_or_else(|| {
+            panic!(
+                "the routed call is not dominated by the hoisted ShapeId's reload and compare:\n{guard_body:#?}"
+            )
+        });
     assert!(
         !ir.lines()
-            .any(|line| line.contains("call void @js_shadow_slot_bind") && line.contains(slot)),
+            .any(|line| line.contains("call void @js_shadow_slot_bind") && line.contains(&slot)),
         "a ShapeId scalar must not be registered as a moving GC root:\n{ir}"
     );
-    // 3. … which the guard block reloads …
-    let expected = guard_body
-        .iter()
-        .find_map(|l| {
-            let l = l.trim();
-            l.ends_with(&format!("load i32, ptr {}", slot))
-                .then(|| l.split(' ').next().expect("ssa name").to_string())
-        })
-        .unwrap_or_else(|| {
-            panic!("the guard block never reads the hoisted ShapeId:\n{guard_body:#?}")
-        });
-    // 4. … and compares against the receiver's live ShapeId.
     assert!(
         guard_body
             .iter()

crates/perry-codegen/src/lower_call/property_get/dynamic_dispatch.rs

@@ -10,7 +10,7 @@ use perry_hir::Expr;
 use crate::expr::{lower_expr, nanbox_pointer_inline, unbox_to_i64, FnCtx};
 use crate::nanbox::double_literal;
 use crate::type_analysis::receiver_class_name;
-use crate::types::{DOUBLE, I32, I64};
+use crate::types::{DOUBLE, I1, I32, I64};
 
 // Reach the override-emit helpers (`pub(super)` of `lower_call`) by their
 // canonical crate-relative path.
@@ -24,6 +24,49 @@ use crate::lower_call::method_override::{
 /// call it replaces — so past this width the site keeps the single-arm guard.
 const MAX_SUBCLASS_DISPATCH_ARMS: usize = 8;
 
+/// Can an exact canonical shape prove that `property` is not an own field?
+///
+/// A post-construction assignment such as `this.run = f` mints a successor
+/// ShapeId, so an exact canonical-shape match excludes that override. A
+/// declared field is different: it is already part of the canonical shape and
+/// may intentionally shadow a prototype method (#620). Computed fields and
+/// incomplete/dynamic parent chains are similarly unknowable here and retain
+/// the runtime own-property probe.
+fn canonical_shape_excludes_own_property(
+    ctx: &FnCtx<'_>,
+    class_name: &str,
+    property: &str,
+) -> bool {
+    let mut current = Some(class_name.to_string());
+    let mut seen = std::collections::HashSet::new();
+    while let Some(name) = current {
+        if !seen.insert(name.clone()) {
+            return false;
+        }
+        let Some(class) = ctx.classes.get(&name) else {
+            return false;
+        };
+        if class
+            .fields
+            .iter()
+            .any(|field| field.key_expr.is_some() || field.name == property)
+        {
+            return false;
+        }
+        if class.extends_expr.is_some() || class.native_extends.is_some() {
+            return false;
+        }
+        current = class.extends_name.clone().or_else(|| {
+            class.extends.and_then(|parent_id| {
+                ctx.classes
+                    .iter()
+                    .find_map(|(name, candidate)| (candidate.id == parent_id).then(|| name.clone()))
+            })
+        });
+    }
+    true
+}
+
 /// A declared class may select the direct-method guard, but never prove the
 /// direct call. The guard validates the live class id, keys token, own
 /// override, and resolved method pointer; every miss uses dynamic dispatch.
@@ -338,6 +381,11 @@ pub(crate) fn try_lower_instance_method_call(
         // instance with a longer chain. Inherited dispatch gets `None` and
         // keeps today's lowering.
         let mut impl_owner: Vec<Option<String>> = Vec::new();
+        // Concrete receiver class for each implementor entry. Unlike
+        // `impl_owner`, this is present for inherited implementations too and
+        // lets the override probe compare the receiver against that class's
+        // canonical ShapeId.
+        let mut impl_class: Vec<String> = Vec::new();
         let mut seen_pairs: std::collections::HashSet<(u32, String)> =
             std::collections::HashSet::new();
         // Walk `class_ids` in a FIXED order, not `HashMap` order (#7622). Each
@@ -371,6 +419,7 @@ pub(crate) fn try_lower_instance_method_call(
                             crate::codegen::arguments::method_has_user_rest(ctx, &c, property);
                         let decl = ctx.method_param_counts.get(&key).copied().unwrap_or(0);
                         impl_owner.push((c == *start_cls).then(|| start_cls.clone()));
+                        impl_class.push(start_cls.clone());
                         implementors.push((start_cid, fname));
                         impl_meta.push((has_rest, has_synthetic_arguments, has_user_rest, decl));
                     }
@@ -449,6 +498,69 @@ pub(crate) fn try_lower_instance_method_call(
             let probe_entry = ctx.strings.entry(key_idx_probe);
             let probe_bytes_global = format!("@{}", probe_entry.bytes_global);
             let probe_name_len_str = probe_entry.byte_len.to_string();
+            let probe_override_idx = ctx.new_block("idisp.override");
+            let probe_dispatch_idx = ctx.new_block("idisp.dispatch");
+            let probe_outer_merge_idx = ctx.new_block("idisp.outer_merge");
+            let probe_override_label = ctx.block_label(probe_override_idx);
+            let probe_dispatch_label = ctx.block_label(probe_dispatch_idx);
+            let probe_outer_merge_label = ctx.block_label(probe_outer_merge_idx);
+
+            // #8406: an exact compiler-published (class id, ShapeId) pair can
+            // prove that no post-construction own-method override was added.
+            // Probe the receiver once and bypass the keys-array scan for those
+            // canonical shapes. Classes whose canonical layout itself may
+            // contain `property` stay on the old probe, as do wide towers to
+            // keep code-size growth bounded.
+            let shape_probe_arms: Vec<(u32, String)> = if implementors.len()
+                <= MAX_SUBCLASS_DISPATCH_ARMS
+            {
+                implementors
+                    .iter()
+                    .zip(impl_class.iter())
+                    .filter_map(|((class_id, _), class_name)| {
+                        if !canonical_shape_excludes_own_property(ctx, class_name, property) {
+                            return None;
+                        }
+                        let keys_global = ctx.class_keys_globals.get(class_name)?;
+                        let expected_shape =
+                            crate::typed_shape::load_class_shape_id(ctx, class_name, keys_global);
+                        Some((*class_id, expected_shape))
+                    })
+                    .collect()
+            } else {
+                Vec::new()
+            };
+            let mut shape_probe_cid: Option<String> = None;
+            if !shape_probe_arms.is_empty() {
+                let shape_slot = ctx.func.alloca_entry(I32);
+                let cid = ctx.block().call(
+                    I32,
+                    "js_method_direct_shape_class",
+                    &[(DOUBLE, &recv_box), (crate::types::PTR, &shape_slot)],
+                );
+                let shape_id = ctx.block().load(I32, &shape_slot);
+                shape_probe_cid = Some(cid.clone());
+                let own_idx = ctx.new_block("idisp.own_probe");
+                let test_idxs: Vec<usize> = (1..shape_probe_arms.len())
+                    .map(|i| ctx.new_block(&format!("idisp.shape_test{i}")))
+                    .collect();
+                for (i, (class_id, expected_shape)) in shape_probe_arms.iter().enumerate() {
+                    if i > 0 {
+                        ctx.current_block = test_idxs[i - 1];
+                    }
+                    let miss_label = test_idxs
+                        .get(i)
+                        .map(|&idx| ctx.block_label(idx))
+                        .unwrap_or_else(|| ctx.block_label(own_idx));
+                    let blk = ctx.block();
+                    let cid_ok = blk.icmp_eq(I32, &cid, &class_id.to_string());
+                    let shape_ok = blk.icmp_eq(I32, &shape_id, expected_shape);
+                    let exact = blk.and(I1, &cid_ok, &shape_ok);
+                    blk.cond_br(&exact, &probe_dispatch_label, &miss_label);
+                }
+                ctx.current_block = own_idx;
+            }
+
             let own_method_probe = ctx.block().call(
                 DOUBLE,
                 "js_object_get_own_field_or_undef",
@@ -461,12 +573,6 @@ pub(crate) fn try_lower_instance_method_call(
             let own_bits_probe = ctx.block().bitcast_double_to_i64(&own_method_probe);
             let undef_bits_str = format!("{}", crate::nanbox::TAG_UNDEFINED as i64);
             let is_undef_probe = ctx.block().icmp_eq(I64, &own_bits_probe, &undef_bits_str);
-            let probe_override_idx = ctx.new_block("idisp.override");
-            let probe_dispatch_idx = ctx.new_block("idisp.dispatch");
-            let probe_outer_merge_idx = ctx.new_block("idisp.outer_merge");
-            let probe_override_label = ctx.block_label(probe_override_idx);
-            let probe_dispatch_label = ctx.block_label(probe_dispatch_idx);
-            let probe_outer_merge_label = ctx.block_label(probe_outer_merge_idx);
             ctx.block().cond_br(
                 &is_undef_probe,
                 &probe_dispatch_label,
@@ -570,9 +676,17 @@ pub(crate) fn try_lower_instance_method_call(
             // closure-call fallback would also handle this but
             // returning a sentinel is cheaper).
             ctx.current_block = tower_idx;
-            let blk = ctx.block();
-            let recv_handle = unbox_to_i64(blk, &recv_box);
-            let cid = blk.call(I32, "js_object_get_class_id", &[(I64, &recv_handle)]);
+            let recv_handle = unbox_to_i64(ctx.block(), &recv_box);
+            let cid = if let Some(probed_cid) = shape_probe_cid {
+                // Reuse the class id that the shape probe already validated.
+                // Zero is intentional: it sends descriptor/prototype
+                // invalidation and every non-instance receiver to the runtime
+                // fallback instead of re-entering this hard-coded tower.
+                probed_cid
+            } else {
+                ctx.block()
+                    .call(I32, "js_object_get_class_id", &[(I64, &recv_handle)])
+            };
 
             for (i, (case_cid, _)) in implementors.iter().enumerate() {
                 let case_label = ctx.block_label(case_idxs[i]);

crates/perry/tests/issue_8406_dynamic_dispatch_shape_probe.rs

@@ -0,0 +1,74 @@
+//! Regression coverage for #8406's dynamic-dispatch shape shortcut.
+//!
+//! An exact canonical class shape may bypass the runtime own-property scan,
+//! but only when that canonical layout cannot itself contain the method name.
+//! A declared function field and a later own-property assignment must both
+//! continue to override an inherited prototype method.
+
+use std::path::PathBuf;
+use std::process::Command;
+
+fn perry_bin() -> PathBuf {
+    PathBuf::from(env!("CARGO_BIN_EXE_perry"))
+}
+
+#[test]
+fn canonical_and_mutated_own_method_overrides_survive_shape_shortcut() {
+    let dir = tempfile::tempdir().expect("tempdir");
+    let entry = dir.path().join("main.ts");
+    let output = dir.path().join("main_bin");
+    std::fs::write(
+        &entry,
+        r#"
+interface Runner { run(): string }
+
+class Base {
+  run(): string { return "base"; }
+}
+
+class FieldOverride extends Base {
+  run = (): string => "field";
+}
+
+class MutatedOverride extends Base {}
+
+function invoke(value: Runner): string {
+  return value.run();
+}
+
+const mutated: any = new MutatedOverride();
+mutated.run = (): string => "mutated";
+
+console.log(invoke(new Base()), invoke(new FieldOverride()), invoke(mutated));
+"#,
+    )
+    .expect("write source");
+
+    let compile = Command::new(perry_bin())
+        .current_dir(dir.path())
+        .arg("compile")
+        .arg(&entry)
+        .arg("-o")
+        .arg(&output)
+        .arg("--no-cache")
+        .output()
+        .expect("run perry compile");
+    assert!(
+        compile.status.success(),
+        "perry compile failed\nstdout:\n{}\nstderr:\n{}",
+        String::from_utf8_lossy(&compile.stdout),
+        String::from_utf8_lossy(&compile.stderr)
+    );
+
+    let run = Command::new(&output)
+        .current_dir(dir.path())
+        .output()
+        .expect("run compiled binary");
+    assert!(
+        run.status.success(),
+        "compiled binary failed\nstdout:\n{}\nstderr:\n{}",
+        String::from_utf8_lossy(&run.stdout),
+        String::from_utf8_lossy(&run.stderr)
+    );
+    assert_eq!(String::from_utf8_lossy(&run.stdout), "base field mutated\n");
+}