From 0f9bb8f614ff5a69f92d782071acc99de30e0344 Mon Sep 17 00:00:00 2001
From: Rebecca Stambler <rstambler@golang.org>
Date: Mon, 23 Sep 2019 18:18:01 -0400
Subject: [PATCH] internal/lsp: only cache type information for active packages

Currently, we cache source.CheckPackageHandles for each file and package
that we are aware of, as well as dependencies. This is not necessary,
since the active packages pin their imports in memory.

Change-Id: Ia0101f4d4a2d36d5baeb890af3d7c8baec297847
Reviewed-on: https://go-review.googlesource.com/c/tools/+/196982
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
---
 internal/lsp/cache/check.go    | 44 ----------------------------------
 internal/lsp/cache/external.go |  1 +
 internal/lsp/cache/load.go     | 39 ++++++++++++++++++++++++++----
 3 files changed, 36 insertions(+), 48 deletions(-)

diff --git a/internal/lsp/cache/check.go b/internal/lsp/cache/check.go
index 0357a193e7..f1b94f1a94 100644
--- a/internal/lsp/cache/check.go
+++ b/internal/lsp/cache/check.go
@@ -220,11 +220,6 @@ func (imp *importer) Import(pkgPath string) (*types.Package, error) {
 		return nil, err
 	}
 	imp.parentPkg.imports[packagePath(pkgPath)] = pkg
-
-	// Add every file in this package to our cache.
-	if err := imp.cachePackage(ctx, cph); err != nil {
-		return nil, err
-	}
 	return pkg.GetTypes(), nil
 }
 
@@ -335,45 +330,6 @@ func (imp *importer) child(ctx context.Context, pkg *pkg, cph *checkPackageHandl
 	}
 }
 
-func (imp *importer) cachePackage(ctx context.Context, cph *checkPackageHandle) error {
-	for _, ph := range cph.files {
-		uri := ph.File().Identity().URI
-		f, err := imp.view.GetFile(ctx, uri)
-		if err != nil {
-			return errors.Errorf("no such file %s: %v", uri, err)
-		}
-		gof, ok := f.(*goFile)
-		if !ok {
-			return errors.Errorf("%s is not a Go file", uri)
-		}
-		if err := imp.cachePerFile(ctx, gof, ph, cph); err != nil {
-			return errors.Errorf("failed to cache file %s: %v", gof.URI(), err)
-		}
-	}
-	return nil
-}
-
-func (imp *importer) cachePerFile(ctx context.Context, gof *goFile, ph source.ParseGoHandle, cph *checkPackageHandle) error {
-	gof.mu.Lock()
-	defer gof.mu.Unlock()
-
-	// Set the package even if we failed to parse the file.
-	if gof.cphs == nil {
-		gof.cphs = make(map[packageKey]*checkPackageHandle)
-	}
-	gof.cphs[packageKey{
-		id:   cph.m.id,
-		mode: ph.Mode(),
-	}] = cph
-
-	file, _, _, err := ph.Parse(ctx)
-	if err != nil {
-		return err
-	}
-	gof.imports = file.Imports
-	return nil
-}
-
 func (c *cache) appendPkgError(pkg *pkg, err error) {
 	if err == nil {
 		return
diff --git a/internal/lsp/cache/external.go b/internal/lsp/cache/external.go
index 315a13176b..dc1769ee4b 100644
--- a/internal/lsp/cache/external.go
+++ b/internal/lsp/cache/external.go
@@ -53,6 +53,7 @@ func (h *nativeFileHandle) Identity() source.FileIdentity {
 func (h *nativeFileHandle) Read(ctx context.Context) ([]byte, string, error) {
 	ctx, done := trace.StartSpan(ctx, "cache.nativeFileHandle.Read", telemetry.File.Of(h.identity.URI.Filename()))
 	defer done()
+
 	ioLimit <- struct{}{}
 	defer func() { <-ioLimit }()
 	// TODO: this should fail if the version is not the same as the handle
diff --git a/internal/lsp/cache/load.go b/internal/lsp/cache/load.go
index e858164a25..fc2f39988b 100644
--- a/internal/lsp/cache/load.go
+++ b/internal/lsp/cache/load.go
@@ -7,6 +7,7 @@ package cache
 import (
 	"context"
 	"fmt"
+	"go/ast"
 
 	"golang.org/x/tools/go/packages"
 	"golang.org/x/tools/internal/lsp/source"
@@ -38,15 +39,45 @@ func (view *view) loadParseTypecheck(ctx context.Context, f *goFile, fh source.F
 			log.Error(ctx, "loadParseTypeCheck: failed to get CheckPackageHandle", err, telemetry.Package.Of(m.id))
 			continue
 		}
-		// Cache this package on the file object, since all dependencies are cached in the Import function.
-		if err := imp.cachePackage(ctx, cph); err != nil {
-			log.Error(ctx, "loadParseTypeCheck: failed to cache package", err, telemetry.Package.Of(m.id))
-			continue
+		// Cache the package type information for the top-level package.
+		for _, ph := range cph.files {
+			file, _, _, err := ph.Parse(ctx)
+			if err != nil {
+				return err
+			}
+			f, err := imp.view.GetFile(ctx, ph.File().Identity().URI)
+			if err != nil {
+				return errors.Errorf("no such file %s: %v", ph.File().Identity().URI, err)
+			}
+			gof, ok := f.(*goFile)
+			if !ok {
+				return errors.Errorf("%s is not a Go file", ph.File().Identity().URI)
+			}
+			if err := cachePerFile(ctx, gof, ph.Mode(), file.Imports, cph); err != nil {
+				return err
+			}
 		}
 	}
 	return nil
 }
 
+func cachePerFile(ctx context.Context, f *goFile, mode source.ParseMode, imports []*ast.ImportSpec, cph *checkPackageHandle) error {
+	f.mu.Lock()
+	defer f.mu.Unlock()
+
+	f.imports = imports
+
+	if f.cphs == nil {
+		f.cphs = make(map[packageKey]*checkPackageHandle)
+	}
+	f.cphs[packageKey{
+		id:   cph.m.id,
+		mode: mode,
+	}] = cph
+
+	return nil
+}
+
 func (view *view) load(ctx context.Context, f *goFile, fh source.FileHandle) ([]*metadata, error) {
 	ctx, done := trace.StartSpan(ctx, "cache.view.load", telemetry.URI.Of(f.URI()))
 	defer done()