From 5dbe8c18a5de6fa1dba69f5a509c276f694b8d6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Fri, 21 Aug 2026 05:58:49 +0200 Subject: [PATCH] perf(codegen): skip canonical-shape own-method scans (#8406) --- changelog.d/8406-shape-header-reads.md | 1 + .../collectors/proven_this_routing_tests.rs | 55 ++++--- .../property_get/dynamic_dispatch.rs | 134 ++++++++++++++++-- ...issue_8406_dynamic_dispatch_shape_probe.rs | 74 ++++++++++ 4 files changed, 231 insertions(+), 33 deletions(-) create mode 100644 changelog.d/8406-shape-header-reads.md create mode 100644 crates/perry/tests/issue_8406_dynamic_dispatch_shape_probe.rs diff --git a/changelog.d/8406-shape-header-reads.md b/changelog.d/8406-shape-header-reads.md new file mode 100644 index 0000000000..31bb4042d4 --- /dev/null +++ b/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). diff --git a/crates/perry-codegen/src/collectors/proven_this_routing_tests.rs b/crates/perry-codegen/src/collectors/proven_this_routing_tests.rs index 9f3d7cfe8d..e32ffc93c3 100644 --- a/crates/perry-codegen/src/collectors/proven_this_routing_tests.rs +++ b/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() diff --git a/crates/perry-codegen/src/lower_call/property_get/dynamic_dispatch.rs b/crates/perry-codegen/src/lower_call/property_get/dynamic_dispatch.rs index ca00008efe..61b40ae1bd 100644 --- a/crates/perry-codegen/src/lower_call/property_get/dynamic_dispatch.rs +++ b/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> = 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 = 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 = 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 = (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]); diff --git a/crates/perry/tests/issue_8406_dynamic_dispatch_shape_probe.rs b/crates/perry/tests/issue_8406_dynamic_dispatch_shape_probe.rs new file mode 100644 index 0000000000..fa2860ac19 --- /dev/null +++ b/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"); +}