diff --git a/changes/unreleased/memoize-short-named.performance.md b/changes/unreleased/memoize-short-named.performance.md new file mode 100644 index 000000000..5ce7370a7 --- /dev/null +++ b/changes/unreleased/memoize-short-named.performance.md @@ -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. diff --git a/internal/semantic/symbols/index.go b/internal/semantic/symbols/index.go index 173430424..bb15efa71 100644 --- a/internal/semantic/symbols/index.go +++ b/internal/semantic/symbols/index.go @@ -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 @@ -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), @@ -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), @@ -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 { @@ -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 } } diff --git a/internal/semantic/symbols/short_named_test.go b/internal/semantic/symbols/short_named_test.go new file mode 100644 index 000000000..e1df01195 --- /dev/null +++ b/internal/semantic/symbols/short_named_test.go @@ -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 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)") + } +} diff --git a/internal/semantic/symbols/snapshot_test.go b/internal/semantic/symbols/snapshot_test.go index 4f3812613..d489e67d4 100644 --- a/internal/semantic/symbols/snapshot_test.go +++ b/internal/semantic/symbols/snapshot_test.go @@ -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", ) diff --git a/internal/workspace/libs/snapshot_test.go b/internal/workspace/libs/snapshot_test.go index b28172cfe..861dbba9e 100644 --- a/internal/workspace/libs/snapshot_test.go +++ b/internal/workspace/libs/snapshot_test.go @@ -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) }