internal/lsp: memoize all the parsing

Change-Id: Ic7864a564f88b3fe0a421bb825b01d750610dee9
Reviewed-on: https://go-review.googlesource.com/c/tools/+/181119
Run-TryBot: Ian Cottrell <iancottrell@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>

internal/lsp/cache/load.go

@@ -3,9 +3,9 @@ package cache
 import (
 	"context"
 	"fmt"
-	"go/parser"
 
 	"golang.org/x/tools/go/packages"
+	"golang.org/x/tools/internal/lsp/source"
 	"golang.org/x/tools/internal/span"
 )
 
@@ -115,11 +115,7 @@ func (v *view) parseImports(ctx context.Context, f *goFile) bool {
 		return true
 	}
 	// Get file content in case we don't already have it.
-	data, _, err := f.Handle(ctx).Read(ctx)
+	parsed, _ := v.session.cache.ParseGo(f.Handle(ctx), source.ParseHeader).Parse(ctx)
-	if err != nil {
-		return true
-	}
-	parsed, _ := parser.ParseFile(f.FileSet(), f.filename(), data, parser.ImportsOnly)
 	if parsed == nil {
 		return true
 	}

internal/lsp/cache/parse.go

@@ -16,17 +16,84 @@ import (
 	"strings"
 	"sync"
 
+	"golang.org/x/tools/internal/lsp/source"
+	"golang.org/x/tools/internal/memoize"
 	"golang.org/x/tools/internal/span"
 )
 
-func parseFile(fset *token.FileSet, filename string, src []byte) (*ast.File, error) {
+type parseKey struct {
-	return parser.ParseFile(fset, filename, src, parser.AllErrors|parser.ParseComments)
+	file source.FileIdentity
+	mode source.ParseMode
+}
+
+type parseGoHandle struct {
+	handle *memoize.Handle
+	file   source.FileHandle
+	mode   source.ParseMode
+}
+
+type parseGoData struct {
+	memoize.NoCopy
+	ast *ast.File
+	err error
 }
 
 // We use a counting semaphore to limit
 // the number of parallel I/O calls per process.
 var ioLimit = make(chan bool, 20)
 
+func (c *cache) ParseGo(fh source.FileHandle, mode source.ParseMode) source.ParseGoHandle {
+	key := parseKey{
+		file: fh.Identity(),
+		mode: mode,
+	}
+	h := c.store.Bind(key, func(ctx context.Context) interface{} {
+		data := &parseGoData{}
+		data.ast, data.err = parseGo(ctx, c, fh, mode)
+		return data
+	})
+	return &parseGoHandle{
+		handle: h,
+	}
+}
+
+func (h *parseGoHandle) File() source.FileHandle {
+	return h.file
+}
+
+func (h *parseGoHandle) Mode() source.ParseMode {
+	return h.mode
+}
+
+func (h *parseGoHandle) Parse(ctx context.Context) (*ast.File, error) {
+	v := h.handle.Get(ctx)
+	if v == nil {
+		return nil, ctx.Err()
+	}
+	data := v.(*parseGoData)
+	return data.ast, data.err
+}
+
+func parseGo(ctx context.Context, c *cache, fh source.FileHandle, mode source.ParseMode) (*ast.File, error) {
+	buf, _, err := fh.Read(ctx)
+	if err != nil {
+		return nil, err
+	}
+	parserMode := parser.AllErrors | parser.ParseComments
+	if mode == source.ParseHeader {
+		parserMode = parser.ImportsOnly
+	}
+	ast, err := parser.ParseFile(c.fset, fh.Identity().URI.Filename(), buf, parserMode)
+	if err != nil {
+		return ast, err
+	}
+	if mode == source.ParseExported {
+		trimAST(ast)
+	}
+	//TODO: move the ast fixup code into here
+	return ast, nil
+}
+
 // parseFiles reads and parses the Go source files and returns the ASTs
 // of the ones that could be at least partially parsed, along with a list
 // parse errors encountered, and a fatal error that prevented parsing.
@@ -36,41 +103,26 @@ var ioLimit = make(chan bool, 20)
 //
 func (imp *importer) parseFiles(filenames []string, ignoreFuncBodies bool) ([]*astFile, []error, error) {
 	var (
-		wg       sync.WaitGroup
+		wg     sync.WaitGroup
-		mu       sync.Mutex
+		n      = len(filenames)
-		n        = len(filenames)
+		parsed = make([]*astFile, n)
-		parsed   = make([]*astFile, n)
+		errors = make([]error, n)
-		errors   = make([]error, n)
-		fatalErr error
 	)
-
+	// TODO: change this function to return the handles
-	setFatalErr := func(err error) {
+	// TODO: eliminate the wait group at this layer, it should be done in the parser
-		mu.Lock()
-		fatalErr = err
-		mu.Unlock()
-	}
-
 	for i, filename := range filenames {
 		if err := imp.ctx.Err(); err != nil {
-			setFatalErr(err)
+			return nil, nil, err
-			break
 		}
-		// First, check if we have already cached an AST for this file.
+		// get a file handle
-		f, err := imp.view.findFile(span.FileURI(filename))
+		fh := imp.view.session.GetFile(span.FileURI(filename))
-		if err != nil {
+		// now get a parser
-			setFatalErr(err)
+		mode := source.ParseFull
-			break
+		if ignoreFuncBodies {
-		}
+			mode = source.ParseExported
-		if f == nil {
-			setFatalErr(fmt.Errorf("could not find file %s", filename))
-			break
-		}
-
-		gof, ok := f.(*goFile)
-		if !ok {
-			setFatalErr(fmt.Errorf("non-Go file in parse call: %s", filename))
-			break
 		}
+		ph := imp.view.session.cache.ParseGo(fh, mode)
+		// now read and parse in parallel
 		wg.Add(1)
 		go func(i int, filename string) {
 			ioLimit <- true // wait
@@ -78,49 +130,26 @@ func (imp *importer) parseFiles(filenames []string, ignoreFuncBodies bool) ([]*a
 				<-ioLimit // signal done
 				wg.Done()
 			}()
-
-			// If we already have a cached AST, reuse it.
-			// If the AST is trimmed, only use it if we are ignoring function bodies.
-			if gof.ast != nil && gof.ast.isTrimmed == ignoreFuncBodies {
-				parsed[i], errors[i] = gof.ast, gof.ast.err
-				return
-			}
-
-			// We don't have a cached AST for this file, so we read its content and parse it.
-			src, _, err := gof.Handle(imp.ctx).Read(imp.ctx)
-			if err != nil {
-				setFatalErr(err)
-				return
-			}
-			if src == nil {
-				setFatalErr(fmt.Errorf("no source for %v", filename))
-				return
-			}
-
 			// ParseFile may return a partial AST and an error.
-			f, err := parseFile(imp.fset, filename, src)
+			f, err := ph.Parse(imp.ctx)
 			parsed[i], errors[i] = &astFile{
 				file:      f,
 				err:       err,
 				isTrimmed: ignoreFuncBodies,
 			}, err
-
+			// TODO: move fixup into the parse function
-			if ignoreFuncBodies {
-				trimAST(f)
-			}
 			// Fix any badly parsed parts of the AST.
 			if f != nil {
 				tok := imp.fset.File(f.Pos())
-				imp.view.fix(imp.ctx, f, tok, src)
+				src, _, err := fh.Read(imp.ctx)
+				if err == nil {
+					imp.view.fix(imp.ctx, f, tok, src)
+				}
 			}
 		}(i, filename)
 	}
 	wg.Wait()
 
-	if fatalErr != nil {
-		return nil, nil, fatalErr
-	}
-
 	// Eliminate nils, preserving order.
 	var o int
 	for _, f := range parsed {

internal/lsp/cache/view.go

@@ -8,6 +8,7 @@ import (
 	"context"
 	"go/ast"
 	"go/parser"
+	"go/token"
 	"go/types"
 	"os"
 	"path/filepath"
@@ -124,10 +125,12 @@ func (v *view) buildConfig() *packages.Config {
 			packages.NeedImports |
 			packages.NeedDeps |
 			packages.NeedTypesSizes,
-		Fset:      v.session.cache.fset,
+		Fset:    v.session.cache.fset,
-		Overlay:   v.session.buildOverlay(),
+		Overlay: v.session.buildOverlay(),
-		ParseFile: parseFile,
+		ParseFile: func(*token.FileSet, string, []byte) (*ast.File, error) {
-		Tests:     true,
+			panic("go/packages must not be used to parse files")
+		},
+		Tests: true,
 	}
 }
 

internal/lsp/source/view.go

@@ -43,6 +43,35 @@ type FileSystem interface {
 	GetFile(uri span.URI) FileHandle
 }
 
+// ParseGoHandle represents a handle to the ast for a file.
+type ParseGoHandle interface {
+	// File returns a file handle to get the ast for.
+	File() FileHandle
+	// Mode returns the parse mode of this handle.
+	Mode() ParseMode
+	// Parse returns the parsed AST for the file.
+	// If the file is not available, returns nil and an error.
+	Parse(ctx context.Context) (*ast.File, error)
+}
+
+// ParseMode controls the content of the AST produced when parsing a source file.
+type ParseMode int
+
+const (
+	// ParseHeader specifies that the main package declaration and imports are needed.
+	// This is the mode used when attempting to examine the package graph structure.
+	ParseHeader = ParseMode(iota)
+	// ParseExported specifies that the public symbols are needed, but things like
+	// private symbols and function bodies are not.
+	// This mode is used for things where a package is being consumed only as a
+	// dependency.
+	ParseExported
+	// ParseFull specifies the full AST is needed.
+	// This is used for files of direct interest where the entire contents must
+	// be considered.
+	ParseFull
+)
+
 // Cache abstracts the core logic of dealing with the environment from the
 // higher level logic that processes the information to produce results.
 // The cache provides access to files and their contents, so the source
@@ -59,6 +88,9 @@ type Cache interface {
 
 	// FileSet returns the shared fileset used by all files in the system.
 	FileSet() *token.FileSet
+
+	// Parse returns a ParseHandle for the given file handle.
+	ParseGo(FileHandle, ParseMode) ParseGoHandle
 }
 
 // Session represents a single connection from a client.

internal/lsp/tests/tests.go

@@ -8,7 +8,6 @@ import (
 	"context"
 	"flag"
 	"go/ast"
-	"go/parser"
 	"go/token"
 	"io/ioutil"
 	"path/filepath"
@@ -191,7 +190,7 @@ func Load(t testing.TB, exporter packagestest.Exporter, dir string) *Data {
 	data.Config.Fset = token.NewFileSet()
 	data.Config.Context = context.Background()
 	data.Config.ParseFile = func(fset *token.FileSet, filename string, src []byte) (*ast.File, error) {
-		return parser.ParseFile(fset, filename, src, parser.AllErrors|parser.ParseComments)
+		panic("ParseFile should not be called")
 	}
 
 	// Do a first pass to collect special markers for completion.