Skip to content

Commit c203c77

Browse files
proggeramlugRalph Küpper
andauthored
fix(runtime): fence the dense subclass seqlock; specialize imported this-methods and proven Symbols (#8737)
Lands #8735, #8729 and #8731. #8735 fixes a real data race in the dense-array subclass layout cache. `cached_dense_layout` read two payload fields `Relaxed` and then rechecked the sequence counter with an `Acquire` LOAD -- but an acquire load only constrains what follows it, so on weakly ordered hardware the preceding payload reads could sink past the recheck. A reader racing a colliding publisher could then combine one field from the old layout with one from the new and return a WRONG ELEMENT rather than faulting. A standalone `fence(Acquire)` now sits between the payload reads and the recheck, which is the canonical seqlock-reader form. #8729 lowers strict equality against a proven Symbol to raw NaN-boxed identity, keeping loose equality, reassigned locals and erased TypeScript annotation claims on the semantic runtime helpers. #8731 (fixes #8693) publishes producer-authoritative proven-`this` method capabilities through imports, aliases and re-exports, emitting guarded direct imported clone calls while retaining generic dispatch fallbacks. A changelog fragment was added for #8735, which had neither one nor a skip-changelog label. No version bump. Co-authored-by: Ralph Küpper <ralph@skelpo.com>
1 parent 0749bd3 commit c203c77

42 files changed

Lines changed: 1594 additions & 146 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

benchmarks/compiler_output/workloads.toml

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2076,6 +2076,7 @@ allowed_hot_loop_runtime_calls = [
20762076
"js_for_in_keys_stable_value",
20772077
"js_gc_loop_safepoint",
20782078
"js_in_operator",
2079+
"js_value_typeof",
20792080
]
20802081

20812082
[workloads.for_in_stable_keys.vectorization]
@@ -2092,6 +2093,8 @@ allowed_missed_reason_kinds = [
20922093
"unsupported_reduction",
20932094
]
20942095

2096+
[workloads.for_in_stable_keys.runtime_budgets]
2097+
20952098
[[workloads.for_in_stable_keys.stdout_checks]]
20962099
name = "for_in_stable_keys_checksum"
20972100
equals = "for_in_stable_keys:200000\n"
@@ -2114,3 +2117,40 @@ regex_none = [
21142117
"call i64 @js_object_get_own_property_names",
21152118
]
21162119
detail = "the stable arm does not directly allocate or rebuild generic key lists"
2120+
2121+
[workloads.issue_8693_imported_this]
2122+
source = "test-files/fixtures/issue_8693_imported_this/main.js"
2123+
kind = "imported_class_method_specialization"
2124+
allow_hot_loop_conversions = true
2125+
allow_dynamic_property_runtime = true
2126+
2127+
[workloads.issue_8693_imported_this.vectorization]
2128+
min_vectorized_loops = 0
2129+
scalar_baseline = "allowed: this fixture gates cross-module method dispatch, not loop vectorization"
2130+
allowed_missed_reason_kinds = [
2131+
"call_instruction",
2132+
"control_flow",
2133+
"generic_not_vectorized",
2134+
"not_beneficial",
2135+
"uncountable_loop",
2136+
"unknown_trip_count",
2137+
"unsupported_instruction",
2138+
"unsupported_reduction",
2139+
]
2140+
2141+
[workloads.issue_8693_imported_this.runtime_budgets]
2142+
2143+
[workloads.issue_8693_imported_this.native_rep_checks]
2144+
allow_materialization_reasons = ["runtime_api"]
2145+
2146+
[[workloads.issue_8693_imported_this.native_rep_checks.require_records]]
2147+
name = "imported_registry_proven_this_selection"
2148+
consumer = "proven_this_method_direct_call"
2149+
notes_contains = "receiver_provenance=imported_class_metadata"
2150+
min = 2
2151+
2152+
[[workloads.issue_8693_imported_this.native_rep_checks.require_records]]
2153+
name = "imported_registry_generic_fallback_retained"
2154+
consumer = "proven_this_method_direct_call"
2155+
notes_contains = "generic_dispatch_fallback=js_native_call_method_by_id"
2156+
min = 2
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
category: Performance
3+
title: Specialize imported methods that capture this
4+
---
5+
6+
ESM import and re-export metadata now carries producer-proven method
7+
eligibility, allowing guarded direct calls into stable class methods that use
8+
`this`. Generic dispatch remains available for shadowed or mutated receivers,
9+
including prototype replacement, deletion, and recreation.
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
Made strict equality against a proven `Symbol()` or `Symbol.for()` value use
2+
direct identity instead of the generic JavaScript equality helper. The proof
3+
comes only from a stable constructor initializer, never from an erased
4+
TypeScript `symbol` annotation; reassigned bindings and loose equality retain
5+
their semantic runtime paths.
6+
7+
This removes two generic `js_eq` calls from codehz/ecs's 10k-entity
8+
accumulation loop. On an Apple M1 Mac mini, 11 alternating process pairs
9+
measured the row at 0.195845 ms versus 0.615553 ms on the parent change, a
10+
68.193% median paired improvement with 11/11 wins. The full ECS suite remained
11+
7/7 with checksum 50005000, and a forced verified-GC Symbol stress run recorded
12+
87 copying minors and 11,470 copied objects with Node-identical output.
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fixed a data race in the dense-array subclass layout cache. `cached_dense_layout` read two payload fields with `Relaxed` ordering and then rechecked the sequence counter with an `Acquire` load — but an acquire *load* only constrains operations that follow it, so on weakly-ordered hardware the preceding payload reads could sink past the recheck. A reader racing a colliding publisher could then combine one field from the old layout with one from the new and return a wrong element value rather than faulting. A standalone `fence(Acquire)` now sits between the payload reads and the recheck, which is the canonical seqlock-reader form.

crates/perry-codegen/src/codegen/helpers.rs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -339,6 +339,25 @@ pub(super) fn inline_hot_small_size_cap() -> usize {
339339
})
340340
}
341341

342+
/// Give the representation-specialized method body the ordinary small-body
343+
/// inline bias. This lets producer-local chains such as `Registry.add ->
344+
/// Group.pushEntity` optimize through the second method boundary while the
345+
/// externally linked clone remains callable by importers.
346+
pub(super) fn apply_pshape_inline_policy(
347+
lf: &mut crate::function::LlFunction,
348+
method: &perry_hir::Function,
349+
is_pshape_clone: bool,
350+
) {
351+
if !is_pshape_clone || method.is_async || method.is_generator || method.was_plain_async {
352+
return;
353+
}
354+
if method.body.len() <= 8 {
355+
lf.force_inline = true;
356+
} else if inline_hot_small_enabled() && method.body.len() <= inline_hot_small_size_cap() {
357+
lf.inline_hint = true;
358+
}
359+
}
360+
342361
/// Maximum total (module-wide) direct call sites a function may have and still
343362
/// be hinted. This is the anti-bloat backstop: the raised `-inlinehint-threshold`
344363
/// lifts LLVM's ceiling for a hinted callee at *every* one of its call sites, so

crates/perry-codegen/src/codegen/method.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -306,10 +306,18 @@ pub(super) fn compile_method(
306306
let ic_base = llmod.ic_counter;
307307
let buffer_alias_base = llmod.buffer_alias_counter;
308308
let lf = llmod.define_function(&llvm_name, DOUBLE, params);
309-
if is_pshape_clone || is_index_clone || typed_public_trampoline.is_some() || force_generic_body
309+
// Plain `$pshape` clones are producer-published capabilities and need
310+
// external linkage for guarded calls from importing modules. The stricter
311+
// array-cache clone remains module-local: only containment-proven locals
312+
// in this module may select it.
313+
if ptr_array_cache_clone
314+
|| is_index_clone
315+
|| typed_public_trampoline.is_some()
316+
|| force_generic_body
310317
{
311318
lf.linkage = "internal".to_string();
312319
}
320+
super::helpers::apply_pshape_inline_policy(lf, method, is_pshape_clone);
313321
if is_index_clone {
314322
lf.pre_statepoint_inline = true;
315323
}

crates/perry-codegen/src/codegen/method_registry.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,10 @@ pub(crate) fn build_method_names(
216216
let param_types: Vec<crate::types::LlvmType> =
217217
std::iter::repeat_n(DOUBLE, arity).collect();
218218
llmod.declare_function(&llvm_fn, DOUBLE, &param_types);
219+
if ic.proven_this_method_names.contains(method_name) {
220+
let clone = crate::collectors::pshape_method_name(&llvm_fn);
221+
llmod.declare_function(&clone, DOUBLE, &param_types);
222+
}
219223
}
220224

221225
// Cross-module getters. The dispatch site at

crates/perry-codegen/src/codegen/mod.rs

Lines changed: 54 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1808,17 +1808,6 @@ pub fn compile_module(hir: &HirModule, opts: CompileOptions) -> Result<Vec<u8>>
18081808
receiver_class_table,
18091809
&module_dispatch_facts,
18101810
) {
1811-
// #7142: the tower routing site emits its own inline shape
1812-
// re-check, so it only takes the clone where the clone deletes
1813-
// strictly more guarded field sites than that check costs. The
1814-
// other two sites are guard-dominated and route unconditionally.
1815-
if crate::collectors::pshape_tower_route_profitable(
1816-
class,
1817-
method,
1818-
receiver_class_table,
1819-
) {
1820-
pshape_tower_routable.insert((class.name.clone(), method.name.clone()));
1821-
}
18221811
pshape_methods.insert((class.name.clone(), method.name.clone()), fact);
18231812
}
18241813
match typed_abi::typed_f64_method_rejection_reason(method) {
@@ -1933,6 +1922,60 @@ pub fn compile_module(hir: &HirModule, opts: CompileOptions) -> Result<Vec<u8>>
19331922
}
19341923
}
19351924
}
1925+
// #7142: the tower routing site emits its own inline shape re-check, so it
1926+
// only takes a clone where that clone deletes strictly more guarded work
1927+
// than the check costs. Price these routes after all local clone facts are
1928+
// known so nested `this.other()` calls can count an inherited clone too.
1929+
let local_pshape_methods: std::collections::HashSet<(String, String)> =
1930+
pshape_methods.keys().cloned().collect();
1931+
for class in &hir.classes {
1932+
for method in &class.methods {
1933+
let key = (class.name.clone(), method.name.clone());
1934+
if local_pshape_methods.contains(&key)
1935+
&& crate::collectors::pshape_tower_route_profitable(
1936+
class,
1937+
method,
1938+
receiver_class_table,
1939+
&local_pshape_methods,
1940+
)
1941+
{
1942+
pshape_tower_routable.insert(key);
1943+
}
1944+
}
1945+
}
1946+
// Imported classes publish only clone names the defining module proved and
1947+
// emitted. Installing those capabilities in the same registries lets both
1948+
// the ordinary exact-class/shape guarded arm and profitable adapter-field
1949+
// dispatch towers retain the receiver proof across ESM and npm boundaries.
1950+
// The tower subset is producer-authored because only the defining module
1951+
// can see enough of the body to price its additional keys-token check.
1952+
for imported in &opts.imported_classes {
1953+
let effective_name = imported
1954+
.local_alias
1955+
.as_deref()
1956+
.unwrap_or(&imported.name)
1957+
.to_string();
1958+
if hir.classes.iter().any(|class| class.name == effective_name) {
1959+
continue;
1960+
}
1961+
for method in &imported.proven_this_method_names {
1962+
if !imported.method_names.contains(method) {
1963+
continue;
1964+
}
1965+
pshape_methods.insert(
1966+
(effective_name.clone(), method.clone()),
1967+
crate::collectors::PtrShapeLocal {
1968+
class_name: effective_name.clone(),
1969+
numeric_fields: std::collections::HashSet::new(),
1970+
report_name: crate::opt_report::enabled()
1971+
.then(|| format!("imported:{}", imported.source_prefix)),
1972+
},
1973+
);
1974+
if imported.proven_this_tower_method_names.contains(method) {
1975+
pshape_tower_routable.insert((effective_name.clone(), method.clone()));
1976+
}
1977+
}
1978+
}
19361979
let mut compiler_private_async_i32_control_locals = std::collections::HashSet::new();
19371980
let mut compiler_private_async_i1_control_locals = std::collections::HashSet::new();
19381981
crate::boxed_vars::collect_compiler_private_async_control_locals_in_stmts(

crates/perry-codegen/src/codegen/module_globals_emit.rs

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ fn module_global_runtime_type(
4646
Expr::String(_) | Expr::WtfString(_) | Expr::I18nString { .. } | Expr::TypeOf(_) => {
4747
Some(Type::String)
4848
}
49+
Expr::SymbolNew(_) | Expr::SymbolFor(_) => Some(Type::Symbol),
4950
// Compiler-owned allocation HIR establishes these runtime classes
5051
// independently of the erased binding annotation. Keep module-global
5152
// facts aligned with `proven_type_from_init`; otherwise a value that
@@ -75,6 +76,39 @@ fn module_global_runtime_type(
7576
}
7677
}
7778

79+
#[cfg(test)]
80+
mod runtime_type_tests {
81+
use super::module_global_runtime_type;
82+
use perry_hir::types::Type;
83+
use perry_hir::Expr;
84+
85+
#[test]
86+
fn symbol_constructors_are_module_global_runtime_proofs() {
87+
assert_eq!(
88+
module_global_runtime_type(&Expr::SymbolNew(None), true),
89+
Some(Type::Symbol)
90+
);
91+
assert_eq!(
92+
module_global_runtime_type(
93+
&Expr::SymbolFor(Box::new(Expr::String("shared".to_string()))),
94+
true,
95+
),
96+
Some(Type::Symbol)
97+
);
98+
}
99+
100+
#[test]
101+
fn an_object_initializer_cannot_inherit_a_symbol_annotation_as_proof() {
102+
assert_eq!(
103+
module_global_runtime_type(
104+
&Expr::Object(vec![("x".to_string(), Expr::Number(1.0))]),
105+
true,
106+
),
107+
None
108+
);
109+
}
110+
}
111+
78112
fn module_shadows_shared_array_buffer_intrinsic(
79113
hir: &HirModule,
80114
imported_classes: &[ImportedClass],

crates/perry-codegen/src/codegen/opts.rs

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -510,6 +510,16 @@ pub struct ImportedClass {
510510
pub has_instance_fields: bool,
511511
/// Method names defined on this class.
512512
pub method_names: Vec<String>,
513+
/// Own methods for which the defining module emitted an externally
514+
/// callable, guarded proven-`this` clone. Consumers may reference only
515+
/// names in this producer-authored capability set; absence is fail-closed
516+
/// and keeps the public method body on the direct arm.
517+
pub proven_this_method_names: Vec<String>,
518+
/// Subset of `proven_this_method_names` for which the producer also proved
519+
/// that the extra exact-keys check paid by a class-id dispatch-tower arm is
520+
/// profitable. This keeps adapter-field calls fail-closed without asking
521+
/// an importer that cannot see the method body to repeat the decision.
522+
pub proven_this_tower_method_names: Vec<String>,
513523
/// Declared return types parallel to `method_names`. Imported class stubs
514524
/// retain these so a call such as `factory.make().run()` can recover the
515525
/// returned receiver class without value-importing that class directly.
@@ -911,12 +921,11 @@ pub(crate) struct CrossModuleCtx {
911921
/// exported.
912922
pub nonnegative_index_methods: std::collections::HashMap<(String, String), Vec<u32>>,
913923
/// Representation-selection Phase 5a: `(class, method)` pairs that have a
914-
/// generated `internal` proven-`this` clone
915-
/// (`collectors/proven_this.rs`). Keys are OWN declarations of
916-
/// module-local classes only, which is precisely the condition the two
917-
/// routing sites rely on: a hit means the receiver's proven exact class is
918-
/// the class the clone was compiled for, so `this` cannot be a subclass
919-
/// instance with a different chain.
924+
/// generated proven-`this` clone (`collectors/proven_this.rs`). Local keys
925+
/// come from body analysis; imported keys come from an explicit capability
926+
/// published by the defining module. Both represent OWN declarations, so a
927+
/// hit means the receiver's proven exact class is the class the clone was
928+
/// compiled for and `this` cannot have a different subclass chain.
920929
pub pshape_methods:
921930
std::collections::HashMap<(String, String), crate::collectors::PtrShapeLocal>,
922931
/// #7142: the subset of [`Self::pshape_methods`] whose clone the class-id

0 commit comments

Comments
 (0)