From bec0cf16be3beb52370134de9d720cefec13863f Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Mon, 27 Jun 2022 17:42:21 -0400 Subject: [PATCH] internal/lsp/cache: avoid Handle mechanism for workspace dir This change causes (*snapshot).getWorkspaceDir to create a temporary directory directly, rather than via the Store/Generation/Handle mechanism. The work is done at most once per snapshot, and the directory is deleted in Snapshot.Destroy. This removes the last remaining use of Handle's cleanup mechanism, which will be deleted in a follow-up. Change-Id: I32f09a67846d9b5577cb8849b226427f86443303 Reviewed-on: https://go-review.googlesource.com/c/tools/+/414499 gopls-CI: kokoro Run-TryBot: Alan Donovan Reviewed-by: Robert Findley TryBot-Result: Gopher Robot --- internal/lsp/cache/load.go | 86 ++++++++++++---------------------- internal/lsp/cache/snapshot.go | 18 ++++--- 2 files changed, 41 insertions(+), 63 deletions(-) diff --git a/internal/lsp/cache/load.go b/internal/lsp/cache/load.go index 3fb67a7a98..d613dc3337 100644 --- a/internal/lsp/cache/load.go +++ b/internal/lsp/cache/load.go @@ -7,7 +7,6 @@ package cache import ( "bytes" "context" - "crypto/sha256" "errors" "fmt" "io/ioutil" @@ -24,7 +23,6 @@ import ( "golang.org/x/tools/internal/lsp/debug/tag" "golang.org/x/tools/internal/lsp/protocol" "golang.org/x/tools/internal/lsp/source" - "golang.org/x/tools/internal/memoize" "golang.org/x/tools/internal/packagesinternal" "golang.org/x/tools/internal/span" ) @@ -384,77 +382,53 @@ func (s *snapshot) applyCriticalErrorToFiles(ctx context.Context, msg string, fi return srcDiags } -type workspaceDirKey string - -type workspaceDirData struct { - dir string - err error -} - -// getWorkspaceDir gets the URI for the workspace directory associated with -// this snapshot. The workspace directory is a temp directory containing the -// go.mod file computed from all active modules. +// getWorkspaceDir returns the URI for the workspace directory +// associated with this snapshot. The workspace directory is a +// temporary directory containing the go.mod file computed from all +// active modules. func (s *snapshot) getWorkspaceDir(ctx context.Context) (span.URI, error) { s.mu.Lock() - h := s.workspaceDirHandle + dir, err := s.workspaceDir, s.workspaceDirErr s.mu.Unlock() - if h != nil { - return getWorkspaceDir(ctx, h, s.generation) + if dir == "" && err == nil { // cache miss + dir, err = makeWorkspaceDir(ctx, s.workspace, s) + s.mu.Lock() + s.workspaceDir, s.workspaceDirErr = dir, err + s.mu.Unlock() } - file, err := s.workspace.modFile(ctx, s) + return span.URIFromPath(dir), err +} + +// makeWorkspaceDir creates a temporary directory containing a go.mod +// and go.sum file for each module in the workspace. +// Note: snapshot's mutex must be unlocked for it to satisfy FileSource. +func makeWorkspaceDir(ctx context.Context, workspace *workspace, fs source.FileSource) (string, error) { + file, err := workspace.modFile(ctx, fs) if err != nil { return "", err } - hash := sha256.New() modContent, err := file.Format() if err != nil { return "", err } - sumContent, err := s.workspace.sumFile(ctx, s) + sumContent, err := workspace.sumFile(ctx, fs) if err != nil { return "", err } - hash.Write(modContent) - hash.Write(sumContent) - key := workspaceDirKey(hash.Sum(nil)) - s.mu.Lock() - h = s.generation.Bind(key, func(context.Context, memoize.Arg) interface{} { - tmpdir, err := ioutil.TempDir("", "gopls-workspace-mod") - if err != nil { - return &workspaceDirData{err: err} - } - - for name, content := range map[string][]byte{ - "go.mod": modContent, - "go.sum": sumContent, - } { - filename := filepath.Join(tmpdir, name) - if err := ioutil.WriteFile(filename, content, 0644); err != nil { - os.RemoveAll(tmpdir) - return &workspaceDirData{err: err} - } - } - - return &workspaceDirData{dir: tmpdir} - }, func(v interface{}) { - d := v.(*workspaceDirData) - if d.dir != "" { - if err := os.RemoveAll(d.dir); err != nil { - event.Error(context.Background(), "cleaning workspace dir", err) - } - } - }) - s.workspaceDirHandle = h - s.mu.Unlock() - return getWorkspaceDir(ctx, h, s.generation) -} - -func getWorkspaceDir(ctx context.Context, h *memoize.Handle, g *memoize.Generation) (span.URI, error) { - v, err := h.Get(ctx, g, nil) + tmpdir, err := ioutil.TempDir("", "gopls-workspace-mod") if err != nil { return "", err } - return span.URIFromPath(v.(*workspaceDirData).dir), nil + for name, content := range map[string][]byte{ + "go.mod": modContent, + "go.sum": sumContent, + } { + if err := ioutil.WriteFile(filepath.Join(tmpdir, name), content, 0644); err != nil { + os.RemoveAll(tmpdir) // ignore error + return "", err + } + } + return tmpdir, nil } // computeMetadataUpdates populates the updates map with metadata updates to diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go index 39e958e0fc..259345bdc8 100644 --- a/internal/lsp/cache/snapshot.go +++ b/internal/lsp/cache/snapshot.go @@ -115,8 +115,11 @@ type snapshot struct { modTidyHandles map[span.URI]*modTidyHandle modWhyHandles map[span.URI]*modWhyHandle - workspace *workspace - workspaceDirHandle *memoize.Handle + workspace *workspace // (not guarded by mu) + + // The cached result of makeWorkspaceDir, created on demand and deleted by Snapshot.Destroy. + workspaceDir string + workspaceDirErr error // knownSubdirs is the set of subdirectories in the workspace, used to // create glob patterns for file watching. @@ -144,6 +147,12 @@ func (s *snapshot) Destroy(destroyedBy string) { s.files.Destroy() s.goFiles.Destroy() s.parseKeysByURI.Destroy() + + if s.workspaceDir != "" { + if err := os.RemoveAll(s.workspaceDir); err != nil { + event.Error(context.Background(), "cleaning workspace dir", err) + } + } } func (s *snapshot) ID() uint64 { @@ -1709,11 +1718,6 @@ func (s *snapshot) clone(ctx, bgCtx context.Context, changes map[span.URI]*fileC workspace: newWorkspace, } - if !workspaceChanged && s.workspaceDirHandle != nil { - result.workspaceDirHandle = s.workspaceDirHandle - newGen.Inherit(s.workspaceDirHandle) - } - // Copy all of the FileHandles. for k, v := range s.symbols { if change, ok := changes[k]; ok {