Skip to content

perf: shapes 3x instruction-count regression (also cycles, deeplist) between 73df2a6e7 and 905017b1c #8648

Closed

Description

@proggeramlug

Three sweep-corpus benchmarks regressed between 73df2a6e7 and 905017b1c (the nine commits landed on 2026-08-23). shapes is 3× the instruction count.

Evidence

Reproduced on two machines with two different methods, so this is not bench-host noise.

Bench mini, perry/node ratio (same scriptc/porffor/node pins, two independent runs):

bench baseline 73df2a6e7 run 1 run 2
shapes 0.931x 2.917x 2.912x
cycles 0.842x 1.331x 1.363x
deeplist 0.474x 0.569x 0.580x
pipeline 0.919x 1.114x 1.083x

The other 16 rows sit on baseline in both runs. Both runs were flagged DIRTY on load, but contamination picks different victims each time — these reproduce to within ~2%, so the load flag is not the explanation.

Local Mac, wall clock, best-of-3 on the compiled binaries directly:

shapes     old=0.09s  new=0.38s   4.22x SLOWER
cycles     old=0.07s  new=0.12s   1.71x SLOWER
deeplist   old=0.05s  new=0.06s   1.20x SLOWER
fib40      old=0.65s  new=0.52s   0.80x (faster)
tree       old=0.30s  new=0.28s   0.93x (fine)

It is extra mutator work, not GC and not scheduling (/usr/bin/time -l, p_shapes):

old new
instructions retired 1,186,944,293 3,667,840,774 3.09x
cycles elapsed 319,225,659 970,790,835 3.04x
max RSS 33.5 MB 46.0 MB 1.37x
GC diag lines (PERRY_GC_DIAG=1) 26 26 identical

Same instruction/cycle ratio either side, identical collection count — the program simply executes ~2.5 billion more instructions. That is the shape of a per-operation linear scan being added to a hot path.

What it is not

Leading suspect

shapes is by far the most class-heavy row in the corpus (7 classes, inheritance, super() chains, static members, getters, instanceof, virtual dispatch), and severity tracks class complexity rather than mere class presence — tree, tree_wide and push_cls each use one class and are unaffected.

That points at #8630 (class semantics tail, landed 905017b1c). Its receiver_has_own_key guard added to set_field_by_name_object_tail is a keys-array scan on the by-name store path, which is exactly a per-operation linear scan. The #8630 author already noticed the cost and moved own_key_present to be evaluated last and only on the slow path — the question is whether class-instance stores (this.x = x in a constructor, run 120,000 times here) still reach it.

Candidate window, most to least likely: #8630 (class semantics) > #8638 (node:test) > #8632/#8637 (HTTP) > #8633/#8623 (spill, largely excluded above).

Not yet done

A proper bisect. I built 00bddb34b (immediately pre-#8630) to A/B it, but the build was interrupted and the host is at load ~45, so I stopped rather than produce a measurement I could not trust. The decisive next step is a perry built at 00bddb34b timed against 905017b1c on shapes.

Repro:

# old binary is preserved in the sweep artifacts
./sweep-artifacts-0822e/perry/p_shapes     # 73df2a6e7
./sweep-artifacts-0823b/perry/p_shapes     # 905017b1c
/usr/bin/time -l ./…/p_shapes 2>&1 | grep -E "instructions retired|cycles elapsed"

Activity

  1. proggeramlug commented on Aug 23, 2026

    @proggeramlug
    ContributorAuthor

    Root cause found: derived constructors (class B extends A), attributed to #8630

    Isolated with a 2x2 compiler/runtime cross and three micro-benchmarks.

    Attribution — it is #8630, and it is codegen

    Same sources, only the compiler differs (00bddb34b = immediately pre-#8630, 905017b1c = post):

    bench pre-#8630 post-#8630
    shapes 1,159,123,398 3,636,602,206 3.14x
    cycles 1,301,925,013 2,197,271,746 1.69x
    deeplist 750,814,273 966,403,862 1.29x
    tree 4,027,714,831 4,027,701,640 1.00x (control)

    Swapping only the runtime half (pre-compiler + post-runtime) gives 1.07x, so ~all of it is codegen.

    The trigger is inheritance, not field stores

    Three micro-benchmarks, 3M iterations each:

    micro pre post
    plain class, fields from params 381,756,402 381,924,476 1.00x
    field initialized to a null literal 190,435,277 190,017,104 1.00x
    class B extends A + super() 998,471,071 1,892,012,693 1.89x

    So a non-derived constructor is unaffected; a derived one costs ~1.9x.

    What the derived constructor emits now

    micro_inherit's B_constructor, 167 -> 285 IR lines. Calls per construction:

    pre-#8630 post-#8630
    2x js_put_value_set 2x js_typed_feedback_class_field_set_guard
    2x js_class_field_set_fallback
    2x js_throw_reference_error_this_before_super
    2x js_ctor_return_override
    1x js_derived_super_scope_push + 1x js_derived_super_scope_pop

    Two calls became ten. shapes is worst because it has six extends and constructs 120,000 objects.

    Note this also demotes the base class: Node2D extends nothing, yet its constructor shows the same pattern — consistent with the pre-existing rule documented in typed_shape.rs that a class with any subclass loses its at-allocation layout declaration for the whole chain. Pre-#8630 that fallback was one js_put_value_set; now it is guard + fallback.

    Ruled out

    Concrete fix point

    crates/perry-codegen/src/codegen/method.rs:735 emits the shared super scope for every derived constructor, gated only on "has extends":

    if class.extends.is_some() || class.extends_name.is_some()
        || class.native_extends.is_some() || class.extends_expr.is_some() {
        push_shared_super_called_slot(&mut ctx);      // emits js_derived_super_scope_push/pop
        ctx.shared_super_scope_active = true;
    }

    But push_shared_super_called_slot's own doc states its purpose: "Enter a constructor scope whose binding must also be visible to arrows compiled as separate LLVM functions." A derived constructor with no nested arrow referencing the super binding only needs push_super_called_slot — the pure-alloca form, no runtime call. Gating on that should remove the push/pop pair.

    That is the cleanest of the four costs, but on its own it is only part of the 1.9x (a TLS + RefCell + Vec push/pop is tens of instructions against a ~894M delta over 3M iterations). The other three want the same treatment:

    • js_throw_reference_error_this_before_super x2 — elidable when this is provably not read before super() returns;
    • js_ctor_return_override x2 — elidable when the constructor has no explicit return;
    • the field store falling to guard + fallback rather than js_put_value_set.

    All four are elisions of machinery that is unnecessary in the common shape, so none should require touching the semantics #8630 added.

  2. proggeramlug commented on Aug 23, 2026

    @proggeramlug
    ContributorAuthor

    Partial fix on fix/8648-derived-ctor, plus what has been ruled out

    Fixed: the runtime super scope for closure-free derived constructors

    codegen/method.rs emitted the shared derived-super scope for every derived
    constructor, gated only on "has extends". The shared form's runtime calls
    (js_derived_super_scope_push/pop) maintain a thread-local stack whose only
    readers are js_derived_super_bind_current / js_derived_this_check_current —
    the lookups an arrow needs when it compiles as its own LLVM function and cannot
    name the constructor's i1 alloca. With no closure in the body, nothing can
    perform that lookup and bind_derived_this_after_super reads the alloca
    directly, so the pair was a thread-local round trip per construction for a cell
    no one could reach.

    Now gated on body_contains_closure. Conservative: any closure keeps the shared
    form, without asking whether it mentions this or super.

    Measured (instructions retired, vs the pre-#8630 compiler at 00bddb34b):

    bench recovered
    micro_inherit (1.89x -> 1.67x) 27%
    deeplist 6%
    cycles 2%
    shapes 1%

    perry-codegen 1189 passed / 0 failed; all ratchets + fmt green.

    Ruled out — each by measurement, recorded so nobody repeats them

    What remains

    The dominant cost is the constructor field store moving from one
    js_put_value_set to js_typed_feedback_class_field_set_guard +
    js_class_field_set_fallback. typed_shape.rs documents 528,000 by-name field
    stores per shapes.ts run, which is the right order of magnitude for the
    ~2.5B instruction delta.

    I did not isolate why #8630 flipped that eligibility. The selection lives in
    a deeply-nested arm of expr/property_set.rs::lower (~line 1250-1405), and
    property_set.rs itself is unchanged by #8630 — so the trigger is an input to
    it (class metadata built during lowering), not the emitter. That is the next
    thread to pull.

  3. proggeramlug commented on Aug 23, 2026

    @proggeramlug
    ContributorAuthor

    Partial fix landed on main in e5b87e1 via #8649 — the closure-free derived-constructor super-scope elision (27% of the micro regression, 1% on shapes).

    Still open: the dominant cost, the constructor field store moving from js_put_value_set to guard + fallback. The revert bisect over #8630's codegen files is running (lower_call/new.rs first, then field_init.rs, then method.rs).

  4. proggeramlug commented on Aug 23, 2026

    @proggeramlug
    ContributorAuthor

    94% of the regression is fixed — landed on main in c2da034 via #8653.

    Root cause, found by revert-bisect over #8630's codegen files: reverting only lower_call/field_init.rs recovered 94.6%, while lower_call/new.rs moved nothing (0.8%). #8630 replaced field-initializer lowering with an unconditional js_class_field_add — a full [[DefineOwnProperty]] behind a handle scope, per field per construction. A bare x: number; still gets a synthesized undefined initializer, so shapes.ts paid it ~2M times.

    shapes: 3,662,616,604 -> 1,320,393,329 instructions against a 1,179,031,124 baseline (3.11x -> 1.12x).

    The fix takes the fast path only where CreateDataProperty and a plain store provably agree — no accessor on the chain, and no constructor able to return a replacement this. #8630's spec fix is preserved and verified against Node: the inherited-setter case prints d.v=5 as Node does, where the pre-#8630 compiler printed SETTER RAN / undefined.

    Remaining, keeping this issue open: cycles (1.67x), deeplist (1.27x) and a two-class new B(x, y) loop (1.87x) are essentially unmoved, so there is a second independent cause. The nine ruled-out hypotheses above still apply to it.

  5. proggeramlug commented on Aug 23, 2026

    @proggeramlug
    ContributorAuthor

    Reopening: this was auto-closed by the #8653 merge, but only the shapes half is fixed.

    Remaining, and it is a distinct cause: cycles 1.67x, deeplist 1.27x, and a two-class new B(x, y) loop 1.87x are essentially unmoved by both #8649 and #8653 (2%, 6%, 2% respectively). Reverting field_init.rs — which recovered 94% of shapes — moved micro_inherit by 0.8%, so the two regressions are independent.

    What is already excluded for this second cause (all measured, see the earlier comment): GC, dispatch speculation, the root-spill threshold, the class-field inline guard, typed feedback, inheritance depth, null-literal initializers, and non-derived constructors. js_ctor_return_override and the this-before-super TDZ check are also excluded — both sit behind inline branches and do not execute on the common path.

    Next step: continue the revert-bisect over #8630's remaining codegen files against micro_inherit — codegen/method.rs (+106) and expr/this_super_call.rs (+268) are the untested ones.

  6. proggeramlug commented on Aug 23, 2026

    @proggeramlug
    ContributorAuthor

    Closed by #8664 — the second cause is fixed.

    #8630's standalone_ctor_return hunk made every <Class>_constructor symbol return this instead of undefined, which flipped new.rs's is_undef-guarded js_ctor_return_override diamond from never-taken to always-taken. That guarded call probes the typed-array, buffer and Proxy registries plus is_arguments_object and walks GC forwarding chains, per construction.

    two-class new B(x,y): 1.65x → 1.01x. cycles: 1.68x → 1.02x.

    Two corrections to my earlier analysis in this issue, both worth recording:

    1. My "derived constructor" framing was wrong. cycles.ts has no extends at all. What decides who pays is lower_call/ctor_prologue_stores.rs: a constructor body that is purely this.f = param stores skips the call entirely; one literal initializer or a super() disqualifies the plan.
    2. My exclusion list was wrong on js_ctor_return_override. I wrote that it sits behind an inline branch and does not execute. The branch is inline — fix(runtime): complete class semantics tail #8630 changed the value it tests. It was the one exclusion that mattered.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions