gopls/internal/lsp/cache: use persistent.Set in a couple places
Use the new persistent.Set type to track known dirs and unloadable
files.
Change-Id: I3e0d4bdc846f4c37a0046a01bf67d83bc06b9598
Reviewed-on: https://go-review.googlesource.com/c/tools/+/524762
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
Auto-Submit: Robert Findley <rfindley@google.com>
diff --git a/gopls/internal/lsp/cache/maps.go b/gopls/internal/lsp/cache/maps.go
index 3fa866c..edb72d5 100644
--- a/gopls/internal/lsp/cache/maps.go
+++ b/gopls/internal/lsp/cache/maps.go
@@ -76,46 +76,3 @@
}
return overlays
}
-
-type knownDirsSet struct {
- impl *persistent.Map[span.URI, struct{}]
-}
-
-func newKnownDirsSet() knownDirsSet {
- return knownDirsSet{
- impl: new(persistent.Map[span.URI, struct{}]),
- }
-}
-
-func (s knownDirsSet) Clone() knownDirsSet {
- return knownDirsSet{
- impl: s.impl.Clone(),
- }
-}
-
-func (s knownDirsSet) Destroy() {
- s.impl.Destroy()
-}
-
-func (s knownDirsSet) Contains(key span.URI) bool {
- _, ok := s.impl.Get(key)
- return ok
-}
-
-func (s knownDirsSet) Range(do func(key span.URI)) {
- s.impl.Range(func(key span.URI, value struct{}) {
- do(key)
- })
-}
-
-func (s knownDirsSet) SetAll(other knownDirsSet) {
- s.impl.SetAll(other.impl)
-}
-
-func (s knownDirsSet) Insert(key span.URI) {
- s.impl.Set(key, struct{}{}, nil)
-}
-
-func (s knownDirsSet) Remove(key span.URI) {
- s.impl.Delete(key)
-}
diff --git a/gopls/internal/lsp/cache/session.go b/gopls/internal/lsp/cache/session.go
index cd51e6d..1e463fa 100644
--- a/gopls/internal/lsp/cache/session.go
+++ b/gopls/internal/lsp/cache/session.go
@@ -176,13 +176,13 @@
activePackages: new(persistent.Map[PackageID, *Package]),
symbolizeHandles: new(persistent.Map[span.URI, *memoize.Promise]),
workspacePackages: make(map[PackageID]PackagePath),
- unloadableFiles: make(map[span.URI]struct{}),
+ unloadableFiles: new(persistent.Set[span.URI]),
parseModHandles: new(persistent.Map[span.URI, *memoize.Promise]),
parseWorkHandles: new(persistent.Map[span.URI, *memoize.Promise]),
modTidyHandles: new(persistent.Map[span.URI, *memoize.Promise]),
modVulnHandles: new(persistent.Map[span.URI, *memoize.Promise]),
modWhyHandles: new(persistent.Map[span.URI, *memoize.Promise]),
- knownSubdirs: newKnownDirsSet(),
+ knownSubdirs: new(persistent.Set[span.URI]),
workspaceModFiles: wsModFiles,
workspaceModFilesErr: wsModFilesErr,
pkgIndex: typerefs.NewPackageIndex(),
@@ -619,15 +619,15 @@
// knownDirectories returns all of the directories known to the given
// snapshots, including workspace directories and their subdirectories.
// It is responsibility of the caller to destroy the returned set.
-func knownDirectories(ctx context.Context, snapshots []*snapshot) knownDirsSet {
- result := newKnownDirsSet()
+func knownDirectories(ctx context.Context, snapshots []*snapshot) *persistent.Set[span.URI] {
+ result := new(persistent.Set[span.URI])
for _, snapshot := range snapshots {
dirs := snapshot.dirs(ctx)
for _, dir := range dirs {
- result.Insert(dir)
+ result.Add(dir)
}
knownSubdirs := snapshot.getKnownSubdirs(dirs)
- result.SetAll(knownSubdirs)
+ result.AddAll(knownSubdirs)
knownSubdirs.Destroy()
}
return result
diff --git a/gopls/internal/lsp/cache/snapshot.go b/gopls/internal/lsp/cache/snapshot.go
index a914880..94eceed 100644
--- a/gopls/internal/lsp/cache/snapshot.go
+++ b/gopls/internal/lsp/cache/snapshot.go
@@ -130,7 +130,7 @@
shouldLoad map[PackageID][]PackagePath
// unloadableFiles keeps track of files that we've failed to load.
- unloadableFiles map[span.URI]struct{}
+ unloadableFiles *persistent.Set[span.URI]
// TODO(rfindley): rename the handles below to "promises". A promise is
// different from a handle (we mutate the package handle.)
@@ -152,7 +152,7 @@
// knownSubdirs is the set of subdirectory URIs in the workspace,
// used to create glob patterns for file watching.
- knownSubdirs knownDirsSet
+ knownSubdirs *persistent.Set[span.URI]
knownSubdirsCache map[string]struct{} // memo of knownSubdirs as a set of filenames
// unprocessedSubdirChanges are any changes that might affect the set of
// subdirectories in the workspace. They are not reflected to knownSubdirs
@@ -269,6 +269,7 @@
s.modTidyHandles.Destroy()
s.modVulnHandles.Destroy()
s.modWhyHandles.Destroy()
+ s.unloadableFiles.Destroy()
}
func (s *snapshot) SequenceID() uint64 {
@@ -750,7 +751,7 @@
}
// Check if uri is known to be unloadable.
- _, unloadable := s.unloadableFiles[uri]
+ unloadable := s.unloadableFiles.Contains(uri)
s.mu.Unlock()
@@ -803,7 +804,7 @@
// so if we get here and still have
// no IDs, uri is unloadable.
if !unloadable && len(ids) == 0 {
- s.unloadableFiles[uri] = struct{}{}
+ s.unloadableFiles.Add(uri)
}
// Sort packages "narrowest" to "widest" (in practice:
@@ -1017,14 +1018,14 @@
defer s.mu.Unlock()
s.knownSubdirs.Destroy()
- s.knownSubdirs = newKnownDirsSet()
+ s.knownSubdirs = new(persistent.Set[span.URI])
s.knownSubdirsCache = nil
s.files.Range(func(uri span.URI, fh source.FileHandle) {
s.addKnownSubdirLocked(uri, dirs)
})
}
-func (s *snapshot) getKnownSubdirs(wsDirs []span.URI) knownDirsSet {
+func (s *snapshot) getKnownSubdirs(wsDirs []span.URI) *persistent.Set[span.URI] {
s.mu.Lock()
defer s.mu.Unlock()
@@ -1075,7 +1076,7 @@
if s.knownSubdirs.Contains(uri) {
break
}
- s.knownSubdirs.Insert(uri)
+ s.knownSubdirs.Add(uri)
dir = filepath.Dir(dir)
s.knownSubdirsCache = nil
}
@@ -1592,7 +1593,7 @@
s.mu.Lock()
loadable := files[:0]
for _, file := range files {
- if _, unloadable := s.unloadableFiles[file.URI()]; !unloadable {
+ if !s.unloadableFiles.Contains(file.URI()) {
loadable = append(loadable, file)
}
}
@@ -1655,7 +1656,7 @@
// metadata graph that resulted from loading.
uri := file.URI()
if len(s.meta.ids[uri]) == 0 {
- s.unloadableFiles[uri] = struct{}{}
+ s.unloadableFiles.Add(uri)
}
}
@@ -1969,7 +1970,7 @@
files: s.files.Clone(),
symbolizeHandles: s.symbolizeHandles.Clone(),
workspacePackages: make(map[PackageID]PackagePath, len(s.workspacePackages)),
- unloadableFiles: make(map[span.URI]struct{}, len(s.unloadableFiles)),
+ unloadableFiles: s.unloadableFiles.Clone(), // see the TODO for unloadableFiles below
parseModHandles: s.parseModHandles.Clone(),
parseWorkHandles: s.parseWorkHandles.Clone(),
modTidyHandles: s.modTidyHandles.Clone(),
@@ -1993,16 +1994,11 @@
// incref/decref operation that might destroy it prematurely.)
release := result.Acquire()
- // Copy the set of unloadable files.
- //
- // TODO(rfindley): this looks wrong. Shouldn't we clear unloadableFiles on
+ // TODO(rfindley): this looks wrong. Should we clear unloadableFiles on
// changes to environment or workspace layout, or more generally on any
// metadata change?
//
// Maybe not, as major configuration changes cause a new view.
- for k, v := range s.unloadableFiles {
- result.unloadableFiles[k] = v
- }
// Add all of the known subdirectories, but don't update them for the
// changed files. We need to rebuild the workspace module to know the
@@ -2119,7 +2115,7 @@
// TODO(rfindley): this also looks wrong, as typing in an unloadable file
// will result in repeated reloads. We should only delete if metadata
// changed.
- delete(result.unloadableFiles, uri)
+ result.unloadableFiles.Remove(uri)
}
// Deleting an import can cause list errors due to import cycles to be
