From ecc8e0e8a03b909e359ed04ec54246ac94d71c94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Mon, 24 Aug 2026 09:07:41 +0200 Subject: [PATCH] perf(codegen): specialize imported methods capturing this --- benchmarks/compiler_output/workloads.toml | 40 +++ .../8693-imported-this-specialization.md | 9 + crates/perry-codegen/src/codegen/helpers.rs | 19 ++ crates/perry-codegen/src/codegen/method.rs | 10 +- .../src/codegen/method_registry.rs | 4 + crates/perry-codegen/src/codegen/mod.rs | 65 +++- crates/perry-codegen/src/codegen/opts.rs | 21 +- crates/perry-codegen/src/collectors/mod.rs | 1 + .../src/collectors/proven_this.rs | 72 ++++- .../src/collectors/repsel_benefit.rs | 28 +- .../src/collectors/repsel_benefit/tests.rs | 65 +++- crates/perry-codegen/src/lib.rs | 13 + .../src/lower_call/field_init.rs | 60 +++- .../src/lower_call/field_init/tests.rs | 88 ++++- .../src/lower_call/method_override.rs | 46 ++- .../src/lower_call/typed_shape_bake_tests.rs | 2 + .../src/object/class_registry.rs | 7 +- .../class_registry/prototype_methods.rs | 16 +- .../perry-runtime/src/object/delete_rest.rs | 36 ++- .../src/object/field_set_by_name.rs | 128 ++++---- .../src/object/native_call_method.rs | 6 +- .../native_call_method/handle_methods.rs | 18 +- .../src/object/object_ops/define_property.rs | 7 + crates/perry-runtime/src/proxy.rs | 15 +- .../src/commands/compile/object_cache.rs | 8 + .../object_cache/object_cache_tests.rs | 16 + .../src/commands/compile/run_pipeline.rs | 105 +++++- ...issue_8693_imported_this_specialization.rs | 305 ++++++++++++++++++ .../issue_8693_imported_this/barrel.js | 15 + .../fixtures/issue_8693_imported_this/main.js | 14 + .../issue_8693_imported_this/package.json | 3 + .../issue_8693_imported_this/registry.js | 24 ++ .../issue_8693_imported_this/semantics.js | 143 ++++++++ 33 files changed, 1264 insertions(+), 145 deletions(-) create mode 100644 changelog.d/8693-imported-this-specialization.md create mode 100644 crates/perry/tests/issue_8693_imported_this_specialization.rs create mode 100644 test-files/fixtures/issue_8693_imported_this/barrel.js create mode 100644 test-files/fixtures/issue_8693_imported_this/main.js create mode 100644 test-files/fixtures/issue_8693_imported_this/package.json create mode 100644 test-files/fixtures/issue_8693_imported_this/registry.js create mode 100644 test-files/fixtures/issue_8693_imported_this/semantics.js diff --git a/benchmarks/compiler_output/workloads.toml b/benchmarks/compiler_output/workloads.toml index 349b82acf2..5f17f84c6f 100644 --- a/benchmarks/compiler_output/workloads.toml +++ b/benchmarks/compiler_output/workloads.toml @@ -2076,6 +2076,7 @@ allowed_hot_loop_runtime_calls = [ "js_for_in_keys_stable_value", "js_gc_loop_safepoint", "js_in_operator", + "js_value_typeof", ] [workloads.for_in_stable_keys.vectorization] @@ -2092,6 +2093,8 @@ allowed_missed_reason_kinds = [ "unsupported_reduction", ] +[workloads.for_in_stable_keys.runtime_budgets] + [[workloads.for_in_stable_keys.stdout_checks]] name = "for_in_stable_keys_checksum" equals = "for_in_stable_keys:200000\n" @@ -2114,3 +2117,40 @@ regex_none = [ "call i64 @js_object_get_own_property_names", ] detail = "the stable arm does not directly allocate or rebuild generic key lists" + +[workloads.issue_8693_imported_this] +source = "test-files/fixtures/issue_8693_imported_this/main.js" +kind = "imported_class_method_specialization" +allow_hot_loop_conversions = true +allow_dynamic_property_runtime = true + +[workloads.issue_8693_imported_this.vectorization] +min_vectorized_loops = 0 +scalar_baseline = "allowed: this fixture gates cross-module method dispatch, not loop vectorization" +allowed_missed_reason_kinds = [ + "call_instruction", + "control_flow", + "generic_not_vectorized", + "not_beneficial", + "uncountable_loop", + "unknown_trip_count", + "unsupported_instruction", + "unsupported_reduction", +] + +[workloads.issue_8693_imported_this.runtime_budgets] + +[workloads.issue_8693_imported_this.native_rep_checks] +allow_materialization_reasons = ["runtime_api"] + +[[workloads.issue_8693_imported_this.native_rep_checks.require_records]] +name = "imported_registry_proven_this_selection" +consumer = "proven_this_method_direct_call" +notes_contains = "receiver_provenance=imported_class_metadata" +min = 2 + +[[workloads.issue_8693_imported_this.native_rep_checks.require_records]] +name = "imported_registry_generic_fallback_retained" +consumer = "proven_this_method_direct_call" +notes_contains = "generic_dispatch_fallback=js_native_call_method_by_id" +min = 2 diff --git a/changelog.d/8693-imported-this-specialization.md b/changelog.d/8693-imported-this-specialization.md new file mode 100644 index 0000000000..b9811da159 --- /dev/null +++ b/changelog.d/8693-imported-this-specialization.md @@ -0,0 +1,9 @@ +--- +category: Performance +title: Specialize imported methods that capture this +--- + +ESM import and re-export metadata now carries producer-proven method +eligibility, allowing guarded direct calls into stable class methods that use +`this`. Generic dispatch remains available for shadowed or mutated receivers, +including prototype replacement, deletion, and recreation. diff --git a/crates/perry-codegen/src/codegen/helpers.rs b/crates/perry-codegen/src/codegen/helpers.rs index b13b217820..9e6b038da3 100644 --- a/crates/perry-codegen/src/codegen/helpers.rs +++ b/crates/perry-codegen/src/codegen/helpers.rs @@ -339,6 +339,25 @@ pub(super) fn inline_hot_small_size_cap() -> usize { }) } +/// Give the representation-specialized method body the ordinary small-body +/// inline bias. This lets producer-local chains such as `Registry.add -> +/// Group.pushEntity` optimize through the second method boundary while the +/// externally linked clone remains callable by importers. +pub(super) fn apply_pshape_inline_policy( + lf: &mut crate::function::LlFunction, + method: &perry_hir::Function, + is_pshape_clone: bool, +) { + if !is_pshape_clone || method.is_async || method.is_generator || method.was_plain_async { + return; + } + if method.body.len() <= 8 { + lf.force_inline = true; + } else if inline_hot_small_enabled() && method.body.len() <= inline_hot_small_size_cap() { + lf.inline_hint = true; + } +} + /// Maximum total (module-wide) direct call sites a function may have and still /// be hinted. This is the anti-bloat backstop: the raised `-inlinehint-threshold` /// lifts LLVM's ceiling for a hinted callee at *every* one of its call sites, so diff --git a/crates/perry-codegen/src/codegen/method.rs b/crates/perry-codegen/src/codegen/method.rs index cab728750c..b4779ce74a 100644 --- a/crates/perry-codegen/src/codegen/method.rs +++ b/crates/perry-codegen/src/codegen/method.rs @@ -306,10 +306,18 @@ pub(super) fn compile_method( let ic_base = llmod.ic_counter; let buffer_alias_base = llmod.buffer_alias_counter; let lf = llmod.define_function(&llvm_name, DOUBLE, params); - if is_pshape_clone || is_index_clone || typed_public_trampoline.is_some() || force_generic_body + // Plain `$pshape` clones are producer-published capabilities and need + // external linkage for guarded calls from importing modules. The stricter + // array-cache clone remains module-local: only containment-proven locals + // in this module may select it. + if ptr_array_cache_clone + || is_index_clone + || typed_public_trampoline.is_some() + || force_generic_body { lf.linkage = "internal".to_string(); } + super::helpers::apply_pshape_inline_policy(lf, method, is_pshape_clone); if is_index_clone { lf.pre_statepoint_inline = true; } diff --git a/crates/perry-codegen/src/codegen/method_registry.rs b/crates/perry-codegen/src/codegen/method_registry.rs index 202a20a83c..a1c1b2d43b 100644 --- a/crates/perry-codegen/src/codegen/method_registry.rs +++ b/crates/perry-codegen/src/codegen/method_registry.rs @@ -216,6 +216,10 @@ pub(crate) fn build_method_names( let param_types: Vec = std::iter::repeat_n(DOUBLE, arity).collect(); llmod.declare_function(&llvm_fn, DOUBLE, ¶m_types); + if ic.proven_this_method_names.contains(method_name) { + let clone = crate::collectors::pshape_method_name(&llvm_fn); + llmod.declare_function(&clone, DOUBLE, ¶m_types); + } } // Cross-module getters. The dispatch site at diff --git a/crates/perry-codegen/src/codegen/mod.rs b/crates/perry-codegen/src/codegen/mod.rs index acc8e7f11e..ca3d4b9b7d 100644 --- a/crates/perry-codegen/src/codegen/mod.rs +++ b/crates/perry-codegen/src/codegen/mod.rs @@ -1808,17 +1808,6 @@ pub fn compile_module(hir: &HirModule, opts: CompileOptions) -> Result> receiver_class_table, &module_dispatch_facts, ) { - // #7142: the tower routing site emits its own inline shape - // re-check, so it only takes the clone where the clone deletes - // strictly more guarded field sites than that check costs. The - // other two sites are guard-dominated and route unconditionally. - if crate::collectors::pshape_tower_route_profitable( - class, - method, - receiver_class_table, - ) { - pshape_tower_routable.insert((class.name.clone(), method.name.clone())); - } pshape_methods.insert((class.name.clone(), method.name.clone()), fact); } match typed_abi::typed_f64_method_rejection_reason(method) { @@ -1933,6 +1922,60 @@ pub fn compile_module(hir: &HirModule, opts: CompileOptions) -> Result> } } } + // #7142: the tower routing site emits its own inline shape re-check, so it + // only takes a clone where that clone deletes strictly more guarded work + // than the check costs. Price these routes after all local clone facts are + // known so nested `this.other()` calls can count an inherited clone too. + let local_pshape_methods: std::collections::HashSet<(String, String)> = + pshape_methods.keys().cloned().collect(); + for class in &hir.classes { + for method in &class.methods { + let key = (class.name.clone(), method.name.clone()); + if local_pshape_methods.contains(&key) + && crate::collectors::pshape_tower_route_profitable( + class, + method, + receiver_class_table, + &local_pshape_methods, + ) + { + pshape_tower_routable.insert(key); + } + } + } + // Imported classes publish only clone names the defining module proved and + // emitted. Installing those capabilities in the same registries lets both + // the ordinary exact-class/shape guarded arm and profitable adapter-field + // dispatch towers retain the receiver proof across ESM and npm boundaries. + // The tower subset is producer-authored because only the defining module + // can see enough of the body to price its additional keys-token check. + for imported in &opts.imported_classes { + let effective_name = imported + .local_alias + .as_deref() + .unwrap_or(&imported.name) + .to_string(); + if hir.classes.iter().any(|class| class.name == effective_name) { + continue; + } + for method in &imported.proven_this_method_names { + if !imported.method_names.contains(method) { + continue; + } + pshape_methods.insert( + (effective_name.clone(), method.clone()), + crate::collectors::PtrShapeLocal { + class_name: effective_name.clone(), + numeric_fields: std::collections::HashSet::new(), + report_name: crate::opt_report::enabled() + .then(|| format!("imported:{}", imported.source_prefix)), + }, + ); + if imported.proven_this_tower_method_names.contains(method) { + pshape_tower_routable.insert((effective_name.clone(), method.clone())); + } + } + } let mut compiler_private_async_i32_control_locals = std::collections::HashSet::new(); let mut compiler_private_async_i1_control_locals = std::collections::HashSet::new(); crate::boxed_vars::collect_compiler_private_async_control_locals_in_stmts( diff --git a/crates/perry-codegen/src/codegen/opts.rs b/crates/perry-codegen/src/codegen/opts.rs index 356fa91125..d09903049a 100644 --- a/crates/perry-codegen/src/codegen/opts.rs +++ b/crates/perry-codegen/src/codegen/opts.rs @@ -510,6 +510,16 @@ pub struct ImportedClass { pub has_instance_fields: bool, /// Method names defined on this class. pub method_names: Vec, + /// Own methods for which the defining module emitted an externally + /// callable, guarded proven-`this` clone. Consumers may reference only + /// names in this producer-authored capability set; absence is fail-closed + /// and keeps the public method body on the direct arm. + pub proven_this_method_names: Vec, + /// Subset of `proven_this_method_names` for which the producer also proved + /// that the extra exact-keys check paid by a class-id dispatch-tower arm is + /// profitable. This keeps adapter-field calls fail-closed without asking + /// an importer that cannot see the method body to repeat the decision. + pub proven_this_tower_method_names: Vec, /// Declared return types parallel to `method_names`. Imported class stubs /// retain these so a call such as `factory.make().run()` can recover the /// returned receiver class without value-importing that class directly. @@ -911,12 +921,11 @@ pub(crate) struct CrossModuleCtx { /// exported. pub nonnegative_index_methods: std::collections::HashMap<(String, String), Vec>, /// Representation-selection Phase 5a: `(class, method)` pairs that have a - /// generated `internal` proven-`this` clone - /// (`collectors/proven_this.rs`). Keys are OWN declarations of - /// module-local classes only, which is precisely the condition the two - /// routing sites rely on: a hit means the receiver's proven exact class is - /// the class the clone was compiled for, so `this` cannot be a subclass - /// instance with a different chain. + /// generated proven-`this` clone (`collectors/proven_this.rs`). Local keys + /// come from body analysis; imported keys come from an explicit capability + /// published by the defining module. Both represent OWN declarations, so a + /// hit means the receiver's proven exact class is the class the clone was + /// compiled for and `this` cannot have a different subclass chain. pub pshape_methods: std::collections::HashMap<(String, String), crate::collectors::PtrShapeLocal>, /// #7142: the subset of [`Self::pshape_methods`] whose clone the class-id diff --git a/crates/perry-codegen/src/collectors/mod.rs b/crates/perry-codegen/src/collectors/mod.rs index 53a0f632e8..780ca10d48 100644 --- a/crates/perry-codegen/src/collectors/mod.rs +++ b/crates/perry-codegen/src/collectors/mod.rs @@ -87,6 +87,7 @@ pub(crate) use number_by_construction::collect_number_by_construction_locals; pub(crate) use param_ranges::{collect_param_int_ranges, ParamIntRanges}; pub(crate) use pointer_locals::collect_pointer_typed_locals; pub(crate) use proven_this::{ + exportable_method_capabilities as exportable_proven_this_method_capabilities, method_proven_this, prune_unregistered_clones, pshape_method_name, ptr_array_cache_fields, ptr_array_cache_method_name, ptr_array_cached_method, tower_route_profitable as pshape_tower_route_profitable, diff --git a/crates/perry-codegen/src/collectors/proven_this.rs b/crates/perry-codegen/src/collectors/proven_this.rs index 9d732d4166..c5df31e195 100644 --- a/crates/perry-codegen/src/collectors/proven_this.rs +++ b/crates/perry-codegen/src/collectors/proven_this.rs @@ -15,7 +15,7 @@ //! 2. `lower_call/property_get/dynamic_dispatch.rs` — the Phase 3b guard-free //! `Ptr` receiver arm, whose receiver is a shape-proven local. //! -//! Phase 5a emits an `internal` `{public}$pshape` clone of the method whose +//! Phase 5a emits a `{public}$pshape` clone of the method whose //! `this` carries the [`PtrShapeLocal`] proof, and routes those two sites to //! it. Net new proof work: zero. Net new GC work: zero — the clone keeps the //! identical `(double this, double args…)` ABI and the identical shadow-bound @@ -148,9 +148,10 @@ pub fn ptr_shape_this_enabled() -> bool { /// composed-symbol collision prune here.) /// /// This symbol is NEVER registered into a runtime vtable -/// (`js_register_class_method` keeps the public name) and is reachable only -/// from the proven call sites. [`tests::pshape_symbol_reachability`] -/// ratchets that. +/// (`js_register_class_method` keeps the public name). It is externally +/// visible solely so an importing native module can use it after receiving a +/// producer-authored capability; every call remains one of the proven direct +/// sites. [`tests::pshape_symbol_reachability`] ratchets that. pub(crate) fn pshape_method_name(public_name: &str) -> String { format!("{public_name}$pshape") } @@ -602,6 +603,60 @@ pub(crate) fn method_proven_this( }) } +/// Producer-side capabilities that may be published to native-module +/// consumers. +/// +/// This deliberately uses only classes defined in `hir`. A class whose parent +/// is imported therefore stays out of the published set even when the full +/// compile options later make its chain resolvable. Under-publishing only +/// leaves a guarded call on the public body; over-publishing could make a +/// consumer reference a clone the producer did not emit. +pub(crate) fn exportable_method_capabilities( + hir: &perry_hir::Module, +) -> (HashMap>, HashMap>) { + let classes: HashMap = hir + .classes + .iter() + .map(|class| (class.name.clone(), class)) + .collect(); + let module_dispatch = super::collect_module_dispatch_facts(hir); + let mut exported = HashMap::new(); + let mut tower_routable = HashMap::new(); + let mut eligible = HashSet::new(); + + for class in &hir.classes { + let mut methods = Vec::new(); + for method in &class.methods { + if method_proven_this(class, method, &classes, &module_dispatch).is_none() { + continue; + } + methods.push(method.name.clone()); + eligible.insert((class.name.clone(), method.name.clone())); + } + if !methods.is_empty() { + exported.insert(class.name.clone(), methods); + } + } + + // Price tower routes only after every clone capability is known: a clone + // can delete the public guard at a nested `this.other()` boundary too, + // including when `other` is inherited from another class in the chain. + for class in &hir.classes { + let tower_methods: Vec = class + .methods + .iter() + .filter(|method| eligible.contains(&(class.name.clone(), method.name.clone()))) + .filter(|method| tower_route_profitable(class, method, &classes, &eligible)) + .map(|method| method.name.clone()) + .collect(); + if !tower_methods.is_empty() { + tower_routable.insert(class.name.clone(), tower_methods); + } + } + + (exported, tower_routable) +} + /// #7142 profitability: should a class-id dispatch-tower case route to /// `method`'s `{public}$pshape` clone? /// @@ -617,13 +672,18 @@ pub(crate) fn tower_route_profitable( class: &Class, method: &Function, classes: &HashMap, + eligible: &HashSet<(String, String)>, ) -> bool { let chain = chain_classes(classes, &class.name); if chain.is_empty() { return false; } let fields = chain_field_names(&chain); - super::repsel_benefit::tower_route_profitable(method, &fields) + let pshape_methods = chain_method_map(&chain) + .into_iter() + .filter_map(|(name, (owner, _))| eligible.contains(&(owner, name.clone())).then_some(name)) + .collect(); + super::repsel_benefit::tower_route_profitable(method, &fields, &pshape_methods) } /// Does the method body READ `this.` anywhere? @@ -777,7 +837,7 @@ mod tests { assert!( offenders.is_empty(), "proven-`this` clone symbol fragments found outside the allowlist \ - (the clone is `internal` and must NEVER be registered into a \ + (the clone must NEVER be registered into a \ runtime vtable or reached indirectly): {offenders:?}" ); } diff --git a/crates/perry-codegen/src/collectors/repsel_benefit.rs b/crates/perry-codegen/src/collectors/repsel_benefit.rs index 2415bfe30b..1f7f17e0e5 100644 --- a/crates/perry-codegen/src/collectors/repsel_benefit.rs +++ b/crates/perry-codegen/src/collectors/repsel_benefit.rs @@ -128,7 +128,7 @@ use perry_hir::{BinaryOp, Expr, Function, Stmt, UnaryOp}; /// This is a *count*, not a cost model: it has no target-specific term, so it /// says the same thing on AArch64 and x86-64 (contrast the fusion term in /// #7146, which does not). It deliberately under-counts — see -/// [`proven_receiver_clone_field_sites`]. +/// [`proven_receiver_clone_benefit_sites`]. const TOWER_ROUTE_MIN_FIELD_SITES: u32 = 2; /// What the *consumer* of a read wants the value to be. @@ -493,9 +493,14 @@ pub(crate) fn collect_unprofitable_canonical_i32_locals( /// refusing a route rather than taking one: /// /// * a site inside a loop is counted once, though it is paid per iteration; -/// * `this.other()` calls that the clone also lowers guard-free (the -/// `ThisFlowAnalysis` walk vets them transitively) are not followed. -fn proven_receiver_clone_field_sites(method: &Function, chain_fields: &HashSet) -> u32 { +/// A direct `this.other()` call counts when `other` also has a proven-receiver +/// clone: the caller clone routes that nested boundary directly, deleting the +/// public body's method guard in addition to its own field sites. +fn proven_receiver_clone_benefit_sites( + method: &Function, + chain_fields: &HashSet, + pshape_methods: &HashSet, +) -> u32 { let mut sites = 0u32; super::scalar_method_dispatch::for_each_expr_in_stmts(&method.body, &mut |e| { let named = match e { @@ -512,6 +517,12 @@ fn proven_receiver_clone_field_sites(method: &Function, chain_fields: &HashSet matches!( + callee.as_ref(), + Expr::PropertyGet { object, property, .. } + if matches!(object.as_ref(), Expr::This) + && pshape_methods.contains(property.as_str()) + ), _ => false, }; if named { @@ -528,8 +539,13 @@ fn proven_receiver_clone_field_sites(method: &Function, chain_fields: &HashSet) -> bool { - proven_receiver_clone_field_sites(method, chain_fields) >= TOWER_ROUTE_MIN_FIELD_SITES +pub(crate) fn tower_route_profitable( + method: &Function, + chain_fields: &HashSet, + pshape_methods: &HashSet, +) -> bool { + proven_receiver_clone_benefit_sites(method, chain_fields, pshape_methods) + >= TOWER_ROUTE_MIN_FIELD_SITES } #[cfg(test)] diff --git a/crates/perry-codegen/src/collectors/repsel_benefit/tests.rs b/crates/perry-codegen/src/collectors/repsel_benefit/tests.rs index 80b44780dc..11fbcfe711 100644 --- a/crates/perry-codegen/src/collectors/repsel_benefit/tests.rs +++ b/crates/perry-codegen/src/collectors/repsel_benefit/tests.rs @@ -622,16 +622,22 @@ fn tower_route_break_even_is_one_field_site() { let fields = declared(&["a", "c"]); let one = method(vec![Stmt::Return(Some(this_get("a")))]); - assert_eq!(proven_receiver_clone_field_sites(&one, &fields), 1); - assert!(!tower_route_profitable(&one, &fields)); + assert_eq!( + proven_receiver_clone_benefit_sites(&one, &fields, &HashSet::new()), + 1 + ); + assert!(!tower_route_profitable(&one, &fields, &HashSet::new())); let two = method(vec![Stmt::Return(Some(bin( BinaryOp::Add, this_get("a"), this_get("c"), )))]); - assert_eq!(proven_receiver_clone_field_sites(&two, &fields), 2); - assert!(tower_route_profitable(&two, &fields)); + assert_eq!( + proven_receiver_clone_benefit_sites(&two, &fields, &HashSet::new()), + 2 + ); + assert!(tower_route_profitable(&two, &fields, &HashSet::new())); } /// Only DECLARED chain fields get a fixed slot; anything else keeps its by-name @@ -645,8 +651,11 @@ fn tower_route_counts_only_declared_fields() { Stmt::Expr(this_get("expando")), Stmt::Expr(this_get("alsoNotDeclared")), ]); - assert_eq!(proven_receiver_clone_field_sites(&m, &fields), 1); - assert!(!tower_route_profitable(&m, &fields)); + assert_eq!( + proven_receiver_clone_benefit_sites(&m, &fields, &HashSet::new()), + 1 + ); + assert!(!tower_route_profitable(&m, &fields, &HashSet::new())); } /// A field access on something that is not `this` is somebody else's receiver @@ -662,7 +671,10 @@ fn tower_route_ignores_non_this_receivers() { byte_offset: 0, }), ]); - assert_eq!(proven_receiver_clone_field_sites(&m, &fields), 1); + assert_eq!( + proven_receiver_clone_benefit_sites(&m, &fields, &HashSet::new()), + 1 + ); } /// Writes count as well as reads — a `this.f = …` site pays the same inline @@ -684,6 +696,41 @@ fn tower_route_counts_writes() { strict: true, }), ]); - assert_eq!(proven_receiver_clone_field_sites(&m, &fields), 2); - assert!(tower_route_profitable(&m, &fields)); + assert_eq!( + proven_receiver_clone_benefit_sites(&m, &fields, &HashSet::new()), + 2 + ); + assert!(tower_route_profitable(&m, &fields, &HashSet::new())); +} + +/// A proven-receiver clone also removes the public method guard at a nested +/// `this.helper()` boundary. It counts only when the resolved helper has its +/// own clone capability; otherwise that boundary remains generic. +#[test] +fn tower_route_counts_nested_proven_receiver_clone() { + let fields = declared(&["a"]); + let nested = Expr::Call { + callee: Box::new(Expr::PropertyGet { + object: Box::new(Expr::This), + property: "helper".to_string(), + byte_offset: 0, + }), + args: Vec::new(), + type_args: Vec::new(), + byte_offset: 0, + }; + let m = method(vec![Stmt::Expr(this_get("a")), Stmt::Expr(nested)]); + + assert_eq!( + proven_receiver_clone_benefit_sites(&m, &fields, &HashSet::new()), + 1 + ); + assert!(!tower_route_profitable(&m, &fields, &HashSet::new())); + + let methods = declared(&["helper"]); + assert_eq!( + proven_receiver_clone_benefit_sites(&m, &fields, &methods), + 2 + ); + assert!(tower_route_profitable(&m, &fields, &methods)); } diff --git a/crates/perry-codegen/src/lib.rs b/crates/perry-codegen/src/lib.rs index 95f01a63c2..c0d5068640 100644 --- a/crates/perry-codegen/src/lib.rs +++ b/crates/perry-codegen/src/lib.rs @@ -76,6 +76,19 @@ pub use codegen::{ }; pub use collectors::CjsPreambleCensus; +/// Return the guarded proven-`this` method-clone capabilities a native module +/// may safely publish to importing codegen units. The first map contains all +/// eligible methods; the second is the profitable subset for class-ID dispatch +/// towers, whose extra receiver-shape recheck is not free. +pub fn exported_proven_this_method_capabilities( + hir: &perry_hir::Module, +) -> ( + std::collections::HashMap>, + std::collections::HashMap>, +) { + collectors::exportable_proven_this_method_capabilities(hir) +} + /// The shadow-stack field offsets generated code bakes into its inline root /// stores (#7088). /// diff --git a/crates/perry-codegen/src/lower_call/field_init.rs b/crates/perry-codegen/src/lower_call/field_init.rs index fd14763c34..9090685aaa 100644 --- a/crates/perry-codegen/src/lower_call/field_init.rs +++ b/crates/perry-codegen/src/lower_call/field_init.rs @@ -463,6 +463,22 @@ pub(crate) enum FieldInitMode { AfterRoot, } +/// Whether a named public field initializer can populate the allocation's +/// predeclared own slot through the ordinary by-name store. +/// +/// A fresh ordinary instance already owns every named field in its class-key +/// layout, so overwriting that slot has the same DefineField semantics as +/// CreateDataProperty: an inherited setter cannot intercept an existing own +/// data property. The exception is a constructor chain that can replace +/// `this` (the replacement may be a Proxy), or a name whose chain contains an +/// accessor/redeclaration and therefore has no stable global field index. +/// Those cases must keep `js_class_field_add` and its full +/// `[[DefineOwnProperty]]` behavior. +fn can_store_predeclared_public_field(ctx: &FnCtx<'_>, class_name: &str, property: &str) -> bool { + !crate::lower_call::ctor_chain_can_replace_this(ctx.classes, class_name) + && crate::type_analysis::class_field_global_index(ctx, class_name, property).is_some() +} + pub(crate) fn apply_field_initializers_recursive( ctx: &mut FnCtx<'_>, class_name: &str, @@ -824,15 +840,34 @@ pub(crate) fn apply_field_initializers_recursive( let key_handle_global = format!("@{}", ctx.strings.entry(key_idx).handle_global); let blk = ctx.block(); let key_box = blk.load(DOUBLE, &key_handle_global); - blk.call( - DOUBLE, - "js_class_field_add", - &[ - (DOUBLE, &this_val), - (DOUBLE, &key_box), - (DOUBLE, &closure_val), - ], - ); + if can_store_predeclared_public_field(ctx, &class_name_in_chain, &prop) { + // The field is already an own key in the freshly allocated + // exact class shape. Store by name so the runtime fills the + // existing slot without `mark_object_dynamic_shape_unknown`. + // This matters for the exact-shape guards emitted inside a + // hot captures-`this` arrow: full DefineOwnProperty used to + // change the receiver's shape before the arrow was ever + // called, making every guard miss (#8693 / perform-ecs). + let blk = ctx.block(); + let this_bits = blk.bitcast_double_to_i64(&this_val); + let this_raw = blk.and(I64, &this_bits, POINTER_MASK_I64); + let key_bits = blk.bitcast_double_to_i64(&key_box); + let key_raw = blk.and(I64, &key_bits, POINTER_MASK_I64); + blk.call_void( + "js_object_set_field_by_name", + &[(I64, &this_raw), (I64, &key_raw), (DOUBLE, &closure_val)], + ); + } else { + ctx.block().call( + DOUBLE, + "js_class_field_add", + &[ + (DOUBLE, &this_val), + (DOUBLE, &key_box), + (DOUBLE, &closure_val), + ], + ); + } continue; } @@ -858,12 +893,7 @@ pub(crate) fn apply_field_initializers_recursive( // `PropertySet` path (inline shape precheck -> direct slot store) // exactly as this did before #8630. Anything else keeps the full // DefineField call. - let chain_can_replace_this = - crate::lower_call::ctor_chain_can_replace_this(ctx.classes, &class_name_in_chain); - let no_accessor_on_chain = - crate::type_analysis::class_field_global_index(ctx, &class_name_in_chain, &prop) - .is_some(); - if no_accessor_on_chain && !chain_can_replace_this { + if can_store_predeclared_public_field(ctx, &class_name_in_chain, &prop) { let set_expr = Expr::PropertySet { object: Box::new(Expr::This), property: prop, diff --git a/crates/perry-codegen/src/lower_call/field_init/tests.rs b/crates/perry-codegen/src/lower_call/field_init/tests.rs index 1dd7a0517c..9a553a22d4 100644 --- a/crates/perry-codegen/src/lower_call/field_init/tests.rs +++ b/crates/perry-codegen/src/lower_call/field_init/tests.rs @@ -9,7 +9,7 @@ use super::*; use perry_hir::types::Type; -use perry_hir::{Class, ClassField, Function, Param}; +use perry_hir::{Class, ClassField, Function, Module, ModuleInitKind, Param}; fn param(id: u32, name: &str) -> Param { Param { @@ -587,3 +587,89 @@ fn single_class_predicate_still_refuses_heritage() { ); assert!(ctor_prologue_param_assigned_fields(&derived).is_empty()); } + +fn captures_this_arrow_field() -> ClassField { + ClassField { + name: "createEntity".to_string(), + key_expr: None, + ty: Type::Any, + init: Some(Expr::Closure { + func_id: 8693, + params: Vec::new(), + return_type: Type::Any, + body: vec![Stmt::Return(Some(Expr::PropertyGet { + object: Box::new(Expr::This), + property: "value".to_string(), + byte_offset: 0, + }))], + captures: Vec::new(), + mutable_captures: Vec::new(), + captures_this: true, + captures_new_target: false, + enclosing_class: Some("ArrowRegistry".to_string()), + is_arrow: true, + is_async: false, + is_generator: false, + is_strict: true, + }), + is_private: false, + is_readonly: false, + decorators: Vec::new(), + } +} + +fn captures_this_field_ir() -> String { + let mut registry = class( + vec![ + ClassField { + name: "value".to_string(), + key_expr: None, + ty: Type::Number, + init: Some(Expr::Number(1.0)), + is_private: false, + is_readonly: false, + decorators: Vec::new(), + }, + captures_this_arrow_field(), + ], + None, + ); + registry.id = 8693; + registry.name = "ArrowRegistry".to_string(); + + let mut module = Module::new("issue_8693_arrow_field.ts"); + module.classes = vec![registry]; + module.init = vec![Stmt::Expr(Expr::New { + class_name: "ArrowRegistry".to_string(), + args: Vec::new(), + type_args: Vec::new(), + byte_offset: 0, + cap_args_appended: 0, + })]; + module.init_kind = ModuleInitKind::Eager; + let opts = crate::CompileOptions { + emit_ir_only: true, + output_type: "executable".to_string(), + ..Default::default() + }; + String::from_utf8(crate::compile_module(&module, opts).expect("fixture compiles")) + .expect("LLVM IR is UTF-8") +} + +/// #8693: a captures-`this` arrow field on a fresh ordinary class instance +/// must populate the field already present in the allocation's class-key +/// shape. Full `DefineOwnProperty` marks the receiver dynamically shaped, so +/// every exact-shape method guard inside the arrow would miss forever (the +/// perform-ecs `createEntity = (...) => this.addComponentsToEntity(...)` case). +#[test] +fn captures_this_arrow_field_preserves_the_predeclared_class_shape() { + let ir = captures_this_field_ir(); + assert!( + ir.contains("call void @js_object_set_field_by_name("), + "the arrow field must fill its existing own slot:\n{ir}" + ); + assert!( + !ir.contains("call double @js_class_field_add("), + "the ordinary arrow field must not dynamically reshape its receiver:\n{ir}" + ); +} diff --git a/crates/perry-codegen/src/lower_call/method_override.rs b/crates/perry-codegen/src/lower_call/method_override.rs index 8cbf5922c4..6b8a0bda62 100644 --- a/crates/perry-codegen/src/lower_call/method_override.rs +++ b/crates/perry-codegen/src/lower_call/method_override.rs @@ -376,9 +376,9 @@ pub(super) fn emit_guarded_direct_method_call( // the `js_method_direct_shape_guard` / // `js_typed_feedback_method_direct_call_guard` branch, which matched the // exact class id AND the keys token. A `pshape_methods` hit additionally - // proves `receiver_class_name` DECLARES `property` (the map holds own - // declarations of module-local classes only), so the clone's `this` is - // exactly the class it was compiled for and can never be a subclass + // proves `receiver_class_name` DECLARES `property` (locally by analysis or + // across modules by a producer-authored capability), so the clone's `this` + // is exactly the class it was compiled for and can never be a subclass // instance. // // The `perry_static_` exclusion is carried forward from the guard-free @@ -1043,9 +1043,9 @@ pub(super) fn emit_guarded_direct_method_call( // instead; identical ABI, so only the callee name changes. // // A `pshape_methods` hit additionally proves `receiver_class_name` - // DECLARES `property` (the map holds own declarations of - // module-local classes only), so the clone's `this` is exactly the - // class it was compiled for — an inherited `Base::m` reached + // DECLARES `property` (locally by analysis or across modules by a + // producer-authored capability), so the clone's `this` is exactly + // the class it was compiled for — an inherited `Base::m` reached // through a subclass receiver never routes here. // // NOTE: the per-field `js_typed_feedback_class_field_get_guard` @@ -1065,7 +1065,39 @@ pub(super) fn emit_guarded_direct_method_call( let target = nonnegative_index_direct_fn .or(pshape_fn.as_deref()) .unwrap_or(direct_fn); - ctx.block().call(DOUBLE, target, direct_arg_slices) + let result = ctx.block().call(DOUBLE, target, direct_arg_slices); + if nonnegative_index_direct_fn.is_none() { + if let Some(pshape) = pshape_fn.as_deref() { + let receiver_provenance = + if ctx.imported_class_sources.contains_key(receiver_class_name) { + "imported_class_metadata" + } else { + "module_local_analysis" + }; + ctx.record_lowered_value( + "MethodCall", + None, + "proven_this_method_direct_call", + &LoweredValue::js_value(result.clone()), + None, + None, + None, + false, + false, + vec![ + format!("typed_clone={pshape}"), + format!("generic_method={direct_fn}"), + format!("receiver_class={receiver_class_name}"), + format!("method={property}"), + format!("receiver_provenance={receiver_provenance}"), + "this_representation=tagged_js_value_exact_shape".to_string(), + "method_identity_guard=required".to_string(), + "generic_dispatch_fallback=js_native_call_method_by_id".to_string(), + ], + ); + } + } + result } }; let after_fast = ctx.block().label.clone(); diff --git a/crates/perry-codegen/src/lower_call/typed_shape_bake_tests.rs b/crates/perry-codegen/src/lower_call/typed_shape_bake_tests.rs index 65aac203fa..b684b90afe 100644 --- a/crates/perry-codegen/src/lower_call/typed_shape_bake_tests.rs +++ b/crates/perry-codegen/src/lower_call/typed_shape_bake_tests.rs @@ -474,6 +474,8 @@ fn imported_remote() -> ImportedClass { constructor_has_rest: false, has_instance_fields: true, method_names: vec!["read".to_string()], + proven_this_method_names: Vec::new(), + proven_this_tower_method_names: Vec::new(), method_return_types: vec![Type::Number], method_param_counts: vec![0], method_has_rest: vec![false], diff --git a/crates/perry-runtime/src/object/class_registry.rs b/crates/perry-runtime/src/object/class_registry.rs index f318f14910..d7ae50b51f 100644 --- a/crates/perry-runtime/src/object/class_registry.rs +++ b/crates/perry-runtime/src/object/class_registry.rs @@ -105,13 +105,14 @@ pub(crate) use class_meta::{ }; #[cfg(test)] pub(crate) use prototype_methods::{ - class_prototype_fast_guards_invalidated, class_prototype_method_guard_slot, - CLASS_PROTOTYPE_FAST_GUARDS_INVALIDATED, CLASS_PROTOTYPE_FAST_GUARDS_INVALIDATED_BY_METHOD, + class_prototype_fast_guards_invalidated, CLASS_PROTOTYPE_FAST_GUARDS_INVALIDATED, + CLASS_PROTOTYPE_FAST_GUARDS_INVALIDATED_BY_METHOD, }; // ── prototype_methods.rs ──────────────────────────────────────────────────── pub(crate) use prototype_methods::{ - class_prototype_fast_guard_invalidated_for_method, class_prototype_method_root_store, + class_prototype_fast_guard_invalidated_for_method, class_prototype_method_guard_slot, + class_prototype_method_root_remove, class_prototype_method_root_store, invalidate_class_prototype_fast_guards_for_method, mirror_prototype_method_on_object, synthetic_class_id_for_function, }; diff --git a/crates/perry-runtime/src/object/class_registry/prototype_methods.rs b/crates/perry-runtime/src/object/class_registry/prototype_methods.rs index 919bbca739..9a282efccb 100644 --- a/crates/perry-runtime/src/object/class_registry/prototype_methods.rs +++ b/crates/perry-runtime/src/object/class_registry/prototype_methods.rs @@ -204,7 +204,8 @@ pub(crate) fn invalidate_class_prototype_fast_guards() { } pub(crate) fn class_prototype_method_root_store(class_id: u32, name: String, value_bits: u64) { - // Assignment after `delete C.prototype.m` creates the property again. + // Assignment / defineProperty after `delete C.prototype.m` recreates the + // own property and must make it visible to dispatch again. class_unmark_key_deleted(class_id, &name); CLASS_PROTOTYPE_METHODS.with(|table| { let mut guard = table.write().unwrap(); @@ -252,6 +253,19 @@ pub(crate) fn class_prototype_method_root_store(class_id: u32, name: String, val } } +/// Remove a runtime-assigned method when defineProperty restores a declared +/// vtable method. The declaration becomes authoritative again after the +/// caller clears the deletion marker and invalidates dispatch caches. +pub(crate) fn class_prototype_method_root_remove(class_id: u32, name: &str) { + CLASS_PROTOTYPE_METHODS.with(|table| { + if let Ok(mut guard) = table.write() { + if let Some(per_class) = guard.as_mut().and_then(|map| map.get_mut(&class_id)) { + per_class.remove(name); + } + } + }); +} + /// #5024: write a side-table-registered prototype method onto the /// materialized prototype object so the key lands in its `keys_array`. /// `enumerable` carries assignment semantics (`Class.prototype.m = fn` → diff --git a/crates/perry-runtime/src/object/delete_rest.rs b/crates/perry-runtime/src/object/delete_rest.rs index 0efd8fc607..8f905d8a15 100644 --- a/crates/perry-runtime/src/object/delete_rest.rs +++ b/crates/perry-runtime/src/object/delete_rest.rs @@ -270,9 +270,12 @@ pub extern "C" fn js_object_delete_field( if let Some(name) = super::has_own_helpers::str_from_string_header(key) { if name != "constructor" && (super::class_registry::class_own_accessor_ptrs(cid, name).is_some() - || super::native_module::class_has_own_method(cid, name)) + || super::native_module::class_has_own_method(cid, name) + || super::class_registry::lookup_own_prototype_method(cid, name).is_some()) { super::class_registry::class_mark_key_deleted(cid, name); + super::class_registry::invalidate_class_prototype_fast_guards_for_method(name); + crate::typed_feedback::invalidate_method_change(cid); // Accessors have no keys_array entry, so the scan below is a // vacuous success for them; methods DO, so fall through to // remove it. Either way, don't early-return. @@ -435,6 +438,20 @@ fn delete_receiver_is_pointer(obj_value: f64) -> bool { crate::value::JSValue::from_bits(obj_value.to_bits()).is_pointer() } +fn delete_class_prototype_key(class_id: u32, name: &str) -> i32 { + let has_own = name == "constructor" + || super::native_module::class_has_own_method(class_id, name) + || super::class_registry::class_own_accessor_ptrs(class_id, name).is_some() + || super::class_registry::lookup_own_prototype_method(class_id, name).is_some(); + if !has_own { + return 1; + } + super::class_registry::class_mark_key_deleted(class_id, name); + super::class_registry::invalidate_class_prototype_fast_guards_for_method(name); + crate::typed_feedback::invalidate_method_change(class_id); + 1 +} + /// `delete prim.field` (static key): once RequireObjectCoercible has rejected /// null/undefined, a primitive receiver (number/boolean/…) has no deletable own /// property, so `delete` is a no-op that evaluates to `true` (spec ToObject of a @@ -446,6 +463,16 @@ pub extern "C" fn js_object_delete_field_value( obj_value: f64, key: *const crate::StringHeader, ) -> i32 { + if let Some(class_id) = super::class_prototype_ref_id(obj_value) { + if key.is_null() { + return 1; + } + return unsafe { + super::has_own_helpers::str_from_string_header(key) + .map(|name| delete_class_prototype_key(class_id, name)) + .unwrap_or(1) + }; + } // A class reference (`delete C.m` for a `static m()`) is INT32-tagged, so // `is_pointer` is false and the guard below would no-op it. But a static // member delete must still unregister the method/field. `js_object_delete_field` @@ -468,6 +495,13 @@ pub extern "C" fn js_object_delete_field_value( /// `js_object_delete_field_value`, delegating real objects to the dynamic path. #[no_mangle] pub extern "C" fn js_object_delete_dynamic_value(obj_value: f64, key: f64) -> i32 { + if let Some(class_id) = super::class_prototype_ref_id(obj_value) { + return unsafe { + super::native_module::metadata_key_to_string(key) + .map(|name| delete_class_prototype_key(class_id, &name)) + .unwrap_or(1) + }; + } // Class-ref receiver (`delete C["m"]`): see `js_object_delete_field_value`. if let Some(class_id) = super::native_module::class_ref_id(obj_value) { return js_object_delete_dynamic(class_id as usize as *mut ObjectHeader, key); diff --git a/crates/perry-runtime/src/object/field_set_by_name.rs b/crates/perry-runtime/src/object/field_set_by_name.rs index ac6af9c80e..1c58881eab 100644 --- a/crates/perry-runtime/src/object/field_set_by_name.rs +++ b/crates/perry-runtime/src/object/field_set_by_name.rs @@ -372,83 +372,73 @@ pub extern "C" fn js_object_set_field_by_name( let name = std::str::from_utf8(std::slice::from_raw_parts(name_ptr, name_len)) .unwrap_or("") .to_string(); - // Empty-string is a legal accessor key (`set ''(v)`); the - // `!name.is_empty()` guard below skips it, so dispatch a - // prototype-ref instance setter / constructor-ref static setter - // named "" here (Test262 accessor-name-* literal-string-empty). - if name.is_empty() { - let recv = f64::from_bits(bits); - if super::class_prototype_ref_id(recv).is_some() - && super::class_registry::class_instance_setter_apply( - class_id, &name, recv, value, - ) - { - return; - } - if super::class_registry::class_static_accessor_setter_apply( - class_id, &name, recv, value, - ) { - return; - } + let recv = f64::from_bits(bits); + let is_prototype_ref = super::class_prototype_ref_id(recv).is_some(); + if !is_prototype_ref + && name == "name" + && !super::class_registry::class_is_key_deleted(class_id, &name) + && super::class_registry::lookup_static_method_in_chain(class_id, &name) + .is_none() + { + return; } - if !name.is_empty() { - if name == "name" - && !super::class_registry::class_is_key_deleted(class_id, &name) - && super::class_registry::lookup_static_method_in_chain(class_id, &name) - .is_none() - { - return; - } - let has_own_data = CLASS_DYNAMIC_PROPS.with(|m| { + let has_own_data = if is_prototype_ref { + super::class_registry::lookup_own_prototype_method(class_id, &name) + .is_some() + || super::native_module::class_has_own_method(class_id, &name) + } else { + CLASS_DYNAMIC_PROPS.with(|m| { m.borrow() .get(&class_id) .is_some_and(|props| props.contains_key(&name)) - }); - // `C.prototype[key] = v` where `key` is an instance - // `set key(v)` accessor defined on the prototype: invoke the - // setter with `this` = the prototype ref. The prototype ref - // and the constructor ref are both INT32-tagged class refs; - // distinguish via `class_prototype_ref_id`. Instance setters - // live in the vtable; static accessors (below) live in the - // constructor ref's table (Test262 accessor-name-inst). - if !has_own_data - && super::class_prototype_ref_id(f64::from_bits(bits)).is_some() - && super::class_registry::class_instance_setter_apply( - class_id, - &name, - f64::from_bits(bits), - value, - ) - { + }) + }; + // `C.prototype[key] = v` where `key` is an instance + // accessor invokes the setter with `this = C.prototype`. + // A getter-only accessor absorbs a non-strict assignment. + if is_prototype_ref && !has_own_data { + if super::class_registry::class_instance_setter_apply( + class_id, &name, recv, value, + ) { return; } - if !has_own_data - && super::class_registry::class_static_accessor_setter_apply( - class_id, - &name, - f64::from_bits(bits), - value, - ) - { + if super::class_registry::class_has_instance_getter(class_id, &name) { return; } - // Writing `.caller` / `.arguments` on a class constructor - // hits the poison-pill %ThrowTypeError% accessor (which has - // no [[Set]]) on `Function.prototype`, so a strict-mode - // assignment throws. Mirrors the read side in - // get_field_by_name and the ordinary-closure setter path. - // A `defineProperty`-installed own data prop was handled by - // `has_own_data` above; prototype-refs (`C.prototype`) are - // plain objects with no such restriction. - if !has_own_data - && matches!(name.as_str(), "caller" | "arguments") - && super::class_prototype_ref_id(f64::from_bits(bits)).is_none() - { - crate::fs::validate::throw_type_error_with_code( - "Restricted function property access", - "ERR_INVALID_ARG_TYPE", - ); - } + } else if !is_prototype_ref + && !has_own_data + && super::class_registry::class_static_accessor_setter_apply( + class_id, &name, recv, value, + ) + { + return; + } + // Writing `.caller` / `.arguments` on a class constructor + // hits the poison-pill %ThrowTypeError% accessor inherited + // from Function.prototype. Prototype refs are plain objects. + if !is_prototype_ref + && !has_own_data + && matches!(name.as_str(), "caller" | "arguments") + { + crate::fs::validate::throw_type_error_with_code( + "Restricted function property access", + "ERR_INVALID_ARG_TYPE", + ); + } + if is_prototype_ref { + // Imported `C.prototype.m = value` reaches this generic + // class-ref path. Publish it as an enumerable prototype + // data property so instance dispatch sees the replacement. + super::class_registry::class_prototype_method_set_enumerable( + class_id, &name, true, + ); + super::class_registry::class_prototype_method_root_store( + class_id, + name, + value.to_bits(), + ); + crate::typed_feedback::invalidate_method_change(class_id); + } else { class_dynamic_prop_root_store(class_id, &name, value); } } diff --git a/crates/perry-runtime/src/object/native_call_method.rs b/crates/perry-runtime/src/object/native_call_method.rs index 7d3eddd4dc..bc6303a099 100644 --- a/crates/perry-runtime/src/object/native_call_method.rs +++ b/crates/perry-runtime/src/object/native_call_method.rs @@ -2059,7 +2059,11 @@ pub unsafe extern "C-unwind" fn js_native_call_method( // Vtable lookup: check if this class has a registered method in the vtable let class_id = (*obj).class_id; - if class_id != 0 && !class_is_key_deleted(class_id, method_name) { + if class_id != 0 + && (!class_prototype_fast_guard_invalidated_for_method( + class_prototype_method_guard_slot(method_name), + ) || !class_is_key_deleted(class_id, method_name)) + { if let Ok(registry) = CLASS_VTABLE_REGISTRY.read() { if let Some(ref reg) = *registry { if let Some(vtable) = reg.get(&class_id) { diff --git a/crates/perry-runtime/src/object/native_call_method/handle_methods.rs b/crates/perry-runtime/src/object/native_call_method/handle_methods.rs index aec8c9e67f..2ab2541630 100644 --- a/crates/perry-runtime/src/object/native_call_method/handle_methods.rs +++ b/crates/perry-runtime/src/object/native_call_method/handle_methods.rs @@ -1012,16 +1012,24 @@ pub(super) unsafe fn dispatch_handle( }, } let mut resolved_method: Option = None; + // Prototype assignments/deletes for this method are rare. + // Until its scoped guard is retired, the producer-registered + // vtable is authoritative and the per-class side tables cannot + // contain an override for this name. + let prototype_mutated = class_prototype_fast_guard_invalidated_for_method( + class_prototype_method_guard_slot(method_name), + ); if let Ok(registry) = CLASS_VTABLE_REGISTRY.read() { if let Some(ref reg) = *registry { let mut cur_cid = class_id; let mut depth = 0u32; while depth < 32 { - let deleted = class_is_key_deleted(cur_cid, method_name); - // `C.prototype.m = fn` replaces a declared `m` on - // this exact prototype object, so the assignment - // side table must win before the original vtable. - if !deleted { + let deleted = + prototype_mutated && class_is_key_deleted(cur_cid, method_name); + // A runtime assignment is an own property of this + // exact prototype and replaces the declared vtable + // entry. Resolve it first; deletion hides both. + if prototype_mutated && !deleted { if let Some(method_value) = lookup_own_prototype_method(cur_cid, method_name) { diff --git a/crates/perry-runtime/src/object/object_ops/define_property.rs b/crates/perry-runtime/src/object/object_ops/define_property.rs index a3d0a41066..616a30f17e 100644 --- a/crates/perry-runtime/src/object/object_ops/define_property.rs +++ b/crates/perry-runtime/src/object/object_ops/define_property.rs @@ -79,6 +79,13 @@ unsafe fn define_class_prototype_method(target_cid: u32, name: &str, value_bits: }, ); drop(guard); + super::super::class_registry::class_prototype_method_root_remove( + target_cid, name, + ); + super::super::class_registry::class_unmark_key_deleted(target_cid, name); + super::super::class_registry::invalidate_class_prototype_fast_guards_for_method( + name, + ); super::super::class_registry::js_register_class_id(target_cid); crate::typed_feedback::invalidate_method_change(target_cid); return; diff --git a/crates/perry-runtime/src/proxy.rs b/crates/perry-runtime/src/proxy.rs index e85029ad75..63fe558d2e 100644 --- a/crates/perry-runtime/src/proxy.rs +++ b/crates/perry-runtime/src/proxy.rs @@ -1173,6 +1173,19 @@ fn target_set(target: f64, key: f64, value: f64) { // already-heap `STRING_TAG` value, which `js_string_coerce` hands straight // back without touching the allocator. let key_ptr = crate::builtins::js_string_coerce(property_key) as *const crate::StringHeader; + let target_addr = extract_pointer(target.to_bits()) as usize; + if let Some(class_id) = crate::object::class_id_for_decl_prototype_object(target_addr) { + // Imported `C.prototype.m = value` materializes the declaration's + // prototype object before PutValue reaches this shared write tail. + // Keep the runtime method registry authoritative so instance dispatch + // observes the replacement and direct guards retire. + if let Some(name) = key_to_rust_string(property_key) { + crate::object::class_prototype_method_set_enumerable(class_id, &name, true); + crate::object::class_prototype_method_root_store(class_id, name, value.to_bits()); + crate::typed_feedback::invalidate_method_change(class_id); + } + return; + } if crate::object::class_ref_id(target).is_some() { // Preserve the INT32-tagged class-ref bits so class dynamic props // land in CLASS_DYNAMIC_PROPS instead of being pointer-extracted to 0. @@ -1185,7 +1198,7 @@ fn target_set(target: f64, key: f64, value: f64) { } return; } - let obj_addr = extract_pointer(target.to_bits()) as usize; + let obj_addr = target_addr; if crate::closure::is_closure_ptr(obj_addr) { if let Some(name) = key_to_rust_string(property_key) { crate::closure::closure_set_dynamic_prop(obj_addr, &name, value); diff --git a/crates/perry/src/commands/compile/object_cache.rs b/crates/perry/src/commands/compile/object_cache.rs index 8fdc1e1b62..3008287468 100644 --- a/crates/perry/src/commands/compile/object_cache.rs +++ b/crates/perry/src/commands/compile/object_cache.rs @@ -598,6 +598,14 @@ fn compute_object_cache_key_with_env( .collect::>() .join(","), ); + buf.push_str(":proven_this_methods="); + let mut proven_this_methods = c.proven_this_method_names.clone(); + proven_this_methods.sort(); + buf.push_str(&proven_this_methods.join(",")); + buf.push_str(":proven_this_tower_methods="); + let mut proven_this_tower_methods = c.proven_this_tower_method_names.clone(); + proven_this_tower_methods.sort(); + buf.push_str(&proven_this_tower_methods.join(",")); buf.push_str(":method_synthetic_arguments="); buf.push_str( &c.method_has_synthetic_arguments diff --git a/crates/perry/src/commands/compile/object_cache/object_cache_tests.rs b/crates/perry/src/commands/compile/object_cache/object_cache_tests.rs index f7d752bb52..d31e8f9143 100644 --- a/crates/perry/src/commands/compile/object_cache/object_cache_tests.rs +++ b/crates/perry/src/commands/compile/object_cache/object_cache_tests.rs @@ -352,6 +352,8 @@ fn key_stable_for_nested_type_hashmap_order() { constructor_has_rest: false, has_instance_fields: true, method_names: vec![], + proven_this_method_names: vec![], + proven_this_tower_method_names: vec![], method_return_types: vec![], method_param_counts: vec![], method_has_rest: vec![], @@ -408,6 +410,8 @@ fn key_changes_with_imported_class_signature() { constructor_has_rest: false, has_instance_fields: true, method_names: vec!["bar".into()], + proven_this_method_names: vec![], + proven_this_tower_method_names: vec![], method_return_types: vec![perry_hir::types::Type::Number], method_param_counts: vec![0], method_has_rest: vec![false], @@ -437,6 +441,8 @@ fn key_changes_with_imported_class_signature() { constructor_has_rest: false, has_instance_fields: true, method_names: vec!["bar".into()], + proven_this_method_names: vec![], + proven_this_tower_method_names: vec![], method_return_types: vec![perry_hir::types::Type::Number], method_param_counts: vec![0], method_has_rest: vec![false], @@ -474,6 +480,8 @@ fn key_changes_with_imported_class_codegen_surface() { constructor_has_rest: false, has_instance_fields: true, method_names: vec!["bar".into()], + proven_this_method_names: vec![], + proven_this_tower_method_names: vec![], method_return_types: vec![perry_hir::types::Type::Number], method_param_counts: vec![1], method_has_rest: vec![false], @@ -517,6 +525,14 @@ fn key_changes_with_imported_class_codegen_surface() { changed.method_has_synthetic_arguments = vec![true]; assert_ne!(base_key, key_for(changed)); + let mut changed = base.clone(); + changed.proven_this_method_names = vec!["bar".into()]; + assert_ne!(base_key, key_for(changed)); + + let mut changed = base.clone(); + changed.proven_this_tower_method_names = vec!["bar".into()]; + assert_ne!(base_key, key_for(changed)); + let mut changed = base.clone(); changed.method_return_types = vec![perry_hir::types::Type::String]; assert_ne!(base_key, key_for(changed)); diff --git a/crates/perry/src/commands/compile/run_pipeline.rs b/crates/perry/src/commands/compile/run_pipeline.rs index 38eb33345c..2cd1e5794d 100644 --- a/crates/perry/src/commands/compile/run_pipeline.rs +++ b/crates/perry/src/commands/compile/run_pipeline.rs @@ -220,6 +220,8 @@ fn imported_class_from_hir( class: &perry_hir::Class, source_prefix: String, local_alias: Option, + proven_this_method_names: Vec, + proven_this_tower_method_names: Vec, ) -> perry_codegen::ImportedClass { perry_codegen::ImportedClass { name: class.name.clone(), @@ -242,6 +244,8 @@ fn imported_class_from_hir( .iter() .map(|method| method.name.clone()) .collect(), + proven_this_method_names, + proven_this_tower_method_names, method_return_types: class .methods .iter() @@ -338,6 +342,16 @@ fn imported_class_from_hir( } } +fn proven_this_methods_for_import( + class: &perry_hir::Class, + published: &std::collections::HashMap>, +) -> Vec { + published + .get(&(class as *const perry_hir::Class as usize)) + .cloned() + .unwrap_or_default() +} + /// Collect class names reachable through a declared type. Imported class /// metadata is non-owning compile-time information, so following container, /// object, and callable types is conservative: it can make a later derived @@ -891,6 +905,30 @@ pub fn run_with_parse_cache( } } + // Producer-authored proven-`this` capabilities. Key by the concrete HIR + // class object, just like `class_canonical_path`: re-export aliases point + // at the same definition and therefore publish the same clone set. This + // runs before parallel codegen so consumers never infer capabilities from + // their own (necessarily body-less) imported stubs. + let mut class_proven_this_methods: std::collections::HashMap> = + std::collections::HashMap::new(); + let mut class_proven_this_tower_methods: std::collections::HashMap> = + std::collections::HashMap::new(); + for hir_module in ctx.native_modules.values() { + let (by_name, tower_by_name) = + perry_codegen::exported_proven_this_method_capabilities(hir_module); + for class in &hir_module.classes { + if let Some(methods) = by_name.get(&class.name) { + class_proven_this_methods + .insert(class as *const perry_hir::Class as usize, methods.clone()); + } + if let Some(methods) = tower_by_name.get(&class.name) { + class_proven_this_tower_methods + .insert(class as *const perry_hir::Class as usize, methods.clone()); + } + } + } + // Propagate enum re-exports: when module A has `export * from "./B"`, // all enums exported from B should also be accessible via A's path. loop { @@ -3185,6 +3223,14 @@ pub fn run_with_parse_cache( class, class_prefix, Some(local.clone()), + proven_this_methods_for_import( + class, + &class_proven_this_methods, + ), + proven_this_methods_for_import( + class, + &class_proven_this_tower_methods, + ), )); } if let Some(members) = exported_enums.get(&key) { @@ -3339,6 +3385,14 @@ pub fn run_with_parse_cache( class, class_prefix, local_alias, + proven_this_methods_for_import( + class, + &class_proven_this_methods, + ), + proven_this_methods_for_import( + class, + &class_proven_this_tower_methods, + ), )); } if let Some(members) = exported_enums.get(&key) { @@ -3791,6 +3845,14 @@ pub fn run_with_parse_cache( class, class_prefix, local_alias, + proven_this_methods_for_import( + class, + &class_proven_this_methods, + ), + proven_this_methods_for_import( + class, + &class_proven_this_tower_methods, + ), )); } if let Some(members) = exported_enums.get(&key) { @@ -4042,6 +4104,14 @@ pub fn run_with_parse_cache( class, class_prefix.clone(), Some(exported_name.clone()), + proven_this_methods_for_import( + class, + &class_proven_this_methods, + ), + proven_this_methods_for_import( + class, + &class_proven_this_tower_methods, + ), )); } imported_classes.push(imported_class_from_hir( @@ -4052,6 +4122,11 @@ pub fn run_with_parse_cache( } else { None }, + proven_this_methods_for_import(class, &class_proven_this_methods), + proven_this_methods_for_import( + class, + &class_proven_this_tower_methods, + ), )); } @@ -4113,6 +4188,14 @@ pub fn run_with_parse_cache( class, class_prefix, None, + proven_this_methods_for_import( + class, + &class_proven_this_methods, + ), + proven_this_methods_for_import( + class, + &class_proven_this_tower_methods, + ), )); imported_classes.len() - 1 } @@ -4196,7 +4279,16 @@ pub fn run_with_parse_cache( continue; } let class_prefix = compute_module_prefix(&src_path, &ctx.project_root); - imported_classes.push(imported_class_from_hir(class, class_prefix, None)); + imported_classes.push(imported_class_from_hir( + class, + class_prefix, + None, + proven_this_methods_for_import(class, &class_proven_this_methods), + proven_this_methods_for_import( + class, + &class_proven_this_tower_methods, + ), + )); } } } @@ -4623,7 +4715,16 @@ pub fn run_with_parse_cache( } else { None }; - imported_classes.push(imported_class_from_hir(class, class_prefix, alias)); + imported_classes.push(imported_class_from_hir( + class, + class_prefix, + alias, + proven_this_methods_for_import(class, &class_proven_this_methods), + proven_this_methods_for_import( + class, + &class_proven_this_tower_methods, + ), + )); visited_imports.insert(ref_name.clone()); // Process the entry we just pushed (by index, so a // same-named distinct-module class isn't skipped). Refs #26. diff --git a/crates/perry/tests/issue_8693_imported_this_specialization.rs b/crates/perry/tests/issue_8693_imported_this_specialization.rs new file mode 100644 index 0000000000..46bdcd9b2e --- /dev/null +++ b/crates/perry/tests/issue_8693_imported_this_specialization.rs @@ -0,0 +1,305 @@ +//! Cross-module proven-`this` specialization (#8693). +//! +//! The producer is the authority for clone eligibility. Importers may call the +//! published clone only behind the ordinary exact-class/shape guard; its +//! runtime-dispatch fallback remains present for every mutation-sensitive +//! semantic case. + +use std::path::{Path, PathBuf}; +use std::process::{Command, Output}; +use std::sync::Once; + +const GC_ENV_OVERRIDES: &[&str] = &[ + "PERRY_GEN_GC", + "PERRY_GC_SCAVENGE", + "PERRY_GC_SCAVENGE_NURSERY_MB", + "PERRY_GC_MOVING_SAFEPOINT", + "PERRY_GC_MOVING_LOOP_POLLS", + "PERRY_GC_FORCE_EVACUATE", + "PERRY_GC_VERIFY_EVACUATION", + "PERRY_CONSERVATIVE_STACK_SCAN", + "PERRY_WRITE_BARRIERS", + "PERRY_GC_INCREMENTAL", + "PERRY_GC_HEAP_LIMIT", +]; + +fn remove_gc_env_overrides(command: &mut Command) { + for key in GC_ENV_OVERRIDES { + command.env_remove(key); + } +} + +fn perry_bin() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_perry")) +} + +fn workspace_root() -> PathBuf { + PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .join("../..") + .canonicalize() + .expect("canonicalize workspace root") +} + +fn target_debug_dir() -> PathBuf { + let target = std::env::var_os("CARGO_TARGET_DIR") + .map(PathBuf::from) + .unwrap_or_else(|| workspace_root().join("target")); + if cfg!(windows) { + target.join("x86_64-pc-windows-msvc").join("debug") + } else { + target.join("debug") + } +} + +fn ensure_runtime_archive() { + static BUILD_RUNTIME: Once = Once::new(); + BUILD_RUNTIME.call_once(|| { + let cargo = std::env::var_os("CARGO").unwrap_or_else(|| "cargo".into()); + let mut command = Command::new(cargo); + command + .current_dir(workspace_root()) + .arg("build") + .arg("-p") + .arg("perry-runtime-static") + .arg("-p") + .arg("perry-stdlib-static"); + if cfg!(windows) { + command.arg("--target").arg("x86_64-pc-windows-msvc"); + } + let build = command.output().expect("build static runtime archives"); + assert!( + build.status.success(), + "static runtime build failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&build.stdout), + String::from_utf8_lossy(&build.stderr) + ); + }); +} + +fn fixture_dir() -> PathBuf { + workspace_root().join("test-files/fixtures/issue_8693_imported_this") +} + +fn copy_fixture(dir: &Path) { + for file in [ + "package.json", + "registry.js", + "barrel.js", + "main.js", + "semantics.js", + ] { + std::fs::copy(fixture_dir().join(file), dir.join(file)) + .unwrap_or_else(|error| panic!("copy {file}: {error}")); + } +} + +fn compile(dir: &Path, entry: &str, explain: bool) -> PathBuf { + ensure_runtime_archive(); + let output = dir.join(format!("{entry}.bin")); + let mut command = Command::new(perry_bin()); + command + .current_dir(dir) + .arg("compile") + .arg(entry) + .arg("-o") + .arg(&output) + .arg("--no-cache") + .arg("--trace") + .arg("llvm") + .env("PERRY_NO_AUTO_OPTIMIZE", "1") + .env("PERRY_RUNTIME_DIR", target_debug_dir()); + if explain { + command.arg("--opt-report=json").arg("--explain-lowering"); + } + remove_gc_env_overrides(&mut command); + let result = command.output().expect("run perry compile"); + assert_success("perry compile", &result); + output +} + +fn assert_success(label: &str, output: &Output) { + assert!( + output.status.success(), + "{label} failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); +} + +fn run(binary: &Path, dir: &Path, force_evacuation: bool) -> String { + let mut command = Command::new(binary); + command.current_dir(dir); + remove_gc_env_overrides(&mut command); + if force_evacuation { + command + .env("PERRY_GC_SCAVENGE", "1") + .env("PERRY_GC_SCAVENGE_NURSERY_MB", "1") + .env("PERRY_GC_FORCE_EVACUATE", "1") + .env("PERRY_GC_VERIFY_EVACUATION", "1") + .env("PERRY_GC_INCREMENTAL", "0"); + } + let output = command.output().expect("run compiled fixture"); + assert_success("compiled fixture", &output); + String::from_utf8(output.stdout).expect("fixture stdout is UTF-8") +} + +fn run_node(dir: &Path, entry: &str) -> String { + let output = Command::new("node") + .current_dir(dir) + .arg(entry) + .output() + .expect("run Node semantic oracle"); + assert_success("Node semantic oracle", &output); + String::from_utf8(output.stdout).expect("Node stdout is UTF-8") +} + +fn read_native_records(dir: &Path) -> Vec { + let lowering = dir.join(".perry-trace/lowering"); + let run_dir = std::fs::read_dir(&lowering) + .expect("read lowering directory") + .filter_map(Result::ok) + .map(|entry| entry.path()) + .find(|path| path.is_dir()) + .expect("lowering run directory"); + let mut records = Vec::new(); + for entry in std::fs::read_dir(run_dir).expect("read lowering run") { + let path = entry.expect("lowering entry").path(); + let Some(name) = path.file_name().and_then(|name| name.to_str()) else { + continue; + }; + if !name.starts_with("perry_native_reps_") || !name.ends_with(".json") { + continue; + } + let artifact: serde_json::Value = serde_json::from_slice( + &std::fs::read(&path) + .unwrap_or_else(|error| panic!("read {}: {error}", path.display())), + ) + .unwrap_or_else(|error| panic!("parse {}: {error}", path.display())); + records.extend( + artifact["records"] + .as_array() + .unwrap_or_else(|| panic!("missing records in {}", path.display())) + .iter() + .cloned(), + ); + } + records +} + +fn record_notes(record: &serde_json::Value) -> Vec<&str> { + record["notes"] + .as_array() + .into_iter() + .flatten() + .filter_map(serde_json::Value::as_str) + .collect() +} + +#[test] +fn imported_registry_uses_published_proven_this_clones_with_fallbacks() { + let temp = tempfile::tempdir().expect("tempdir"); + copy_fixture(temp.path()); + let binary = compile(temp.path(), "main.js", true); + + let perry_output = run(&binary, temp.path(), false); + let node_output = run_node(temp.path(), "main.js"); + let perry: serde_json::Value = serde_json::from_str(perry_output.trim()).expect("Perry JSON"); + let node: serde_json::Value = serde_json::from_str(node_output.trim()).expect("Node JSON"); + assert_eq!(perry["remaining"], 0); + assert_eq!(perry["remaining"], node["remaining"]); + + let main_ir = std::fs::read_to_string(temp.path().join(".perry-trace/llvm/main_js.ll")) + .expect("read main LLVM IR"); + let registry_ir = std::fs::read_to_string(temp.path().join(".perry-trace/llvm/registry_js.ll")) + .expect("read registry LLVM IR"); + for method in ["add", "remove"] { + let clone = format!("perry_method_registry_js__Registry__{method}$pshape"); + assert!( + main_ir.contains(&format!("declare double @{clone}(")), + "importer must declare producer clone {clone}:\n{main_ir}" + ); + let fast_block = main_ir + .split("\n\n") + .find(|block| block.contains("method_direct.fast.") && block.contains(&clone)) + .unwrap_or_else(|| panic!("no direct fast block calling {clone}:\n{main_ir}")); + assert!( + !fast_block.contains("js_native_call_method_by_id") + && !fast_block.contains("js_typed_feedback_native_call_method_by_id"), + "stable fast arm must not use generic method dispatch:\n{fast_block}" + ); + } + for method in ["pushEntity", "removeEntity"] { + let clone = format!("perry_method_registry_js__Group__{method}$pshape"); + assert!( + registry_ir.contains(&format!("define double @{clone}(")), + "producer clone must have external linkage: {clone}\n{registry_ir}" + ); + let definition = registry_ir + .lines() + .find(|line| line.contains(&format!("define double @{clone}("))) + .unwrap_or_else(|| panic!("missing clone definition for {clone}:\n{registry_ir}")); + assert!( + definition.contains("inlinehint") || definition.contains("alwaysinline"), + "small producer clone must be admitted to inlining: {definition}" + ); + assert!( + registry_ir.contains(&format!("call double @{clone}(")), + "Registry clone must directly select Group clone: {clone}\n{registry_ir}" + ); + } + assert!( + main_ir.contains("call double @js_native_call_method_by_id"), + "guard failure must retain generic runtime fallback:\n{main_ir}" + ); + + let records = read_native_records(temp.path()); + for method in ["add", "remove"] { + let suffix = format!("Registry__{method}$pshape"); + let selected = records.iter().find(|record| { + let notes = record_notes(record); + record["consumer"] == "proven_this_method_direct_call" + && notes + .iter() + .any(|note| note.starts_with("typed_clone=") && note.ends_with(&suffix)) + }); + let record = selected.unwrap_or_else(|| { + panic!("no lowering selection for imported {method}:\n{records:#?}") + }); + let notes = record_notes(record); + assert!(notes.contains(&"receiver_provenance=imported_class_metadata")); + assert!(notes.contains(&"this_representation=tagged_js_value_exact_shape")); + assert!(notes.contains(&"generic_dispatch_fallback=js_native_call_method_by_id")); + assert!( + !notes + .iter() + .any(|note| *note == "typed_clone_rejected=captures_this"), + "valid imported method must not be rejected for capturing this: {record:#}" + ); + } +} + +#[test] +fn imported_clone_guards_preserve_all_method_semantics_under_moving_gc() { + let temp = tempfile::tempdir().expect("tempdir"); + copy_fixture(temp.path()); + let binary = compile(temp.path(), "semantics.js", false); + let node = run_node(temp.path(), "semantics.js"); + let ordinary = run(&binary, temp.path(), false); + let moving = run(&binary, temp.path(), true); + assert_eq!(ordinary, node, "ordinary Perry output differs from Node"); + assert_eq!( + moving, node, + "forced-moving-GC Perry output differs from Node" + ); + + let semantics_ir = + std::fs::read_to_string(temp.path().join(".perry-trace/llvm/semantics_js.ll")) + .expect("read semantic fixture LLVM IR"); + let clone = "perry_method_barrel_js__TowerRegistry__cycle$pshape"; + assert!( + semantics_ir.split("\n\n").any(|block| { + block.contains("idispatch.case") && block.contains(&format!("call double @{clone}(")) + }), + "stable adapter-field tower must route to imported clone {clone}:\n{semantics_ir}" + ); +} diff --git a/test-files/fixtures/issue_8693_imported_this/barrel.js b/test-files/fixtures/issue_8693_imported_this/barrel.js new file mode 100644 index 0000000000..5112cda6ed --- /dev/null +++ b/test-files/fixtures/issue_8693_imported_this/barrel.js @@ -0,0 +1,15 @@ +export { Group, Registry } from './registry.js'; + +// Deliberately has multiple `this` field sites so an importing adapter's +// class-ID dispatch tower can profitably route to the producer's clone. +export class TowerRegistry { + left = []; + right = []; + + cycle(value) { + this.left.push(value); + this.right.push(value); + this.left.pop(); + this.right.pop(); + } +} diff --git a/test-files/fixtures/issue_8693_imported_this/main.js b/test-files/fixtures/issue_8693_imported_this/main.js new file mode 100644 index 0000000000..55c27d21c8 --- /dev/null +++ b/test-files/fixtures/issue_8693_imported_this/main.js @@ -0,0 +1,14 @@ +import { Registry } from './registry.js'; + +const registry = new Registry(); +const iterations = 200_000; +const start = performance.now(); +for (let i = 0; i < iterations; i++) { + const entity = { id: i }; + registry.add(entity); + registry.remove(entity); +} +console.log(JSON.stringify({ + elapsedMs: performance.now() - start, + remaining: registry.group.entities.length, +})); diff --git a/test-files/fixtures/issue_8693_imported_this/package.json b/test-files/fixtures/issue_8693_imported_this/package.json new file mode 100644 index 0000000000..3dbc1ca591 --- /dev/null +++ b/test-files/fixtures/issue_8693_imported_this/package.json @@ -0,0 +1,3 @@ +{ + "type": "module" +} diff --git a/test-files/fixtures/issue_8693_imported_this/registry.js b/test-files/fixtures/issue_8693_imported_this/registry.js new file mode 100644 index 0000000000..4768c5e8c5 --- /dev/null +++ b/test-files/fixtures/issue_8693_imported_this/registry.js @@ -0,0 +1,24 @@ +export class Group { + entities = []; + + pushEntity(entity) { + this.entities.push(entity); + } + + removeEntity(entity) { + const index = this.entities.indexOf(entity); + if (index !== -1) this.entities.splice(index, 1); + } +} + +export class Registry { + group = new Group(); + + add(entity) { + this.group.pushEntity(entity); + } + + remove(entity) { + this.group.removeEntity(entity); + } +} diff --git a/test-files/fixtures/issue_8693_imported_this/semantics.js b/test-files/fixtures/issue_8693_imported_this/semantics.js new file mode 100644 index 0000000000..2825cd80e3 --- /dev/null +++ b/test-files/fixtures/issue_8693_imported_this/semantics.js @@ -0,0 +1,143 @@ +import { + Registry as ImportedRegistry, + TowerRegistry, +} from './barrel.js'; + +function entity(id) { + return { id }; +} + +const results = []; +const stable = new ImportedRegistry(); +const first = entity(1); +stable.add(first); +stable.remove(first); +results.push(`stable:${stable.group.entities.length}`); + +class RegistryAdapter { + constructor(registry) { + this.registry = registry; + } + + cycle(value) { + this.registry.cycle(value); + } +} + +const adapter = new RegistryAdapter(new TowerRegistry()); +adapter.cycle(entity(101)); +results.push(`adapter:${adapter.registry.left.length}:${adapter.registry.right.length}`); + +const shadowed = new ImportedRegistry(); +shadowed.add = function (value) { + this.group.entities.push(entity(value.id + 100)); +}; +shadowed.add(entity(2)); +results.push(`shadow:${shadowed.group.entities[0].id}`); +delete shadowed.add; +shadowed.remove(shadowed.group.entities[0]); +results.push(`unshadow:${shadowed.group.entities.length}`); + +const extracted = stable.add; +let extractedThrows = false; +try { + extracted(entity(3)); +} catch (_error) { + extractedThrows = true; +} +const called = entity(4); +extracted.call(stable, called); +extracted.apply(stable, [entity(5)]); +const rebound = extracted.bind(stable); +rebound(entity(6)); +results.push(`binding:${extractedThrows}:${stable.group.entities.length}`); +stable.group.entities.length = 0; + +class Holder { + constructor(registry) { + this._registry = registry; + } + + get registry() { + return this._registry; + } +} + +const holder = new Holder(stable); +const accessed = entity(7); +holder.registry.add(accessed); +holder.registry.remove(accessed); +const proxy = new Proxy(stable, { + get(target, key, receiver) { + return Reflect.get(target, key, receiver); + }, +}); +proxy.add(entity(8)); +results.push(`accessor-proxy:${stable.group.entities[0].id}`); +stable.group.entities.length = 0; + +class DerivedRegistry extends ImportedRegistry { + add(value) { + super.add(value); + this.added = (this.added || 0) + 1; + } +} + +const derived = new DerivedRegistry(); +derived.add(entity(9)); +derived.remove(derived.group.entities[0]); +results.push(`inheritance:${derived.added}:${derived.group.entities.length}`); + +const mutation = new ImportedRegistry(); +const originalAdd = ImportedRegistry.prototype.add; +ImportedRegistry.prototype.add = function (value) { + this.group.entities.push(entity(value.id + 1000)); +}; +mutation.add(entity(10)); +delete ImportedRegistry.prototype.add; +let deletedThrows = false; +try { + mutation.add(entity(11)); +} catch (_error) { + deletedThrows = true; +} +Object.defineProperty(ImportedRegistry.prototype, 'add', { + value: originalAdd, + writable: true, + configurable: true, +}); +mutation.add(entity(12)); +results.push(`prototype:${mutation.group.entities[0].id}:${deletedThrows}:${mutation.group.entities[1].id}`); + +class PrivateCounter { + #count = 0; + + bump(shouldThrow) { + this.#count++; + if (shouldThrow) throw new Error('private-boom'); + return this.#count; + } + + read() { + return this.#count; + } +} + +const counter = new PrivateCounter(); +let exception = ''; +try { + counter.bump(true); +} catch (error) { + exception = error.message; +} +counter.bump(false); +results.push(`private-exception:${exception}:${counter.read()}`); + +for (let i = 0; i < 20_000; i++) { + const value = entity(i); + stable.add(value); + stable.remove(value); +} +results.push(`gc:${stable.group.entities.length}`); + +console.log(results.join('|'));