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
1 change: 1 addition & 0 deletions changes/unreleased/memoize-short-named.performance.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- Validating a model rich in membership imports no longer re-scans the names registered under a segment on every lookup: `ShortNamed` is memoized per index generation, undoing a ~25% whole-model validate slowdown introduced with the import-prune fix.
24 changes: 24 additions & 0 deletions internal/semantic/symbols/index.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ type Index struct {
directChildrenGeneration uint64
directChildrenCache map[directChildrenKey][]*Symbol
directChildrenByName map[directChildrenKey]map[string][]*Symbol
// shortNamedCache memoizes ShortNamed per segment, reset with the
// direct-children caches.
shortNamedCache map[string]bool

docRoots *layer[string, *Scope] // document name -> root scope
docOfRoot *layer[*Scope, string] // root scope -> document name
Expand Down Expand Up @@ -193,6 +196,7 @@ func NewIndex() *Index {
generation: gen,
directChildrenCache: make(map[directChildrenKey][]*Symbol),
directChildrenByName: make(map[directChildrenKey]map[string][]*Symbol),
shortNamedCache: make(map[string]bool),
docRoots: newLayer[string, *Scope](gen),
docOfRoot: newLayer[*Scope, string](gen),
docKinds: newLayer[string, source.Kind](gen),
Expand Down Expand Up @@ -311,6 +315,7 @@ func NewOverlay(base *Index) *Index {
generation: gen,
directChildrenCache: make(map[directChildrenKey][]*Symbol),
directChildrenByName: make(map[directChildrenKey]map[string][]*Symbol),
shortNamedCache: make(map[string]bool),
docRoots: overLayer(base.docRoots, gen),
docOfRoot: overLayer(base.docOfRoot, gen),
docKinds: overLayer(base.docKinds, gen),
Expand Down Expand Up @@ -1590,6 +1595,24 @@ func (idx *Index) ShortNamed(name string) bool {
return false
}
idx.readSegment(name)
generation := idx.generation.get()
idx.directChildrenMu.Lock()
idx.resetDirectChildrenCachesLocked(generation)
if v, ok := idx.shortNamedCache[name]; ok {
idx.directChildrenMu.Unlock()
return v
}
idx.directChildrenMu.Unlock()
v := idx.shortNamedScan(name)
idx.directChildrenMu.Lock()
if idx.generation.get() == generation {
idx.shortNamedCache[name] = v
}
idx.directChildrenMu.Unlock()
return v
}

func (idx *Index) shortNamedScan(name string) bool {
shortRegistered := func(fqn string) bool {
for _, sym := range idx.fqn.at(fqn) {
if LastSegment(sym.Name) != name {
Expand Down Expand Up @@ -1712,6 +1735,7 @@ func (idx *Index) resetDirectChildrenCachesLocked(generation uint64) {
if idx.directChildrenGeneration != generation {
idx.directChildrenCache = make(map[directChildrenKey][]*Symbol)
idx.directChildrenByName = make(map[directChildrenKey]map[string][]*Symbol)
idx.shortNamedCache = make(map[string]bool)
idx.directChildrenGeneration = generation
}
}
Expand Down
31 changes: 31 additions & 0 deletions internal/semantic/symbols/short_named_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package symbols

import "testing"

func TestShortNamedFollowsIndexGeneration(t *testing.T) {
idx := NewIndex()
if idx.ShortNamed("s") {
t.Fatal("ShortNamed(s) = true on an empty index")
}
if idx.ShortNamed("s") {
t.Fatal("ShortNamed(s) changed on a repeated call of an unchanged index")
}

addDoc(t, idx, "a.sysml", "package P { part def <s> Y; }")
if !idx.ShortNamed("s") {
t.Fatal("ShortNamed(s) = false after adding a short-named member (stale cache)")
}
if !idx.ShortNamed("s") {
t.Fatal("ShortNamed(s) changed on a repeated call of an unchanged index")
}

addDoc(t, idx, "a.sysml", "package P { part def Z; }")
if idx.ShortNamed("s") {
t.Fatal("ShortNamed(s) = true after the document dropped the short name (stale cache)")
}

idx.RemoveDocument("a.sysml")
if idx.ShortNamed("s") {
t.Fatal("ShortNamed(s) = true after removing the document (stale cache)")
}
}
2 changes: 1 addition & 1 deletion internal/semantic/symbols/snapshot_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (
// name leaves behind once its parts are appended elsewhere.
var unobservable = graphcmp.SkipFields(
"Index.directChildrenGeneration", "libraryIdentityMemo.gen",
"Index.directChildrenCache", "Index.directChildrenByName",
"Index.directChildrenCache", "Index.directChildrenByName", "Index.shortNamedCache",
"QualifiedName.part0",
)

Expand Down
2 changes: 1 addition & 1 deletion internal/workspace/libs/snapshot_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ func TestSnapshotIndexMatchesFreshLoad(t *testing.T) {
// the inline storage a multi-part name leaves behind (see symbols' tests).
if err := graphcmp.Equal(fresh, decoded, graphcmp.SkipFields(
"Index.directChildrenGeneration", "libraryIdentityMemo.gen", "Index.directChildrenCache",
"Index.directChildrenByName", "QualifiedName.part0",
"Index.directChildrenByName", "Index.shortNamedCache", "QualifiedName.part0",
)); err != nil {
t.Errorf("decoded index differs from a fresh load: %v", err)
}
Expand Down
Loading