Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 92 additions & 0 deletions changelog.d/7169-tower-pshape-keys-guard.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
### perf(codegen): route the class-id dispatch tower to the proven-`this` clone behind an inline keys check (#7142)

`benchmarks/app-patterns/kernels/batch.ts` — the workload `Ptr<Shape>` exists
for — consumed nothing from representation-selection Phase 5a. Its hot call is
`r.rescore(1.5)` inside `rows.map((r) => …)`, whose closure parameter has no
static class, so `receiver_class_name()` returns `None`, neither of the two
routing sites #7141 fixed is ever reached, and the call lowers to the class-id
switch tower in `lower_call/property_get/dynamic_dispatch.rs`.

#7141 refused to route that tower because a `class_id` match is not a layout
proof: `delete inst.f` compacts the packed inline slots while **preserving**
`class_id` (`object/delete_rest.rs`), so on `class Row { a; b; c }`,
`delete row.b` moves `c` from slot 2 into slot 1 and the clone's bare
fixed-offset loads would read the wrong slot.

The compaction installs a freshly **cloned** keys array, so an inline pointer
compare against the class's `@perry_class_keys_*` token catches it exactly.
That check is now emitted at `idispatch.caseN` — one basic block, 21 IR
instructions, no calls: four loads off the receiver (three from header words the
tower's own `js_object_get_class_id` already touched), one entry-hoisted token
reload, nine ALU ops, one branch. The dereference needs no gate/deref split
because a non-zero class-id match already rejects the handle band, the
Set/Map/RegExp registries, out-of-heap addresses and non-`GC_TYPE_OBJECT`
allocations.

The check is **dynamic on purpose**. The `delete` shape barrier that stands the
whole analysis down is collected per module while receivers alias across modules
(#7143), so at this site a static proof would be the wrong instrument — which is
also why the two pre-existing routing sites were safe and this one was not.

Beyond the keys token the check carries the sticky
`@PERRY_CLASS_FIELD_INLINE_GUARD_DISABLED` latch (prototype-level descriptors,
tracing mode), the per-object `OBJ_FLAG_HAS_DESCRIPTORS` bit (instance-level
installs deliberately do not flip the process-global latch, #5654),
`OBJ_FLAG_FROZEN` (a proven-`this` clone may contain field writes, and Phase 5a
rules a frozen receiver out only through a module-scoped kill — this makes the
tower route strictly stronger than `js_method_direct_shape_guard`), plus
not-forwarded and `object_type == OBJECT_TYPE_REGULAR`. Routing is restricted to
cases whose receiver class **declares** the method, so the clone's `this` is
exactly the class it was compiled for.

Unlike the two guard-dominated sites, this one pays for its own proof, so it
consults a profitability gate (`collectors/repsel_benefit.rs`, the module #7132
added) instead of routing unconditionally: the re-check is one instance of the
same header check the public body runs at every `this.field`, so the trade is
*N in-body checks for 1 at the call site* and the break-even is exactly `N == 1`.
It routes at `N >= 2`. The rule is a count with no target-specific term.

Measured on `batch.ts` (perry-dev, darwin-arm64), the two bodies the tower now
chooses between:

| body | IR lines | class-field guards | by-name field ops | total `js_*` call sites |
|---|---|---|---|---|
| `…__Row__rescore` (public) | 352 | 3 | 3 | 15 |
| `…__Row__rescore__pshape` | 189 | 0 | 0 | 6 |

`…__rescore__pshape` goes from **0 call sites to 1** — the module-wide static
totals are deliberately unchanged, because the public body survives as the miss
arm and as the registered vtable symbol, which is why the A/B is reported at the
call site rather than as a module count.

Timed on a quiet Raspberry Pi 5 with `perf stat`, two compilers from the same
tree (arm `before` has the tower route forced off, nothing else), ASLR disabled,
pinned to one core, interleaved, 12 reps each. The workload is bimodal at ~1%
independently of the arm (3/12 runs per arm in the low mode; zero GC collections
in both, so it is not a GC-schedule effect), so the delta is reported per mode:
**−0.156%** in the low mode and **−0.168%** in the high mode, overall median
−0.17%. That is ≈6.0M instructions over 40,000 `rescore` calls, ≈150
instructions per call; the whole-program figure is small because `rescore` is a
small share of `batch.ts`, which is dominated by allocation, `sort` and
`reduce`. The fixture is there because its *receiver shape* is the hard case,
not because its method is the hot spot.

`test-files/test_gap_repsel_pshape_tower_delete.ts` is the soundness test, built
red-first: construct → `delete` a field through a cross-module alias → call the
method. Against a class-id-only route it prints `after: 103,NaN,309,412` where
Node 26.5.1 prints `103,206,309,412`; with the keys check the whole file is
byte-identical to the oracle. Registered in `test-parity/gc_repsel_corpus.txt`
and GC-live by construction (`copied_objects=5971` at `--pressure 8`), so the
evacuating arms have a receiver to move rather than reporting the file inert.

Three call-site ratchets in `collectors/proven_this_routing_tests.rs` with
disjoint red sets, verified by building each sabotage arm: reverting the routing
reddens only the routing test, removing the keys compare reddens only the
keys-guard test (which traces the token global → entry slot → guard-block reload
→ `icmp eq i64` → the branch into the clone's block), and removing the
profitability gate reddens only the refusal test.

Unchanged `(double this, args…)` ABI; the clone still stores its receiver to a
slot and `js_shadow_slot_bind`s it with no safepoint between (#6925/#6990 —
`GC_TYPE_OBJECT` is movable in the shipped configuration, so the `TaPtr` no-bind
shortcut does not transfer).
1 change: 1 addition & 0 deletions crates/perry-codegen/src/codegen/closure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1000,6 +1000,7 @@ pub(super) fn compile_closure(
typed_i1_function_param_reps: &cross_module.typed_i1_function_param_reps,
typed_f64_methods: &cross_module.typed_f64_methods,
pshape_methods: &cross_module.pshape_methods,
pshape_tower_routable: &cross_module.pshape_tower_routable,
proven_this: None,
typed_i32_methods: &cross_module.typed_i32_methods,
typed_i1_methods: &cross_module.typed_i1_methods,
Expand Down
2 changes: 2 additions & 0 deletions crates/perry-codegen/src/codegen/entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -862,6 +862,7 @@ pub(super) fn compile_module_entry(
typed_i1_function_param_reps: &cross_module.typed_i1_function_param_reps,
typed_f64_methods: &cross_module.typed_f64_methods,
pshape_methods: &cross_module.pshape_methods,
pshape_tower_routable: &cross_module.pshape_tower_routable,
proven_this: None,
typed_i32_methods: &cross_module.typed_i32_methods,
typed_i1_methods: &cross_module.typed_i1_methods,
Expand Down Expand Up @@ -1523,6 +1524,7 @@ pub(super) fn compile_module_entry(
typed_i1_function_param_reps: &cross_module.typed_i1_function_param_reps,
typed_f64_methods: &cross_module.typed_f64_methods,
pshape_methods: &cross_module.pshape_methods,
pshape_tower_routable: &cross_module.pshape_tower_routable,
proven_this: None,
typed_i32_methods: &cross_module.typed_i32_methods,
typed_i1_methods: &cross_module.typed_i1_methods,
Expand Down
1 change: 1 addition & 0 deletions crates/perry-codegen/src/codegen/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -835,6 +835,7 @@ pub(super) fn compile_function(
typed_i1_function_param_reps: &cross_module.typed_i1_function_param_reps,
typed_f64_methods: &cross_module.typed_f64_methods,
pshape_methods: &cross_module.pshape_methods,
pshape_tower_routable: &cross_module.pshape_tower_routable,
proven_this: None,
typed_i32_methods: &cross_module.typed_i32_methods,
typed_i1_methods: &cross_module.typed_i1_methods,
Expand Down
2 changes: 2 additions & 0 deletions crates/perry-codegen/src/codegen/method.rs
Original file line number Diff line number Diff line change
Expand Up @@ -577,6 +577,7 @@ pub(super) fn compile_method(
typed_i1_function_param_reps: &cross_module.typed_i1_function_param_reps,
typed_f64_methods: &cross_module.typed_f64_methods,
pshape_methods: &cross_module.pshape_methods,
pshape_tower_routable: &cross_module.pshape_tower_routable,
proven_this,
typed_i32_methods: &cross_module.typed_i32_methods,
typed_i1_methods: &cross_module.typed_i1_methods,
Expand Down Expand Up @@ -1632,6 +1633,7 @@ pub(super) fn compile_static_method(
typed_i1_function_param_reps: &cross_module.typed_i1_function_param_reps,
typed_f64_methods: &cross_module.typed_f64_methods,
pshape_methods: &cross_module.pshape_methods,
pshape_tower_routable: &cross_module.pshape_tower_routable,
proven_this: None,
typed_i32_methods: &cross_module.typed_i32_methods,
typed_i1_methods: &cross_module.typed_i1_methods,
Expand Down
15 changes: 15 additions & 0 deletions crates/perry-codegen/src/codegen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1388,6 +1388,9 @@ pub fn compile_module(hir: &HirModule, opts: CompileOptions) -> Result<Vec<u8>>
(String, String),
crate::collectors::PtrShapeLocal,
> = std::collections::HashMap::new();
// #7142: the profitability subset the class-id dispatch tower may route to.
let mut pshape_tower_routable: std::collections::HashSet<(String, String)> =
std::collections::HashSet::new();
// Phase 3b typed-receiver widening: chain-global field indexes need the
// full class table — and it must be the SAME table dynamic dispatch's
// call-site gating consults (`class_table`, incl. class-expression
Expand All @@ -1410,6 +1413,17 @@ pub fn compile_module(hir: &HirModule, opts: CompileOptions) -> Result<Vec<u8>>
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) {
Expand Down Expand Up @@ -1708,6 +1722,7 @@ pub fn compile_module(hir: &HirModule, opts: CompileOptions) -> Result<Vec<u8>>
typed_i1_method_param_reps,
typed_f64_receiver_methods,
pshape_methods,
pshape_tower_routable,
typed_f64_closures: std::collections::HashSet::new(),
typed_i32_closures: std::collections::HashSet::new(),
typed_i1_closures: std::collections::HashSet::new(),
Expand Down
7 changes: 7 additions & 0 deletions crates/perry-codegen/src/codegen/opts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -806,6 +806,13 @@ pub(crate) struct CrossModuleCtx {
/// instance with a different chain.
pub pshape_methods:
std::collections::HashMap<(String, String), crate::collectors::PtrShapeLocal>,
/// #7142: the subset of [`Self::pshape_methods`] whose clone the class-id
/// dispatch tower may route to. The other two routing sites are dominated by
/// a shape guard they pay regardless, so the clone is free for them; the
/// tower has to emit its own inline shape re-check, so it only routes where
/// the clone deletes strictly more work than that check costs
/// (`collectors/repsel_benefit.rs`).
pub pshape_tower_routable: std::collections::HashSet<(String, String)>,
/// Inline closure bodies that have a generated internal typed-f64 clone.
/// Only statically-known local closure calls may select these clones after
/// closure identity/arity and numeric argument guards pass.
Expand Down
5 changes: 4 additions & 1 deletion crates/perry-codegen/src/collectors/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,10 @@ pub(crate) use integer_locals::{
pub(crate) use local_refs::{expr_contains_local_get, mark_all_candidate_refs_in_expr};
pub(crate) use mutation::has_any_mutation;
pub(crate) use pointer_locals::collect_pointer_typed_locals;
pub(crate) use proven_this::{method_proven_this, prune_colliding_clones, pshape_method_name};
pub(crate) use proven_this::{
method_proven_this, prune_colliding_clones, pshape_method_name,
tower_route_profitable as pshape_tower_route_profitable,
};
pub(crate) use ptr_numarray::{NumArrayDensity, NumArrayLocal};
pub(crate) use ptr_shape::{ptr_shape_locals_enabled, PtrShapeLocal};
pub(crate) use refs::{
Expand Down
24 changes: 24 additions & 0 deletions crates/perry-codegen/src/collectors/proven_this.rs
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,30 @@ pub(crate) fn method_proven_this(
})
}

/// #7142 profitability: should a class-id dispatch-tower case route to
/// `method`'s `{public}__pshape` clone?
///
/// Only meaningful once [`method_proven_this`] has admitted a clone — this adds
/// the "should we?" half that the admission test (a pure "may we?" conjunction)
/// deliberately does not carry. Refusing is always sound: the tower case keeps
/// calling the public body.
///
/// The two other routing sites do NOT consult this. They are dominated by a
/// shape guard that is paid whether or not the clone is taken, so for them the
/// clone is free; only the tower pays for its own proof.
pub(crate) fn tower_route_profitable(
class: &Class,
method: &Function,
classes: &HashMap<String, &Class>,
) -> 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)
}

/// Does the method body READ `this.<declared chain field>` anywhere?
fn method_reads_chain_field(method: &Function, fields: &HashSet<String>) -> bool {
let mut found = false;
Expand Down
Loading
Loading