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 <adonovan@google.com>
Reviewed-by: David Chase <drchase@google.com>
diff --git a/internal/lsp/cache/check.go b/internal/lsp/cache/check.go
index 5ded278..7ebc777 100644
--- a/internal/lsp/cache/check.go
+++ b/internal/lsp/cache/check.go
@@ -224,7 +224,7 @@
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 00d4ab2..e4fd671 100644
--- a/internal/lsp/cache/load.go
+++ b/internal/lsp/cache/load.go
@@ -220,7 +220,7 @@
// 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 f2958b0..af04177 100644
--- a/internal/lsp/cache/maps.go
+++ b/internal/lsp/cache/maps.go
@@ -112,6 +112,40 @@
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 52b141a..aca46dd 100644
--- a/internal/lsp/cache/session.go
+++ b/internal/lsp/cache/session.go
@@ -225,27 +225,28 @@
},
}
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 85232ea..36dcafe 100644
--- a/internal/lsp/cache/snapshot.go
+++ b/internal/lsp/cache/snapshot.go
@@ -91,6 +91,10 @@
// 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 @@
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 @@
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 @@
// 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 @@
}
}
s.meta = g.Clone(updates)
+ s.resetIsActivePackageLocked()
}
// noValidMetadataForURILocked reports whether there is any valid metadata for
@@ -1377,7 +1384,7 @@
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) 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 @@
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 @@
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
}
