internal/lsp/cache: use GetHandle not Bind for 5 URI-keyed maps

This change replaces the 5 remaining calls to Bind (generational
lifetime) with GetHandle (reference counting). The handles
are now stored in persistent.Maps, which simplifies the
invalidation logic.

All 5 have span.URIs as keys:
  symbolizeHandles
  parse{Mod,Work}Handles
  mod{Tidy,Why}Handles

Also, factor the functions that use these maps to have a common form:
- a fooImpl function that returns an R result and an error;
- a foo wrapper that decorates it with caching.
- a local fooResult type, defined struct{R; error} that is the cache entry.
The functions for getting/setting map entries are all inlined.
The fooHandle types are all replaced by *memoize.Handle, now that
their use is local.

No behavior change is intended.

The other uses of Bind are deleted in these CLs:
https://go-review.googlesource.com/c/tools/+/415975 (astCacheData)
https://go-review.googlesource.com/c/tools/+/415504 (actions)

Change-Id: I77cc4e828936fe171152ca13a12f7a639299e9e5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/415976
Auto-Submit: Alan Donovan <adonovan@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>

6 files changed, 397 insertions, 456 deletions

diff --git a/internal/lsp/cache/maps.go b/internal/lsp/cache/maps.go
index f8e03057c..1ec341515 100644
--- a/internal/lsp/cache/maps.go
+++ b/internal/lsp/cache/maps.go
@@ -16,11 +16,14 @@ type filesMap struct {
 	impl *persistent.Map
 }
 
+// uriLessInterface is the < relation for "any" values containing span.URIs.
+func uriLessInterface(a, b interface{}) bool {
+	return a.(span.URI) < b.(span.URI)
+}
+
 func newFilesMap() filesMap {
 	return filesMap{
-		impl: persistent.NewMap(func(a, b interface{}) bool {
-			return a.(span.URI) < b.(span.URI)
-		}),
+		impl: persistent.NewMap(uriLessInterface),
 	}
 }
 
@@ -152,9 +155,7 @@ type parseKeysByURIMap struct {
 
 func newParseKeysByURIMap() parseKeysByURIMap {
 	return parseKeysByURIMap{
-		impl: persistent.NewMap(func(a, b interface{}) bool {
-			return a.(span.URI) < b.(span.URI)
-		}),
+		impl: persistent.NewMap(uriLessInterface),
 	}
 }
 
diff --git a/internal/lsp/cache/mod.go b/internal/lsp/cache/mod.go
index 843919d7b..1963feea5 100644
--- a/internal/lsp/cache/mod.go
+++ b/internal/lsp/cache/mod.go
@@ -24,152 +24,156 @@ import (
 	"golang.org/x/tools/internal/span"
 )
 
-type parseModHandle struct {
-	handle *memoize.Handle
-}
+// ParseMod parses a go.mod file, using a cache. It may return partial results and an error.
+func (s *snapshot) ParseMod(ctx context.Context, fh source.FileHandle) (*source.ParsedModule, error) {
+	uri := fh.URI()
 
-type parseModData struct {
-	parsed *source.ParsedModule
+	s.mu.Lock()
+	entry, hit := s.parseModHandles.Get(uri)
+	s.mu.Unlock()
 
-	// err is any error encountered while parsing the file.
-	err error
-}
+	type parseModResult struct {
+		parsed *source.ParsedModule
+		err    error
+	}
 
-func (mh *parseModHandle) parse(ctx context.Context, snapshot *snapshot) (*source.ParsedModule, error) {
-	v, err := mh.handle.Get(ctx, snapshot.generation, snapshot)
+	// cache miss?
+	if !hit {
+		handle, release := s.generation.GetHandle(fh.FileIdentity(), func(ctx context.Context, _ memoize.Arg) interface{} {
+			parsed, err := parseModImpl(ctx, fh)
+			return parseModResult{parsed, err}
+		})
+
+		entry = handle
+		s.mu.Lock()
+		s.parseModHandles.Set(uri, entry, func(_, _ interface{}) { release() })
+		s.mu.Unlock()
+	}
+
+	// Await result.
+	v, err := entry.(*memoize.Handle).Get(ctx, s.generation, s)
 	if err != nil {
 		return nil, err
 	}
-	data := v.(*parseModData)
-	return data.parsed, data.err
+	res := v.(parseModResult)
+	return res.parsed, res.err
 }
 
-func (s *snapshot) ParseMod(ctx context.Context, modFH source.FileHandle) (*source.ParsedModule, error) {
-	if handle := s.getParseModHandle(modFH.URI()); handle != nil {
-		return handle.parse(ctx, s)
-	}
-	h := s.generation.Bind(modFH.FileIdentity(), func(ctx context.Context, _ memoize.Arg) interface{} {
-		_, done := event.Start(ctx, "cache.ParseModHandle", tag.URI.Of(modFH.URI()))
-		defer done()
+// parseModImpl parses the go.mod file whose name and contents are in fh.
+// It may return partial results and an error.
+func parseModImpl(ctx context.Context, fh source.FileHandle) (*source.ParsedModule, error) {
+	_, done := event.Start(ctx, "cache.ParseMod", tag.URI.Of(fh.URI()))
+	defer done()
 
-		contents, err := modFH.Read()
-		if err != nil {
-			return &parseModData{err: err}
-		}
-		m := protocol.NewColumnMapper(modFH.URI(), contents)
-		file, parseErr := modfile.Parse(modFH.URI().Filename(), contents, nil)
-		// Attempt to convert the error to a standardized parse error.
-		var parseErrors []*source.Diagnostic
-		if parseErr != nil {
-			mfErrList, ok := parseErr.(modfile.ErrorList)
-			if !ok {
-				return &parseModData{err: fmt.Errorf("unexpected parse error type %v", parseErr)}
-			}
-			for _, mfErr := range mfErrList {
-				rng, err := rangeFromPositions(m, mfErr.Pos, mfErr.Pos)
-				if err != nil {
-					return &parseModData{err: err}
-				}
-				parseErrors = append(parseErrors, &source.Diagnostic{
-					URI:      modFH.URI(),
-					Range:    rng,
-					Severity: protocol.SeverityError,
-					Source:   source.ParseError,
-					Message:  mfErr.Err.Error(),
-				})
+	contents, err := fh.Read()
+	if err != nil {
+		return nil, err
+	}
+	m := protocol.NewColumnMapper(fh.URI(), contents)
+	file, parseErr := modfile.Parse(fh.URI().Filename(), contents, nil)
+	// Attempt to convert the error to a standardized parse error.
+	var parseErrors []*source.Diagnostic
+	if parseErr != nil {
+		mfErrList, ok := parseErr.(modfile.ErrorList)
+		if !ok {
+			return nil, fmt.Errorf("unexpected parse error type %v", parseErr)
+		}
+		for _, mfErr := range mfErrList {
+			rng, err := rangeFromPositions(m, mfErr.Pos, mfErr.Pos)
+			if err != nil {
+				return nil, err
 			}
+			parseErrors = append(parseErrors, &source.Diagnostic{
+				URI:      fh.URI(),
+				Range:    rng,
+				Severity: protocol.SeverityError,
+				Source:   source.ParseError,
+				Message:  mfErr.Err.Error(),
+			})
 		}
-		return &parseModData{
-			parsed: &source.ParsedModule{
-				URI:         modFH.URI(),
-				Mapper:      m,
-				File:        file,
-				ParseErrors: parseErrors,
-			},
-			err: parseErr,
-		}
-	})
+	}
+	return &source.ParsedModule{
+		URI:         fh.URI(),
+		Mapper:      m,
+		File:        file,
+		ParseErrors: parseErrors,
+	}, parseErr
+}
+
+// ParseWork parses a go.work file, using a cache. It may return partial results and an error.
+// TODO(adonovan): move to new work.go file.
+func (s *snapshot) ParseWork(ctx context.Context, fh source.FileHandle) (*source.ParsedWorkFile, error) {
+	uri := fh.URI()
 
-	pmh := &parseModHandle{handle: h}
 	s.mu.Lock()
-	s.parseModHandles[modFH.URI()] = pmh
+	entry, hit := s.parseWorkHandles.Get(uri)
 	s.mu.Unlock()
 
-	return pmh.parse(ctx, s)
-}
-
-type parseWorkHandle struct {
-	handle *memoize.Handle
-}
+	type parseWorkResult struct {
+		parsed *source.ParsedWorkFile
+		err    error
+	}
 
-type parseWorkData struct {
-	parsed *source.ParsedWorkFile
+	// cache miss?
+	if !hit {
+		handle, release := s.generation.GetHandle(fh.FileIdentity(), func(ctx context.Context, _ memoize.Arg) interface{} {
+			parsed, err := parseWorkImpl(ctx, fh)
+			return parseWorkResult{parsed, err}
+		})
 
-	// err is any error encountered while parsing the file.
-	err error
-}
+		entry = handle
+		s.mu.Lock()
+		s.parseWorkHandles.Set(uri, entry, func(_, _ interface{}) { release() })
+		s.mu.Unlock()
+	}
 
-func (mh *parseWorkHandle) parse(ctx context.Context, snapshot *snapshot) (*source.ParsedWorkFile, error) {
-	v, err := mh.handle.Get(ctx, snapshot.generation, snapshot)
+	// Await result.
+	v, err := entry.(*memoize.Handle).Get(ctx, s.generation, s)
 	if err != nil {
 		return nil, err
 	}
-	data := v.(*parseWorkData)
-	return data.parsed, data.err
+	res := v.(parseWorkResult)
+	return res.parsed, res.err
 }
 
-func (s *snapshot) ParseWork(ctx context.Context, modFH source.FileHandle) (*source.ParsedWorkFile, error) {
-	if handle := s.getParseWorkHandle(modFH.URI()); handle != nil {
-		return handle.parse(ctx, s)
-	}
-	h := s.generation.Bind(modFH.FileIdentity(), func(ctx context.Context, _ memoize.Arg) interface{} {
-		_, done := event.Start(ctx, "cache.ParseModHandle", tag.URI.Of(modFH.URI()))
-		defer done()
+// parseWorkImpl parses a go.work file. It may return partial results and an error.
+func parseWorkImpl(ctx context.Context, fh source.FileHandle) (*source.ParsedWorkFile, error) {
+	_, done := event.Start(ctx, "cache.ParseWork", tag.URI.Of(fh.URI()))
+	defer done()
 
-		contents, err := modFH.Read()
-		if err != nil {
-			return &parseWorkData{err: err}
-		}
-		m := protocol.NewColumnMapper(modFH.URI(), contents)
-		file, parseErr := modfile.ParseWork(modFH.URI().Filename(), contents, nil)
-		// Attempt to convert the error to a standardized parse error.
-		var parseErrors []*source.Diagnostic
-		if parseErr != nil {
-			mfErrList, ok := parseErr.(modfile.ErrorList)
-			if !ok {
-				return &parseWorkData{err: fmt.Errorf("unexpected parse error type %v", parseErr)}
-			}
-			for _, mfErr := range mfErrList {
-				rng, err := rangeFromPositions(m, mfErr.Pos, mfErr.Pos)
-				if err != nil {
-					return &parseWorkData{err: err}
-				}
-				parseErrors = append(parseErrors, &source.Diagnostic{
-					URI:      modFH.URI(),
-					Range:    rng,
-					Severity: protocol.SeverityError,
-					Source:   source.ParseError,
-					Message:  mfErr.Err.Error(),
-				})
+	contents, err := fh.Read()
+	if err != nil {
+		return nil, err
+	}
+	m := protocol.NewColumnMapper(fh.URI(), contents)
+	file, parseErr := modfile.ParseWork(fh.URI().Filename(), contents, nil)
+	// Attempt to convert the error to a standardized parse error.
+	var parseErrors []*source.Diagnostic
+	if parseErr != nil {
+		mfErrList, ok := parseErr.(modfile.ErrorList)
+		if !ok {
+			return nil, fmt.Errorf("unexpected parse error type %v", parseErr)
+		}
+		for _, mfErr := range mfErrList {
+			rng, err := rangeFromPositions(m, mfErr.Pos, mfErr.Pos)
+			if err != nil {
+				return nil, err
 			}
+			parseErrors = append(parseErrors, &source.Diagnostic{
+				URI:      fh.URI(),
+				Range:    rng,
+				Severity: protocol.SeverityError,
+				Source:   source.ParseError,
+				Message:  mfErr.Err.Error(),
+			})
 		}
-		return &parseWorkData{
-			parsed: &source.ParsedWorkFile{
-				URI:         modFH.URI(),
-				Mapper:      m,
-				File:        file,
-				ParseErrors: parseErrors,
-			},
-			err: parseErr,
-		}
-	})
-
-	pwh := &parseWorkHandle{handle: h}
-	s.mu.Lock()
-	s.parseWorkHandles[modFH.URI()] = pwh
-	s.mu.Unlock()
-
-	return pwh.parse(ctx, s)
+	}
+	return &source.ParsedWorkFile{
+		URI:         fh.URI(),
+		Mapper:      m,
+		File:        file,
+		ParseErrors: parseErrors,
+	}, parseErr
 }
 
 // goSum reads the go.sum file for the go.mod file at modURI, if it exists. If
@@ -198,104 +202,100 @@ func sumFilename(modURI span.URI) string {
 	return strings.TrimSuffix(modURI.Filename(), ".mod") + ".sum"
 }
 
-// modKey is uniquely identifies cached data for `go mod why` or dependencies
-// to upgrade.
-type modKey struct {
-	sessionID string
-	env       source.Hash
-	view      string
-	mod       source.FileIdentity
-	verb      modAction
-}
+// ModWhy returns the "go mod why" result for each module named in a
+// require statement in the go.mod file.
+// TODO(adonovan): move to new mod_why.go file.
+func (s *snapshot) ModWhy(ctx context.Context, fh source.FileHandle) (map[string]string, error) {
+	uri := fh.URI()
 
-type modAction int
+	if s.View().FileKind(fh) != source.Mod {
+		return nil, fmt.Errorf("%s is not a go.mod file", uri)
+	}
 
-const (
-	why modAction = iota
-	upgrade
-)
+	s.mu.Lock()
+	entry, hit := s.modWhyHandles.Get(uri)
+	s.mu.Unlock()
 
-type modWhyHandle struct {
-	handle *memoize.Handle
-}
+	type modWhyResult struct {
+		why map[string]string
+		err error
+	}
 
-type modWhyData struct {
-	// why keeps track of the `go mod why` results for each require statement
-	// in the go.mod file.
-	why map[string]string
+	// cache miss?
+	if !hit {
+		// TODO(adonovan): use a simpler cache of promises that
+		// is shared across snapshots. See comment at modTidyKey.
+		type modWhyKey struct {
+			// TODO(rfindley): is sessionID used to identify overlays because modWhy
+			// looks at overlay state? In that case, I am not sure that this key
+			// is actually correct. The key should probably just be URI, and
+			// invalidated in clone when any import changes.
+			sessionID string
+			env       source.Hash
+			view      string
+			mod       source.FileIdentity
+		}
+		key := modWhyKey{
+			sessionID: s.view.session.id,
+			env:       hashEnv(s),
+			mod:       fh.FileIdentity(),
+			view:      s.view.rootURI.Filename(),
+		}
+		handle, release := s.generation.GetHandle(key, func(ctx context.Context, arg memoize.Arg) interface{} {
+			why, err := modWhyImpl(ctx, arg.(*snapshot), fh)
+			return modWhyResult{why, err}
+		})
 
-	err error
-}
+		entry = handle
+		s.mu.Lock()
+		s.modWhyHandles.Set(uri, entry, func(_, _ interface{}) { release() })
+		s.mu.Unlock()
+	}
 
-func (mwh *modWhyHandle) why(ctx context.Context, snapshot *snapshot) (map[string]string, error) {
-	v, err := mwh.handle.Get(ctx, snapshot.generation, snapshot)
+	// Await result.
+	v, err := entry.(*memoize.Handle).Get(ctx, s.generation, s)
 	if err != nil {
 		return nil, err
 	}
-	data := v.(*modWhyData)
-	return data.why, data.err
+	res := v.(modWhyResult)
+	return res.why, res.err
 }
 
-func (s *snapshot) ModWhy(ctx context.Context, fh source.FileHandle) (map[string]string, error) {
-	if s.View().FileKind(fh) != source.Mod {
-		return nil, fmt.Errorf("%s is not a go.mod file", fh.URI())
+// modWhyImpl returns the result of "go mod why -m" on the specified go.mod file.
+func modWhyImpl(ctx context.Context, snapshot *snapshot, fh source.FileHandle) (map[string]string, error) {
+	ctx, done := event.Start(ctx, "cache.ModWhy", tag.URI.Of(fh.URI()))
+	defer done()
+
+	pm, err := snapshot.ParseMod(ctx, fh)
+	if err != nil {
+		return nil, err
 	}
-	if handle := s.getModWhyHandle(fh.URI()); handle != nil {
-		return handle.why(ctx, s)
+	// No requires to explain.
+	if len(pm.File.Require) == 0 {
+		return nil, nil // empty result
 	}
-	key := modKey{
-		sessionID: s.view.session.id,
-		env:       hashEnv(s),
-		mod:       fh.FileIdentity(),
-		view:      s.view.rootURI.Filename(),
-		verb:      why,
+	// Run `go mod why` on all the dependencies.
+	inv := &gocommand.Invocation{
+		Verb:       "mod",
+		Args:       []string{"why", "-m"},
+		WorkingDir: filepath.Dir(fh.URI().Filename()),
 	}
-	h := s.generation.Bind(key, func(ctx context.Context, arg memoize.Arg) interface{} {
-		ctx, done := event.Start(ctx, "cache.ModWhyHandle", tag.URI.Of(fh.URI()))
-		defer done()
-
-		snapshot := arg.(*snapshot)
-
-		pm, err := snapshot.ParseMod(ctx, fh)
-		if err != nil {
-			return &modWhyData{err: err}
-		}
-		// No requires to explain.
-		if len(pm.File.Require) == 0 {
-			return &modWhyData{}
-		}
-		// Run `go mod why` on all the dependencies.
-		inv := &gocommand.Invocation{
-			Verb:       "mod",
-			Args:       []string{"why", "-m"},
-			WorkingDir: filepath.Dir(fh.URI().Filename()),
-		}
-		for _, req := range pm.File.Require {
-			inv.Args = append(inv.Args, req.Mod.Path)
-		}
-		stdout, err := snapshot.RunGoCommandDirect(ctx, source.Normal, inv)
-		if err != nil {
-			return &modWhyData{err: err}
-		}
-		whyList := strings.Split(stdout.String(), "\n\n")
-		if len(whyList) != len(pm.File.Require) {
-			return &modWhyData{
-				err: fmt.Errorf("mismatched number of results: got %v, want %v", len(whyList), len(pm.File.Require)),
-			}
-		}
-		why := make(map[string]string, len(pm.File.Require))
-		for i, req := range pm.File.Require {
-			why[req.Mod.Path] = whyList[i]
-		}
-		return &modWhyData{why: why}
-	})
-
-	mwh := &modWhyHandle{handle: h}
-	s.mu.Lock()
-	s.modWhyHandles[fh.URI()] = mwh
-	s.mu.Unlock()
-
-	return mwh.why(ctx, s)
+	for _, req := range pm.File.Require {
+		inv.Args = append(inv.Args, req.Mod.Path)
+	}
+	stdout, err := snapshot.RunGoCommandDirect(ctx, source.Normal, inv)
+	if err != nil {
+		return nil, err
+	}
+	whyList := strings.Split(stdout.String(), "\n\n")
+	if len(whyList) != len(pm.File.Require) {
+		return nil, fmt.Errorf("mismatched number of results: got %v, want %v", len(whyList), len(pm.File.Require))
+	}
+	why := make(map[string]string, len(pm.File.Require))
+	for i, req := range pm.File.Require {
+		why[req.Mod.Path] = whyList[i]
+	}
+	return why, nil
 }
 
 // extractGoCommandError tries to parse errors that come from the go command
diff --git a/internal/lsp/cache/mod_tidy.go b/internal/lsp/cache/mod_tidy.go
index 913946595..84f369ef3 100644
--- a/internal/lsp/cache/mod_tidy.go
+++ b/internal/lsp/cache/mod_tidy.go
@@ -28,125 +28,139 @@ import (
 	"golang.org/x/tools/internal/span"
 )
 
-type modTidyKey struct {
-	sessionID       string
-	env             source.Hash
-	gomod           source.FileIdentity
-	imports         source.Hash
-	unsavedOverlays source.Hash
-	view            string
-}
+// modTidyImpl runs "go mod tidy" on a go.mod file, using a cache.
+//
+// REVIEWERS: what does it mean to cache an operation that has side effects?
+// Or are we de-duplicating operations in flight on the same file?
+func (s *snapshot) ModTidy(ctx context.Context, pm *source.ParsedModule) (*source.TidiedModule, error) {
+	uri := pm.URI
+	if pm.File == nil {
+		return nil, fmt.Errorf("cannot tidy unparseable go.mod file: %v", uri)
+	}
 
-type modTidyHandle struct {
-	handle *memoize.Handle
-}
+	s.mu.Lock()
+	entry, hit := s.modTidyHandles.Get(uri)
+	s.mu.Unlock()
 
-type modTidyData struct {
-	tidied *source.TidiedModule
-	err    error
-}
+	type modTidyResult struct {
+		tidied *source.TidiedModule
+		err    error
+	}
+
+	// Cache miss?
+	if !hit {
+		fh, err := s.GetFile(ctx, pm.URI)
+		if err != nil {
+			return nil, err
+		}
+		// If the file handle is an overlay, it may not be written to disk.
+		// The go.mod file has to be on disk for `go mod tidy` to work.
+		// TODO(rfindley): is this still true with Go 1.16 overlay support?
+		if _, ok := fh.(*overlay); ok {
+			if info, _ := os.Stat(fh.URI().Filename()); info == nil {
+				return nil, source.ErrNoModOnDisk
+			}
+		}
+		if criticalErr := s.GetCriticalError(ctx); criticalErr != nil {
+			return &source.TidiedModule{
+				Diagnostics: criticalErr.DiagList,
+			}, nil
+		}
+		workspacePkgs, err := s.workspacePackageHandles(ctx)
+		if err != nil {
+			return nil, err
+		}
+
+		s.mu.Lock()
+		overlayHash := hashUnsavedOverlays(s.files)
+		s.mu.Unlock()
+
+		// There's little reason at to use the shared cache for mod
+		// tidy (and mod why) as their key includes the view and session.
+		// TODO(adonovan): use a simpler cache of promises that
+		// is shared across snapshots.
+		type modTidyKey struct {
+			// TODO(rfindley): this key is also suspicious (see modWhyKey).
+			sessionID       string
+			env             source.Hash
+			gomod           source.FileIdentity
+			imports         source.Hash
+			unsavedOverlays source.Hash
+			view            string
+		}
+		key := modTidyKey{
+			sessionID:       s.view.session.id,
+			view:            s.view.folder.Filename(),
+			imports:         s.hashImports(ctx, workspacePkgs),
+			unsavedOverlays: overlayHash,
+			gomod:           fh.FileIdentity(),
+			env:             hashEnv(s),
+		}
+		handle, release := s.generation.GetHandle(key, func(ctx context.Context, arg memoize.Arg) interface{} {
+			tidied, err := modTidyImpl(ctx, arg.(*snapshot), fh, pm, workspacePkgs)
+			return modTidyResult{tidied, err}
+		})
 
-func (mth *modTidyHandle) tidy(ctx context.Context, snapshot *snapshot) (*source.TidiedModule, error) {
-	v, err := mth.handle.Get(ctx, snapshot.generation, snapshot)
+		entry = handle
+		s.mu.Lock()
+		s.modTidyHandles.Set(uri, entry, func(_, _ interface{}) { release() })
+		s.mu.Unlock()
+	}
+
+	// Await result.
+	v, err := entry.(*memoize.Handle).Get(ctx, s.generation, s)
 	if err != nil {
 		return nil, err
 	}
-	data := v.(*modTidyData)
-	return data.tidied, data.err
+	res := v.(modTidyResult)
+	return res.tidied, res.err
 }
 
-func (s *snapshot) ModTidy(ctx context.Context, pm *source.ParsedModule) (*source.TidiedModule, error) {
-	if pm.File == nil {
-		return nil, fmt.Errorf("cannot tidy unparseable go.mod file: %v", pm.URI)
-	}
-	if handle := s.getModTidyHandle(pm.URI); handle != nil {
-		return handle.tidy(ctx, s)
+// modTidyImpl runs "go mod tidy" on a go.mod file.
+func modTidyImpl(ctx context.Context, snapshot *snapshot, fh source.FileHandle, pm *source.ParsedModule, workspacePkgs []*packageHandle) (*source.TidiedModule, error) {
+	ctx, done := event.Start(ctx, "cache.ModTidy", tag.URI.Of(fh.URI()))
+	defer done()
+
+	inv := &gocommand.Invocation{
+		Verb:       "mod",
+		Args:       []string{"tidy"},
+		WorkingDir: filepath.Dir(fh.URI().Filename()),
 	}
-	fh, err := s.GetFile(ctx, pm.URI)
+	tmpURI, inv, cleanup, err := snapshot.goCommandInvocation(ctx, source.WriteTemporaryModFile, inv)
 	if err != nil {
 		return nil, err
 	}
-	// If the file handle is an overlay, it may not be written to disk.
-	// The go.mod file has to be on disk for `go mod tidy` to work.
-	if _, ok := fh.(*overlay); ok {
-		if info, _ := os.Stat(fh.URI().Filename()); info == nil {
-			return nil, source.ErrNoModOnDisk
-		}
+	// Keep the temporary go.mod file around long enough to parse it.
+	defer cleanup()
+
+	if _, err := snapshot.view.session.gocmdRunner.Run(ctx, *inv); err != nil {
+		return nil, err
 	}
-	if criticalErr := s.GetCriticalError(ctx); criticalErr != nil {
-		return &source.TidiedModule{
-			Diagnostics: criticalErr.DiagList,
-		}, nil
+
+	// Go directly to disk to get the temporary mod file,
+	// since it is always on disk.
+	tempContents, err := ioutil.ReadFile(tmpURI.Filename())
+	if err != nil {
+		return nil, err
 	}
-	workspacePkgs, err := s.workspacePackageHandles(ctx)
+	ideal, err := modfile.Parse(tmpURI.Filename(), tempContents, nil)
 	if err != nil {
+		// We do not need to worry about the temporary file's parse errors
+		// since it has been "tidied".
 		return nil, err
 	}
 
-	s.mu.Lock()
-	overlayHash := hashUnsavedOverlays(s.files)
-	s.mu.Unlock()
-
-	key := modTidyKey{
-		sessionID:       s.view.session.id,
-		view:            s.view.folder.Filename(),
-		imports:         s.hashImports(ctx, workspacePkgs),
-		unsavedOverlays: overlayHash,
-		gomod:           fh.FileIdentity(),
-		env:             hashEnv(s),
-	}
-	h := s.generation.Bind(key, func(ctx context.Context, arg memoize.Arg) interface{} {
-		ctx, done := event.Start(ctx, "cache.ModTidyHandle", tag.URI.Of(fh.URI()))
-		defer done()
-
-		snapshot := arg.(*snapshot)
-		inv := &gocommand.Invocation{
-			Verb:       "mod",
-			Args:       []string{"tidy"},
-			WorkingDir: filepath.Dir(fh.URI().Filename()),
-		}
-		tmpURI, inv, cleanup, err := snapshot.goCommandInvocation(ctx, source.WriteTemporaryModFile, inv)
-		if err != nil {
-			return &modTidyData{err: err}
-		}
-		// Keep the temporary go.mod file around long enough to parse it.
-		defer cleanup()
-
-		if _, err := s.view.session.gocmdRunner.Run(ctx, *inv); err != nil {
-			return &modTidyData{err: err}
-		}
-		// Go directly to disk to get the temporary mod file, since it is
-		// always on disk.
-		tempContents, err := ioutil.ReadFile(tmpURI.Filename())
-		if err != nil {
-			return &modTidyData{err: err}
-		}
-		ideal, err := modfile.Parse(tmpURI.Filename(), tempContents, nil)
-		if err != nil {
-			// We do not need to worry about the temporary file's parse errors
-			// since it has been "tidied".
-			return &modTidyData{err: err}
-		}
-		// Compare the original and tidied go.mod files to compute errors and
-		// suggested fixes.
-		diagnostics, err := modTidyDiagnostics(ctx, snapshot, pm, ideal, workspacePkgs)
-		if err != nil {
-			return &modTidyData{err: err}
-		}
-		return &modTidyData{
-			tidied: &source.TidiedModule{
-				Diagnostics:   diagnostics,
-				TidiedContent: tempContents,
-			},
-		}
-	})
-
-	mth := &modTidyHandle{handle: h}
-	s.mu.Lock()
-	s.modTidyHandles[fh.URI()] = mth
-	s.mu.Unlock()
+	// Compare the original and tidied go.mod files to compute errors and
+	// suggested fixes.
+	diagnostics, err := modTidyDiagnostics(ctx, snapshot, pm, ideal, workspacePkgs)
+	if err != nil {
+		return nil, err
+	}
 
-	return mth.tidy(ctx, s)
+	return &source.TidiedModule{
+		Diagnostics:   diagnostics,
+		TidiedContent: tempContents,
+	}, nil
 }
 
 func (s *snapshot) hashImports(ctx context.Context, wsPackages []*packageHandle) source.Hash {
diff --git a/internal/lsp/cache/session.go b/internal/lsp/cache/session.go
index 98d3c2504..80468bc59 100644
--- a/internal/lsp/cache/session.go
+++ b/internal/lsp/cache/session.go
@@ -238,14 +238,14 @@ func (s *Session) createView(ctx context.Context, name string, folder span.URI,
 		isActivePackageCache: newIsActivePackageCacheMap(),
 		goFiles:              newGoFilesMap(),
 		parseKeysByURI:       newParseKeysByURIMap(),
-		symbols:              make(map[span.URI]*symbolHandle),
+		symbolizeHandles:     persistent.NewMap(uriLessInterface),
 		actions:              persistent.NewMap(actionKeyLessInterface),
 		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),
+		parseModHandles:      persistent.NewMap(uriLessInterface),
+		parseWorkHandles:     persistent.NewMap(uriLessInterface),
+		modTidyHandles:       persistent.NewMap(uriLessInterface),
+		modWhyHandles:        persistent.NewMap(uriLessInterface),
 		knownSubdirs:         newKnownDirsSet(),
 		workspace:            workspace,
 	}
diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go
index 93316653a..b962435b6 100644
--- a/internal/lsp/cache/snapshot.go
+++ b/internal/lsp/cache/snapshot.go
@@ -85,8 +85,9 @@ type snapshot struct {
 	goFiles        goFilesMap
 	parseKeysByURI parseKeysByURIMap
 
-	// TODO(rfindley): consider merging this with files to reduce burden on clone.
-	symbols map[span.URI]*symbolHandle
+	// symbolizeHandles maps each file URI to a handle for the future
+	// result of computing the symbols declared in that file.
+	symbolizeHandles *persistent.Map // from span.URI to *memoize.Handle
 
 	// packages maps a packageKey to a *packageHandle.
 	// It may be invalidated when a file's content changes.
@@ -109,17 +110,17 @@ type snapshot struct {
 
 	// parseModHandles keeps track of any parseModHandles for the snapshot.
 	// The handles need not refer to only the view's go.mod file.
-	parseModHandles map[span.URI]*parseModHandle
+	parseModHandles *persistent.Map // from span.URI to *memoize.Handle
 
 	// parseWorkHandles keeps track of any parseWorkHandles for the snapshot.
 	// The handles need not refer to only the view's go.work file.
-	parseWorkHandles map[span.URI]*parseWorkHandle
+	parseWorkHandles *persistent.Map // from span.URI to *memoize.Handle
 
 	// Preserve go.mod-related handles to avoid garbage-collecting the results
 	// of various calls to the go command. The handles need not refer to only
 	// the view's go.mod file.
-	modTidyHandles map[span.URI]*modTidyHandle
-	modWhyHandles  map[span.URI]*modWhyHandle
+	modTidyHandles *persistent.Map // from span.URI to *memoize.Handle
+	modWhyHandles  *persistent.Map // from span.URI to *memoize.Handle
 
 	workspace *workspace // (not guarded by mu)
 
@@ -156,6 +157,11 @@ func (s *snapshot) Destroy(destroyedBy string) {
 	s.goFiles.Destroy()
 	s.parseKeysByURI.Destroy()
 	s.knownSubdirs.Destroy()
+	s.symbolizeHandles.Destroy()
+	s.parseModHandles.Destroy()
+	s.parseWorkHandles.Destroy()
+	s.modTidyHandles.Destroy()
+	s.modWhyHandles.Destroy()
 
 	if s.workspaceDir != "" {
 		if err := os.RemoveAll(s.workspaceDir); err != nil {
@@ -700,30 +706,6 @@ func (s *snapshot) addGoFile(key parseKey, pgh *parseGoHandle, release func()) *
 	return pgh
 }
 
-func (s *snapshot) getParseModHandle(uri span.URI) *parseModHandle {
-	s.mu.Lock()
-	defer s.mu.Unlock()
-	return s.parseModHandles[uri]
-}
-
-func (s *snapshot) getParseWorkHandle(uri span.URI) *parseWorkHandle {
-	s.mu.Lock()
-	defer s.mu.Unlock()
-	return s.parseWorkHandles[uri]
-}
-
-func (s *snapshot) getModWhyHandle(uri span.URI) *modWhyHandle {
-	s.mu.Lock()
-	defer s.mu.Unlock()
-	return s.modWhyHandles[uri]
-}
-
-func (s *snapshot) getModTidyHandle(uri span.URI) *modTidyHandle {
-	s.mu.Lock()
-	defer s.mu.Unlock()
-	return s.modTidyHandles[uri]
-}
-
 func (s *snapshot) getImportedBy(id PackageID) []PackageID {
 	s.mu.Lock()
 	defer s.mu.Unlock()
@@ -1039,12 +1021,12 @@ func (s *snapshot) Symbols(ctx context.Context) map[span.URI][]source.Symbol {
 		iolimit <- struct{}{} // acquire token
 		group.Go(func() error {
 			defer func() { <-iolimit }() // release token
-			v, err := s.buildSymbolHandle(ctx, f).handle.Get(ctx, s.generation, s)
+			symbols, err := s.symbolize(ctx, f)
 			if err != nil {
 				return err
 			}
 			resultMu.Lock()
-			result[uri] = v.(*symbolData).symbols
+			result[uri] = symbols
 			resultMu.Unlock()
 			return nil
 		})
@@ -1159,26 +1141,6 @@ func (s *snapshot) getPackage(id PackageID, mode source.ParseMode) *packageHandl
 	return ph
 }
 
-func (s *snapshot) getSymbolHandle(uri span.URI) *symbolHandle {
-	s.mu.Lock()
-	defer s.mu.Unlock()
-
-	return s.symbols[uri]
-}
-
-func (s *snapshot) addSymbolHandle(uri span.URI, sh *symbolHandle) *symbolHandle {
-	s.mu.Lock()
-	defer s.mu.Unlock()
-
-	// If the package handle has already been cached,
-	// return the cached handle instead of overriding it.
-	if sh, ok := s.symbols[uri]; ok {
-		return sh
-	}
-	s.symbols[uri] = sh
-	return sh
-}
-
 func (s *snapshot) getActionHandle(id PackageID, m source.ParseMode, a *analysis.Analyzer) *actionHandle {
 	key := actionKey{
 		pkg: packageKey{
@@ -1732,42 +1694,23 @@ func (s *snapshot) clone(ctx, bgCtx context.Context, changes map[span.URI]*fileC
 		files:                s.files.Clone(),
 		goFiles:              s.goFiles.Clone(),
 		parseKeysByURI:       s.parseKeysByURI.Clone(),
-		symbols:              make(map[span.URI]*symbolHandle, len(s.symbols)),
+		symbolizeHandles:     s.symbolizeHandles.Clone(),
 		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)),
+		parseModHandles:      s.parseModHandles.Clone(),
+		parseWorkHandles:     s.parseWorkHandles.Clone(),
+		modTidyHandles:       s.modTidyHandles.Clone(),
+		modWhyHandles:        s.modWhyHandles.Clone(),
 		knownSubdirs:         s.knownSubdirs.Clone(),
 		workspace:            newWorkspace,
 	}
 
-	// Copy all of the FileHandles.
-	for k, v := range s.symbols {
-		if change, ok := changes[k]; ok {
-			if change.exists {
-				result.symbols[k] = result.buildSymbolHandle(ctx, change.fileHandle)
-			}
-			continue
-		}
-		newGen.Inherit(v.handle)
-		result.symbols[k] = v
-	}
-
 	// Copy the set of unloadable files.
 	for k, v := range s.unloadableFiles {
 		result.unloadableFiles[k] = v
 	}
-	// Copy all of the modHandles.
-	for k, v := range s.parseModHandles {
-		result.parseModHandles[k] = v
-	}
-	// Copy all of the parseWorkHandles.
-	for k, v := range s.parseWorkHandles {
-		result.parseWorkHandles[k] = v
-	}
 
+	// TODO(adonovan): merge loops over "changes".
 	for uri := range changes {
 		keys, ok := result.parseKeysByURI.Get(uri)
 		if ok {
@@ -1776,21 +1719,13 @@ func (s *snapshot) clone(ctx, bgCtx context.Context, changes map[span.URI]*fileC
 			}
 			result.parseKeysByURI.Delete(uri)
 		}
-	}
 
-	// Copy all of the go.mod-related handles. They may be invalidated later,
-	// so we inherit them at the end of the function.
-	for k, v := range s.modTidyHandles {
-		if _, ok := changes[k]; ok {
-			continue
-		}
-		result.modTidyHandles[k] = v
-	}
-	for k, v := range s.modWhyHandles {
-		if _, ok := changes[k]; ok {
-			continue
-		}
-		result.modWhyHandles[k] = v
+		// Invalidate go.mod-related handles.
+		result.modTidyHandles.Delete(uri)
+		result.modWhyHandles.Delete(uri)
+
+		// Invalidate handles for cached symbols.
+		result.symbolizeHandles.Delete(uri)
 	}
 
 	// Add all of the known subdirectories, but don't update them for the
@@ -1857,17 +1792,16 @@ func (s *snapshot) clone(ctx, bgCtx context.Context, changes map[span.URI]*fileC
 		// Invalidate the previous modTidyHandle if any of the files have been
 		// saved or if any of the metadata has been invalidated.
 		if invalidateMetadata || fileWasSaved(originalFH, change.fileHandle) {
-			// TODO(rstambler): Only delete mod handles for which the
-			// withoutURI is relevant.
-			for k := range s.modTidyHandles {
-				delete(result.modTidyHandles, k)
-			}
-			for k := range s.modWhyHandles {
-				delete(result.modWhyHandles, k)
-			}
+			// TODO(maybe): Only delete mod handles for
+			// which the withoutURI is relevant.
+			// Requires reverse-engineering the go command. (!)
+
+			result.modTidyHandles.Clear()
+			result.modWhyHandles.Clear()
 		}
-		delete(result.parseModHandles, uri)
-		delete(result.parseWorkHandles, uri)
+
+		result.parseModHandles.Delete(uri)
+		result.parseWorkHandles.Delete(uri)
 		// Handle the invalidated file; it may have new contents or not exist.
 		if !change.exists {
 			result.files.Delete(uri)
@@ -2011,19 +1945,6 @@ func (s *snapshot) clone(ctx, bgCtx context.Context, changes map[span.URI]*fileC
 		result.workspacePackages = s.workspacePackages
 	}
 
-	// Inherit all of the go.mod-related handles.
-	for _, v := range result.modTidyHandles {
-		newGen.Inherit(v.handle)
-	}
-	for _, v := range result.modWhyHandles {
-		newGen.Inherit(v.handle)
-	}
-	for _, v := range result.parseModHandles {
-		newGen.Inherit(v.handle)
-	}
-	for _, v := range result.parseWorkHandles {
-		newGen.Inherit(v.handle)
-	}
 	// Don't bother copying the importedBy graph,
 	// as it changes each time we update metadata.
 
diff --git a/internal/lsp/cache/symbols.go b/internal/lsp/cache/symbols.go
index 50d7b123e..ab031bf64 100644
--- a/internal/lsp/cache/symbols.go
+++ b/internal/lsp/cache/symbols.go
@@ -18,43 +18,48 @@ import (
 	"golang.org/x/tools/internal/memoize"
 )
 
-// A symbolHandle contains a handle to the result of symbolizing a file.
-type symbolHandle struct {
-	handle *memoize.Handle
-}
+// symbolize returns the result of symbolizing the file identified by fh, using a cache.
+func (s *snapshot) symbolize(ctx context.Context, fh source.FileHandle) ([]source.Symbol, error) {
+	uri := fh.URI()
 
-// symbolData contains the data produced by extracting symbols from a file.
-type symbolData struct {
-	symbols []source.Symbol
-	err     error
-}
+	s.mu.Lock()
+	entry, hit := s.symbolizeHandles.Get(uri)
+	s.mu.Unlock()
 
-// buildSymbolHandle returns a handle to the future result of
-// symbolizing the file identified by fh,
-// if necessary creating it and saving it in the snapshot.
-func (s *snapshot) buildSymbolHandle(ctx context.Context, fh source.FileHandle) *symbolHandle {
-	if h := s.getSymbolHandle(fh.URI()); h != nil {
-		return h
+	type symbolizeResult struct {
+		symbols []source.Symbol
+		err     error
 	}
-	type symbolHandleKey source.Hash
-	key := symbolHandleKey(fh.FileIdentity().Hash)
-	handle := s.generation.Bind(key, func(_ context.Context, arg memoize.Arg) interface{} {
-		snapshot := arg.(*snapshot)
-		symbols, err := symbolize(snapshot, fh)
-		return &symbolData{symbols, err}
-	})
 
-	sh := &symbolHandle{
-		handle: handle,
+	// Cache miss?
+	if !hit {
+		type symbolHandleKey source.Hash
+		key := symbolHandleKey(fh.FileIdentity().Hash)
+		handle, release := s.generation.GetHandle(key, func(_ context.Context, arg memoize.Arg) interface{} {
+			symbols, err := symbolizeImpl(arg.(*snapshot), fh)
+			return symbolizeResult{symbols, err}
+		})
+
+		entry = handle
+
+		s.mu.Lock()
+		s.symbolizeHandles.Set(uri, entry, func(_, _ interface{}) { release() })
+		s.mu.Unlock()
 	}
 
-	return s.addSymbolHandle(fh.URI(), sh)
+	// Await result.
+	v, err := entry.(*memoize.Handle).Get(ctx, s.generation, s)
+	if err != nil {
+		return nil, err
+	}
+	res := v.(symbolizeResult)
+	return res.symbols, res.err
 }
 
-// symbolize reads and parses a file and extracts symbols from it.
+// symbolizeImpl reads and parses a file and extracts symbols from it.
 // It may use a parsed file already present in the cache but
 // otherwise does not populate the cache.
-func symbolize(snapshot *snapshot, fh source.FileHandle) ([]source.Symbol, error) {
+func symbolizeImpl(snapshot *snapshot, fh source.FileHandle) ([]source.Symbol, error) {
 	src, err := fh.Read()
 	if err != nil {
 		return nil, err