internal/lsp/cache: don't walk URIs to invalidate metadata
Since ids is derived from metadata, we should not have to walk ids to
see which metadata is still active. Just compute metadata updates
directly.
Benchmark (didChange in k8s): ~45ms->41ms
For golang/go#45686
Change-Id: Id557ed3f2e05c903e4bb3f3f6a4af864751c4546
Reviewed-on: https://go-review.googlesource.com/c/tools/+/340854
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go
index 56ffdf8..7240357 100644
--- a/internal/lsp/cache/snapshot.go
+++ b/internal/lsp/cache/snapshot.go
@@ -1933,6 +1933,12 @@
// If a file has been deleted, we must delete metadata for all packages
// containing that file.
workspaceModeChanged := s.workspaceMode() != result.workspaceMode()
+
+ // Don't keep package metadata for packages that have lost files.
+ //
+ // TODO(rfindley): why not keep invalid metadata in this case? If we
+ // otherwise allow operate on invalid metadata, why not continue to do so,
+ // skipping the missing file?
skipID := map[PackageID]bool{}
for _, c := range changes {
if c.exists {
@@ -1967,41 +1973,23 @@
addForwardDeps(id)
}
- // Compute which IDs are in the snapshot.
- //
- // TODO(rfindley): this step shouldn't be necessary, since we compute skipID
- // above based on meta.ids.
- deleteInvalidMetadata := forceReloadMetadata || workspaceModeChanged
- idsInSnapshot := map[PackageID]bool{} // track all known IDs
- for _, ids := range s.meta.ids {
- for _, id := range ids {
- if skipID[id] || deleteInvalidMetadata && idsToInvalidate[id] {
- continue
- }
- // The ID is not reachable from any workspace package, so it should
- // be deleted.
- if !reachableID[id] {
- continue
- }
- idsInSnapshot[id] = true
- }
- }
- // TODO(adonovan): opt: represent PackageID as an index into a process-global
- // dup-free list of all package names ever seen, then use a bitmap instead of
- // a hash table for "PackageSet" (e.g. idsInSnapshot).
-
// Compute which metadata updates are required. We only need to invalidate
// packages directly containing the affected file, and only if it changed in
// a relevant way.
metadataUpdates := make(map[PackageID]*KnownMetadata)
+ deleteInvalidMetadata := forceReloadMetadata || workspaceModeChanged
for k, v := range s.meta.metadata {
- if !idsInSnapshot[k] {
- // Delete metadata for IDs that are no longer reachable from files
- // in the snapshot.
+ invalidateMetadata := idsToInvalidate[k]
+ if skipID[k] || (invalidateMetadata && deleteInvalidMetadata) {
metadataUpdates[k] = nil
continue
}
- invalidateMetadata := idsToInvalidate[k]
+ // The ID is not reachable from any workspace package, so it should
+ // be deleted.
+ if !reachableID[k] {
+ metadataUpdates[k] = nil
+ continue
+ }
valid := v.Valid && !invalidateMetadata
pkgFilesChanged := v.PkgFilesChanged || changedPkgFiles[k]
shouldLoad := v.ShouldLoad || invalidateMetadata
