From 9e9638cd2e618c988286118a57655cf94829e9af Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Wed, 21 Oct 2026 10:59:07 +0000 Subject: [PATCH 1/2] perf(semantic): compute a feature's redefinition closure once per model AllRedefinedFeatures rebuilt its visited set and closure on every call, and the implicit parameter redefinitions behind it rebuilt the owner's owned-parameter list on every query although parametersOf memoizes that list. An execution root frame asks for the closure of every feature it holds, so each frame paid for both. The closure is now a journaled per-symbol table like the other semantic memos, and the position lookup reads the memoized list. Co-Authored-By: jason.han --- ...icit-parameter-position-from-memo.fixed.md | 1 + .../semantics/implicit_parameter_memo_test.go | 28 +++++++++++++++++++ internal/semantic/semantics/masking.go | 6 ++++ internal/semantic/semantics/model.go | 2 ++ internal/semantic/semantics/redefinition.go | 3 +- 5 files changed, 39 insertions(+), 1 deletion(-) create mode 100644 changes/unreleased/implicit-parameter-position-from-memo.fixed.md create mode 100644 internal/semantic/semantics/implicit_parameter_memo_test.go diff --git a/changes/unreleased/implicit-parameter-position-from-memo.fixed.md b/changes/unreleased/implicit-parameter-position-from-memo.fixed.md new file mode 100644 index 0000000000..58862a2ee5 --- /dev/null +++ b/changes/unreleased/implicit-parameter-position-from-memo.fixed.md @@ -0,0 +1 @@ +- The features a feature redefines, directly or through them, are computed once per model, and a parameter's implicit redefinitions take its position from its owner's memoized parameter list instead of rebuilding it on every query; same redefinitions, fewer allocations per query — and per execution root frame, which asks for every feature it holds. diff --git a/internal/semantic/semantics/implicit_parameter_memo_test.go b/internal/semantic/semantics/implicit_parameter_memo_test.go new file mode 100644 index 0000000000..13ed5a4f51 --- /dev/null +++ b/internal/semantic/semantics/implicit_parameter_memo_test.go @@ -0,0 +1,28 @@ +package semantics + +import "testing" + +// The redefinition closure of a feature is computed once per model, and a +// parameter's implicit redefinitions take its position from the owner's +// memoized parameter list instead of rebuilding it: repeated queries allocate +// nothing beyond what the owner's implied generals cost. +func TestRedefinedFeaturesAreComputedOncePerSymbol(t *testing.T) { + m, root := buildModel(t, `package P { + action def Base { in x; in y; out z; } + action def Proc :> Base { in a; in b; out c; } + }`) + p := sym(t, root, "P") + proc := nested(t, p.Scope, "Proc") + a := nested(t, p.Scope, "Proc", "a") + wantX := nested(t, p.Scope, "Base", "x") + if got := m.AllRedefinedFeatures(a); len(got) != 1 || got[0] != wantX { + t.Fatalf("AllRedefinedFeatures(Proc::a) = %v, want [Base::x]", got) + } + if got := testing.AllocsPerRun(20, func() { m.AllRedefinedFeatures(a) }); got != 0 { + t.Fatalf("a repeated AllRedefinedFeatures query allocates %.0f times, want 0", got) + } + generals := testing.AllocsPerRun(20, func() { m.ImplicitGenerals(proc) }) + if got := testing.AllocsPerRun(20, func() { m.ImplicitParameterRedefinitions(a) }); got > generals+1 { + t.Fatalf("a repeated ImplicitParameterRedefinitions query allocates %.0f times, want at most its result beyond the owner's implied generals (%.0f)", got, generals) + } +} diff --git a/internal/semantic/semantics/masking.go b/internal/semantic/semantics/masking.go index d02c4dc117..26eae714ec 100644 --- a/internal/semantic/semantics/masking.go +++ b/internal/semantic/semantics/masking.go @@ -74,6 +74,10 @@ func (m *Model) AllRedefinedFeatures(sym *symbols.Symbol) []*symbols.Symbol { if m == nil || sym == nil { return nil } + defer m.own(sym).LeaveDoc() + if cached, ok := m.allRedefined[sym]; ok { + return cached + } var out []*symbols.Symbol seen := map[*symbols.Symbol]bool{sym: true} var visit func(*symbols.Symbol) @@ -88,6 +92,8 @@ func (m *Model) AllRedefinedFeatures(sym *symbols.Symbol) []*symbols.Symbol { } } visit(sym) + journal(m, m.allRedefined, sym, sym.Decl) + m.allRedefined[sym] = out return out } diff --git a/internal/semantic/semantics/model.go b/internal/semantic/semantics/model.go index bfdf8e7558..f0fffcc006 100644 --- a/internal/semantic/semantics/model.go +++ b/internal/semantic/semantics/model.go @@ -41,6 +41,7 @@ type Model struct { lookupOrder map[*symbols.Symbol][]lookupSource // name-lookup order contributed map[*symbols.Symbol][]*symbols.Symbol // memoized contributors nestedRedefs map[*symbols.Symbol][]NestedRedefinition + allRedefined map[*symbols.Symbol][]*symbols.Symbol // memoized redefinition closure primTypes map[*symbols.Symbol]PrimType scalars map[*symbols.Symbol]PrimType // stdlib scalar symbols, resolved once params map[*symbols.Symbol]behaviorParameters @@ -168,6 +169,7 @@ func NewModel(resolver *resolve.Resolver) *Model { lookupOrder: make(map[*symbols.Symbol][]lookupSource), contributed: make(map[*symbols.Symbol][]*symbols.Symbol), nestedRedefs: make(map[*symbols.Symbol][]NestedRedefinition), + allRedefined: make(map[*symbols.Symbol][]*symbols.Symbol), primTypes: make(map[*symbols.Symbol]PrimType), params: make(map[*symbols.Symbol]behaviorParameters), invocations: make(map[invocationKey]*InvocationSelection), diff --git a/internal/semantic/semantics/redefinition.go b/internal/semantic/semantics/redefinition.go index f5ab8143dc..23fbbea7b8 100644 --- a/internal/semantic/semantics/redefinition.go +++ b/internal/semantic/semantics/redefinition.go @@ -391,7 +391,8 @@ func (m *Model) implicitParameterTargets(sym *symbols.Symbol, matchDirection boo return nil } position := -1 - for i, p := range positionalParameters(ownedParameters(owner)) { + // The owner's memoized parameters list its owned ones first, in order. + for i, p := range m.parametersOf(owner).positional { if p.sym == sym { position = i break From 606dac8d4e04c3ca5fb4bb168f4d7c17d254ad5a Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Wed, 21 Oct 2026 11:12:16 +0000 Subject: [PATCH 2/2] fix(semantic): memoize a redefinition closure only once it is settled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A closure walked while a guard cut a nested query short — a redefinition still resolving its clause, or an owner whose parameter list is still being computed — may lack a target. AllRedefinedFeatures now runs under a resolver lookup frame and caches only when that frame settled, and parametersOf cuts a re-entrant reader short the way RedefinedFeatures does, instead of answering it silently with an empty list. Co-Authored-By: jason.han --- internal/semantic/semantics/masking.go | 6 ++++ internal/semantic/semantics/model.go | 32 ++++++++++--------- .../semantics/parameters_guard_test.go | 28 ++++++++++++++++ internal/semantic/semantics/redefinition.go | 12 +++++++ .../redefinition_closure_guard_test.go | 28 ++++++++++++++++ 5 files changed, 91 insertions(+), 15 deletions(-) create mode 100644 internal/semantic/semantics/parameters_guard_test.go create mode 100644 internal/semantic/semantics/redefinition_closure_guard_test.go diff --git a/internal/semantic/semantics/masking.go b/internal/semantic/semantics/masking.go index 26eae714ec..45c223f47c 100644 --- a/internal/semantic/semantics/masking.go +++ b/internal/semantic/semantics/masking.go @@ -78,6 +78,7 @@ func (m *Model) AllRedefinedFeatures(sym *symbols.Symbol) []*symbols.Symbol { if cached, ok := m.allRedefined[sym]; ok { return cached } + m.resolver.Enter() var out []*symbols.Symbol seen := map[*symbols.Symbol]bool{sym: true} var visit func(*symbols.Symbol) @@ -92,6 +93,11 @@ func (m *Model) AllRedefinedFeatures(sym *symbols.Symbol) []*symbols.Symbol { } } visit(sym) + // A closure walked while a guard cut a query short may lack a target, so it + // is recomputed on the next query, not memoized. + if !m.resolver.Leave() { + return out + } journal(m, m.allRedefined, sym, sym.Decl) m.allRedefined[sym] = out return out diff --git a/internal/semantic/semantics/model.go b/internal/semantic/semantics/model.go index f0fffcc006..37c8cb049a 100644 --- a/internal/semantic/semantics/model.go +++ b/internal/semantic/semantics/model.go @@ -34,21 +34,22 @@ type Model struct { computingSupers map[*symbols.Symbol]int // valuing holds the features whose typing value is being judged, so a value // that leads back to its own feature is not followed again. - valuing map[*symbols.Symbol]bool - referenced map[*symbols.Symbol]*symbols.Symbol - resolvingRef map[*symbols.Symbol]bool - memberSources map[*symbols.Symbol][]*symbols.Symbol - lookupOrder map[*symbols.Symbol][]lookupSource // name-lookup order - contributed map[*symbols.Symbol][]*symbols.Symbol // memoized contributors - nestedRedefs map[*symbols.Symbol][]NestedRedefinition - allRedefined map[*symbols.Symbol][]*symbols.Symbol // memoized redefinition closure - primTypes map[*symbols.Symbol]PrimType - scalars map[*symbols.Symbol]PrimType // stdlib scalar symbols, resolved once - params map[*symbols.Symbol]behaviorParameters - invocations map[invocationKey]*InvocationSelection - arguments ArgumentTyper // the checker's argument typing, nil when no checker runs - sourceText source.Lookup // notation of the loaded documents, nil when unavailable - sourceFile source.Locate // the files the documents were read from, nil for source.FileNamed + valuing map[*symbols.Symbol]bool + referenced map[*symbols.Symbol]*symbols.Symbol + resolvingRef map[*symbols.Symbol]bool + memberSources map[*symbols.Symbol][]*symbols.Symbol + lookupOrder map[*symbols.Symbol][]lookupSource // name-lookup order + contributed map[*symbols.Symbol][]*symbols.Symbol // memoized contributors + nestedRedefs map[*symbols.Symbol][]NestedRedefinition + allRedefined map[*symbols.Symbol][]*symbols.Symbol // memoized redefinition closure + primTypes map[*symbols.Symbol]PrimType + scalars map[*symbols.Symbol]PrimType // stdlib scalar symbols, resolved once + params map[*symbols.Symbol]behaviorParameters + computingParams map[*symbols.Symbol]int // lookup depth of a params entry being computed + invocations map[invocationKey]*InvocationSelection + arguments ArgumentTyper // the checker's argument typing, nil when no checker runs + sourceText source.Lookup // notation of the loaded documents, nil when unavailable + sourceFile source.Locate // the files the documents were read from, nil for source.FileNamed // typingArgs holds the calls whose arguments are being typed, so an argument // whose type leads back to its own call is not typed again. typingArgs map[*ast.InvocationExpr]bool @@ -172,6 +173,7 @@ func NewModel(resolver *resolve.Resolver) *Model { allRedefined: make(map[*symbols.Symbol][]*symbols.Symbol), primTypes: make(map[*symbols.Symbol]PrimType), params: make(map[*symbols.Symbol]behaviorParameters), + computingParams: make(map[*symbols.Symbol]int), invocations: make(map[invocationKey]*InvocationSelection), typingArgs: make(map[*ast.InvocationExpr]bool), composed: make(map[composedKey][]*symbols.Symbol), diff --git a/internal/semantic/semantics/parameters_guard_test.go b/internal/semantic/semantics/parameters_guard_test.go new file mode 100644 index 0000000000..9271fd59d8 --- /dev/null +++ b/internal/semantic/semantics/parameters_guard_test.go @@ -0,0 +1,28 @@ +package semantics + +import "testing" + +// A parameter list read while its owner's list is still being computed cuts +// the reading query short, so a closure built on it is not memoized either. +func TestParameterListGuardCutsDependentQueriesShort(t *testing.T) { + m, root := buildModel(t, `package P { + action def Base { in x; out z; } + action def Proc :> Base { in a; out c; } + }`) + p := sym(t, root, "P") + proc := nested(t, p.Scope, "Proc") + a := nested(t, p.Scope, "Proc", "a") + wantX := nested(t, p.Scope, "Base", "x") + // As parametersOf stands mid-computation for the owner. + m.params[proc] = behaviorParameters{} + m.computingParams[proc] = m.resolver.Enter() + if got := m.AllRedefinedFeatures(a); len(got) != 0 { + t.Fatalf("AllRedefinedFeatures(Proc::a) under the guard = %v, want the guard's empty answer", got) + } + m.resolver.Leave() + delete(m.params, proc) + delete(m.computingParams, proc) + if got := m.AllRedefinedFeatures(a); len(got) != 1 || got[0] != wantX { + t.Fatalf("AllRedefinedFeatures(Proc::a) after the guard = %v, want [Base::x]", got) + } +} diff --git a/internal/semantic/semantics/redefinition.go b/internal/semantic/semantics/redefinition.go index 23fbbea7b8..e76c0cb421 100644 --- a/internal/semantic/semantics/redefinition.go +++ b/internal/semantic/semantics/redefinition.go @@ -159,11 +159,17 @@ func (m *Model) BehaviorParametersOf(sym *symbols.Symbol) []BehaviorParameter { func (m *Model) parametersOf(sym *symbols.Symbol) behaviorParameters { defer m.own(sym).LeaveDoc() if cached, ok := m.params[sym]; ok { + // The seed answers a re-entrant query with nothing, cutting that query short. + if depth := m.computingParams[sym]; depth != 0 { + m.resolver.CutShort(depth) + } return cached } // Guard against re-entrancy on cyclic specialization graphs. journal(m, m.params, sym, sym.Decl) m.params[sym] = behaviorParameters{} + m.computingParams[sym] = m.resolver.Enter() + defer delete(m.computingParams, sym) owned := ownedParameters(sym) out := behaviorParameters{positional: positionalParameters(owned), result: resultParameter(owned)} @@ -194,6 +200,12 @@ func (m *Model) parametersOf(sym *symbols.Symbol) behaviorParameters { } } + // A list derived while a guard cut a query short may have missed a + // parameter, so it is recomputed on the next query, not memoized. + if !m.resolver.Leave() { + delete(m.params, sym) + return out + } m.params[sym] = out return out } diff --git a/internal/semantic/semantics/redefinition_closure_guard_test.go b/internal/semantic/semantics/redefinition_closure_guard_test.go new file mode 100644 index 0000000000..4cc09d6479 --- /dev/null +++ b/internal/semantic/semantics/redefinition_closure_guard_test.go @@ -0,0 +1,28 @@ +package semantics + +import "testing" + +// A redefinition closure walked while a guard cuts a query short is answered +// but not memoized: once the guard lifts, the next query sees the target. +func TestRedefinitionClosureIsNotCachedWhileAGuardCutsItShort(t *testing.T) { + m, root := buildModel(t, `package P { + part def W { attribute x; } + part def A { part w : W; } + part def B :> A { attribute z :>> w.x; } + }`) + p := sym(t, root, "P") + wantX := nested(t, p.Scope, "W", "x") + bz := nested(t, p.Scope, "B", "z") + // As RedefinedFeatures stands mid-computation: seeded, with its lookup open. + m.redefined[bz] = nil + m.computingRedefined[bz] = m.resolver.Enter() + if got := m.AllRedefinedFeatures(bz); len(got) != 0 { + t.Fatalf("AllRedefinedFeatures(B::z) under the guard = %v, want the guard's empty answer", got) + } + m.resolver.Leave() + delete(m.redefined, bz) + delete(m.computingRedefined, bz) + if got := m.AllRedefinedFeatures(bz); len(got) != 1 || got[0] != wantX { + t.Fatalf("AllRedefinedFeatures(B::z) after the guard = %v, want [W::x]", got) + } +}