From d69bac6d882444114a07779657ae97128a4fac76 Mon Sep 17 00:00:00 2001 From: Ruslan Nigmatullin Date: Wed, 22 Jun 2022 22:39:54 +0000 Subject: [PATCH] internal/lsp/cache: cache isActiveLocked calculation across snapshots This speeds up workspace initialization time in DegradeClosed memory mode from 3m to 1m by avoiding unnecessary recomputation of results. Change-Id: Ie5df82952d50ab42125defd148136329f0d50a48 Reviewed-on: https://go-review.googlesource.com/c/tools/+/413658 Reviewed-by: Alan Donovan Reviewed-by: David Chase --- internal/lsp/cache/check.go | 2 +- internal/lsp/cache/load.go | 2 +- internal/lsp/cache/maps.go | 34 +++++++++++++++ internal/lsp/cache/session.go | 43 +++++++++--------- internal/lsp/cache/snapshot.go | 79 ++++++++++++++++++++-------------- 5 files changed, 104 insertions(+), 56 deletions(-) diff --git a/internal/lsp/cache/check.go b/internal/lsp/cache/check.go index 5ded278f51..7ebc777d76 100644 --- a/internal/lsp/cache/check.go +++ b/internal/lsp/cache/check.go @@ -224,7 +224,7 @@ func (s *snapshot) workspaceParseMode(id PackageID) source.ParseMode { if s.view.Options().MemoryMode == source.ModeNormal { return source.ParseFull } - if s.isActiveLocked(id, nil) { + if s.isActiveLocked(id) { return source.ParseFull } return source.ParseExported diff --git a/internal/lsp/cache/load.go b/internal/lsp/cache/load.go index 00d4ab2cbb..e4fd671987 100644 --- a/internal/lsp/cache/load.go +++ b/internal/lsp/cache/load.go @@ -220,7 +220,7 @@ func (s *snapshot) load(ctx context.Context, allowNetwork bool, scopes ...interf // invalidate the reverse transitive closure of packages that have changed. invalidatedPackages := s.meta.reverseTransitiveClosure(true, loadedIDs...) s.meta = s.meta.Clone(updates) - + s.resetIsActivePackageLocked() // Invalidate any packages we may have associated with this metadata. // // TODO(rfindley): this should not be necessary, as we should have already diff --git a/internal/lsp/cache/maps.go b/internal/lsp/cache/maps.go index f2958b0e12..af04177718 100644 --- a/internal/lsp/cache/maps.go +++ b/internal/lsp/cache/maps.go @@ -112,6 +112,40 @@ func (m goFilesMap) Delete(key parseKey) { m.impl.Delete(key) } +type isActivePackageCacheMap struct { + impl *persistent.Map +} + +func newIsActivePackageCacheMap() isActivePackageCacheMap { + return isActivePackageCacheMap{ + impl: persistent.NewMap(func(a, b interface{}) bool { + return a.(PackageID) < b.(PackageID) + }), + } +} + +func (m isActivePackageCacheMap) Clone() isActivePackageCacheMap { + return isActivePackageCacheMap{ + impl: m.impl.Clone(), + } +} + +func (m isActivePackageCacheMap) Destroy() { + m.impl.Destroy() +} + +func (m isActivePackageCacheMap) Get(key PackageID) (bool, bool) { + value, ok := m.impl.Get(key) + if !ok { + return false, false + } + return value.(bool), true +} + +func (m isActivePackageCacheMap) Set(key PackageID, value bool) { + m.impl.Set(key, value, nil) +} + type parseKeysByURIMap struct { impl *persistent.Map } diff --git a/internal/lsp/cache/session.go b/internal/lsp/cache/session.go index 52b141a78d..aca46dd2a8 100644 --- a/internal/lsp/cache/session.go +++ b/internal/lsp/cache/session.go @@ -225,27 +225,28 @@ func (s *Session) createView(ctx context.Context, name string, folder span.URI, }, } v.snapshot = &snapshot{ - id: snapshotID, - view: v, - backgroundCtx: backgroundCtx, - cancel: cancel, - initializeOnce: &sync.Once{}, - generation: s.cache.store.Generation(generationName(v, 0)), - packages: newPackagesMap(), - meta: &metadataGraph{}, - files: newFilesMap(), - goFiles: newGoFilesMap(), - parseKeysByURI: newParseKeysByURIMap(), - symbols: make(map[span.URI]*symbolHandle), - actions: make(map[actionKey]*actionHandle), - workspacePackages: make(map[PackageID]PackagePath), - unloadableFiles: make(map[span.URI]struct{}), - parseModHandles: make(map[span.URI]*parseModHandle), - parseWorkHandles: make(map[span.URI]*parseWorkHandle), - modTidyHandles: make(map[span.URI]*modTidyHandle), - modWhyHandles: make(map[span.URI]*modWhyHandle), - knownSubdirs: newKnownDirsSet(), - workspace: workspace, + id: snapshotID, + view: v, + backgroundCtx: backgroundCtx, + cancel: cancel, + initializeOnce: &sync.Once{}, + generation: s.cache.store.Generation(generationName(v, 0)), + packages: newPackagesMap(), + meta: &metadataGraph{}, + files: newFilesMap(), + isActivePackageCache: newIsActivePackageCacheMap(), + goFiles: newGoFilesMap(), + parseKeysByURI: newParseKeysByURIMap(), + symbols: make(map[span.URI]*symbolHandle), + actions: make(map[actionKey]*actionHandle), + workspacePackages: make(map[PackageID]PackagePath), + unloadableFiles: make(map[span.URI]struct{}), + parseModHandles: make(map[span.URI]*parseModHandle), + parseWorkHandles: make(map[span.URI]*parseWorkHandle), + modTidyHandles: make(map[span.URI]*modTidyHandle), + modWhyHandles: make(map[span.URI]*modWhyHandle), + knownSubdirs: newKnownDirsSet(), + workspace: workspace, } // Initialize the view without blocking. diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go index 85232ea835..36dcafeaca 100644 --- a/internal/lsp/cache/snapshot.go +++ b/internal/lsp/cache/snapshot.go @@ -91,6 +91,10 @@ type snapshot struct { // It may be invalidated when a file's content changes. packages packagesMap + // isActivePackageCache maps package ID to the cached value if it is active or not. + // It may be invalidated when metadata changes or a new file is opened or closed. + isActivePackageCache isActivePackageCacheMap + // actions maps an actionkey to its actionHandle. actions map[actionKey]*actionHandle @@ -144,6 +148,7 @@ type actionKey struct { func (s *snapshot) Destroy(destroyedBy string) { s.generation.Destroy(destroyedBy) s.packages.Destroy() + s.isActivePackageCache.Destroy() s.files.Destroy() s.goFiles.Destroy() s.parseKeysByURI.Destroy() @@ -754,24 +759,20 @@ func (s *snapshot) activePackageIDs() (ids []PackageID) { s.mu.Lock() defer s.mu.Unlock() - seen := make(map[PackageID]bool) for id := range s.workspacePackages { - if s.isActiveLocked(id, seen) { + if s.isActiveLocked(id) { ids = append(ids, id) } } return ids } -func (s *snapshot) isActiveLocked(id PackageID, seen map[PackageID]bool) (active bool) { - if seen == nil { - seen = make(map[PackageID]bool) - } - if seen, ok := seen[id]; ok { +func (s *snapshot) isActiveLocked(id PackageID) (active bool) { + if seen, ok := s.isActivePackageCache.Get(id); ok { return seen } defer func() { - seen[id] = active + s.isActivePackageCache.Set(id, active) }() m, ok := s.meta.metadata[id] if !ok { @@ -785,13 +786,18 @@ func (s *snapshot) isActiveLocked(id PackageID, seen map[PackageID]bool) (active // TODO(rfindley): it looks incorrect that we don't also check GoFiles here. // If a CGo file is open, we want to consider the package active. for _, dep := range m.Deps { - if s.isActiveLocked(dep, seen) { + if s.isActiveLocked(dep) { return true } } return false } +func (s *snapshot) resetIsActivePackageLocked() { + s.isActivePackageCache.Destroy() + s.isActivePackageCache = newIsActivePackageCacheMap() +} + const fileExtensions = "go,mod,sum,work" func (s *snapshot) fileWatchingGlobPatterns(ctx context.Context) map[string]struct{} { @@ -1287,6 +1293,7 @@ func (s *snapshot) clearShouldLoad(scopes ...interface{}) { } } s.meta = g.Clone(updates) + s.resetIsActivePackageLocked() } // noValidMetadataForURILocked reports whether there is any valid metadata for @@ -1377,7 +1384,7 @@ func (s *snapshot) openFiles() []source.VersionedFileHandle { var open []source.VersionedFileHandle s.files.Range(func(uri span.URI, fh source.VersionedFileHandle) { - if s.isOpenLocked(fh.URI()) { + if isFileOpen(fh) { open = append(open, fh) } }) @@ -1386,6 +1393,10 @@ func (s *snapshot) openFiles() []source.VersionedFileHandle { func (s *snapshot) isOpenLocked(uri span.URI) bool { fh, _ := s.files.Get(uri) + return isFileOpen(fh) +} + +func isFileOpen(fh source.VersionedFileHandle) bool { _, open := fh.(*overlay) return open } @@ -1695,28 +1706,29 @@ func (s *snapshot) clone(ctx, bgCtx context.Context, changes map[span.URI]*fileC newGen := s.view.session.cache.store.Generation(generationName(s.view, s.id+1)) bgCtx, cancel := context.WithCancel(bgCtx) result := &snapshot{ - id: s.id + 1, - generation: newGen, - view: s.view, - backgroundCtx: bgCtx, - cancel: cancel, - builtin: s.builtin, - initializeOnce: s.initializeOnce, - initializedErr: s.initializedErr, - packages: s.packages.Clone(), - actions: make(map[actionKey]*actionHandle, len(s.actions)), - files: s.files.Clone(), - goFiles: s.goFiles.Clone(), - parseKeysByURI: s.parseKeysByURI.Clone(), - symbols: make(map[span.URI]*symbolHandle, len(s.symbols)), - workspacePackages: make(map[PackageID]PackagePath, len(s.workspacePackages)), - unloadableFiles: make(map[span.URI]struct{}, len(s.unloadableFiles)), - parseModHandles: make(map[span.URI]*parseModHandle, len(s.parseModHandles)), - parseWorkHandles: make(map[span.URI]*parseWorkHandle, len(s.parseWorkHandles)), - modTidyHandles: make(map[span.URI]*modTidyHandle, len(s.modTidyHandles)), - modWhyHandles: make(map[span.URI]*modWhyHandle, len(s.modWhyHandles)), - knownSubdirs: s.knownSubdirs.Clone(), - workspace: newWorkspace, + id: s.id + 1, + generation: newGen, + view: s.view, + backgroundCtx: bgCtx, + cancel: cancel, + builtin: s.builtin, + initializeOnce: s.initializeOnce, + initializedErr: s.initializedErr, + packages: s.packages.Clone(), + isActivePackageCache: s.isActivePackageCache.Clone(), + actions: make(map[actionKey]*actionHandle, len(s.actions)), + files: s.files.Clone(), + goFiles: s.goFiles.Clone(), + parseKeysByURI: s.parseKeysByURI.Clone(), + symbols: make(map[span.URI]*symbolHandle, len(s.symbols)), + workspacePackages: make(map[PackageID]PackagePath, len(s.workspacePackages)), + unloadableFiles: make(map[span.URI]struct{}, len(s.unloadableFiles)), + parseModHandles: make(map[span.URI]*parseModHandle, len(s.parseModHandles)), + parseWorkHandles: make(map[span.URI]*parseWorkHandle, len(s.parseWorkHandles)), + modTidyHandles: make(map[span.URI]*modTidyHandle, len(s.modTidyHandles)), + modWhyHandles: make(map[span.URI]*modWhyHandle, len(s.modWhyHandles)), + knownSubdirs: s.knownSubdirs.Clone(), + workspace: newWorkspace, } // Copy all of the FileHandles. @@ -1975,9 +1987,10 @@ func (s *snapshot) clone(ctx, bgCtx context.Context, changes map[span.URI]*fileC result.meta = s.meta } - // Update workspace packages, if necessary. + // Update workspace and active packages, if necessary. if result.meta != s.meta || anyFileOpenedOrClosed { result.workspacePackages = computeWorkspacePackagesLocked(result, result.meta) + result.resetIsActivePackageLocked() } else { result.workspacePackages = s.workspacePackages }