internal/memoize: switch from GC-driven to explicit deletion

The GC-based cache has given us a number of problems. First, memory
leaks driven by reference cycles: the Go runtime cannot collect cycles
involving finalizers, which prevents us from writing natural code in
Bind callbacks. If we screw it up, we get a mysterious leak that takes a
long time to track down. Second, the behavior is generally mysterious;
it's hard to predict how long a value lasts, and harder to tell if a
value being live is a bug. Third, we think that it may be interacting
poorly with the GC, resulting in unnecessary memory usage.

The structure of the values we put in the cache is not actually that
complicated -- there are only 5 significant types: parse, typecheck,
analyze, parse mod, and analyze mod. Managing them manually should not
be conceptually difficult, and in fact we already do most of the work
in (*snapshot).clone.

In this CL the cache adds the concept of "generations", which function
as reference counts on cache entries. Entries are still global and
shared across generations, but will be explicitly deleted once no
generations refer to them. The idea is that each snapshot is a new
generation, and can inherit entries from the previous snapshot or leave
them behind to be deleted.

One obvious risk of this scheme is that we'll leave dangling references
to values without actually inheriting them across generations. To
prevent that, getting a value requires passing in the generation at
which it's being read, and an error will be returned if that generation
is dead.

Change-Id: I4b30891efd7be4e10f2b84f4c067b0dee43dcf9c
Reviewed-on: https://go-review.googlesource.com/c/tools/+/242838
Run-TryBot: Heschi Kreinick <heschi@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>

1 files changed, 6 insertions, 10 deletions

diff --git a/internal/lsp/cache/parse.go b/internal/lsp/cache/parse.go
index b5ce9dff6..1c4c64ac8 100644
--- a/internal/lsp/cache/parse.go
+++ b/internal/lsp/cache/parse.go
@@ -41,8 +41,6 @@ type parseGoHandle struct {
 }
 
 type parseGoData struct {
-	memoize.NoCopy
-
 	parsed *source.ParsedGoFile
 
 	// If true, we adjusted the AST to make it type check better, and
@@ -59,12 +57,12 @@ func (s *snapshot) parseGoHandle(ctx context.Context, fh source.FileHandle, mode
 	if pgh := s.getGoFile(key); pgh != nil {
 		return pgh
 	}
-	parseHandle := s.view.session.cache.store.Bind(key, func(ctx context.Context, arg memoize.Arg) interface{} {
+	parseHandle := s.generation.Bind(key, func(ctx context.Context, arg memoize.Arg) interface{} {
 		snapshot := arg.(*snapshot)
 		return parseGo(ctx, snapshot.view.session.cache.fset, fh, mode)
 	})
 
-	astHandle := s.view.session.cache.store.Bind(astCacheKey(key), func(ctx context.Context, arg memoize.Arg) interface{} {
+	astHandle := s.generation.Bind(astCacheKey(key), func(ctx context.Context, arg memoize.Arg) interface{} {
 		snapshot := arg.(*snapshot)
 		return buildASTCache(ctx, snapshot, parseHandle)
 	})
@@ -97,7 +95,7 @@ func (s *snapshot) ParseGo(ctx context.Context, fh source.FileHandle, mode sourc
 }
 
 func (s *snapshot) parseGo(ctx context.Context, pgh *parseGoHandle) (*source.ParsedGoFile, bool, error) {
-	d, err := pgh.handle.Get(ctx, s)
+	d, err := pgh.handle.Get(ctx, s.generation, s)
 	if err != nil {
 		return nil, false, err
 	}
@@ -112,7 +110,7 @@ func (s *snapshot) PosToDecl(ctx context.Context, pgf *source.ParsedGoFile) (map
 	}
 
 	pgh := s.parseGoHandle(ctx, fh, pgf.Mode)
-	d, err := pgh.astCacheHandle.Get(ctx, s)
+	d, err := pgh.astCacheHandle.Get(ctx, s.generation, s)
 	if err != nil {
 		return nil, err
 	}
@@ -128,7 +126,7 @@ func (s *snapshot) PosToField(ctx context.Context, pgf *source.ParsedGoFile) (ma
 	}
 
 	pgh := s.parseGoHandle(ctx, fh, pgf.Mode)
-	d, err := pgh.astCacheHandle.Get(ctx, s)
+	d, err := pgh.astCacheHandle.Get(ctx, s.generation, s)
 	if err != nil || d == nil {
 		return nil, err
 	}
@@ -138,8 +136,6 @@ func (s *snapshot) PosToField(ctx context.Context, pgf *source.ParsedGoFile) (ma
 }
 
 type astCacheData struct {
-	memoize.NoCopy
-
 	err error
 
 	posToDecl  map[token.Pos]ast.Decl
@@ -156,7 +152,7 @@ func buildASTCache(ctx context.Context, snapshot *snapshot, parseHandle *memoize
 		decls []ast.Decl
 	)
 
-	v, err := parseHandle.Get(ctx, snapshot)
+	v, err := parseHandle.Get(ctx, snapshot.generation, snapshot)
 	if err != nil {
 		return &astCacheData{err: err}
 	}