internal/lsp/cache: use cached parsed files for symbols, if available

Optimize building the symbol index for a file, in two ways:
 - use the cached full parse tree, if it already exists
 - if it doesn't exist, optimize parsing by skipping both comments and
   object resolution, which aren't necessary for symbols

This results in around 3x faster initial indexing of symbols. In my
manual testing, indexing of Kubernetes went from 16s->5s, and indexing
of x/tools went from 2.4s->700ms.

Also fix a typo in gopls/internal/regtest/bench/bench_test.go.

Fixes #52602

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

gopls/internal/regtest/bench/bench_test.go

@@ -130,7 +130,7 @@ func TestBenchmarkSymbols(t *testing.T) {
 }
 
 var (
-	benchDir     = flag.String("didchange_dir", "", "If set, run benchmarks in this dir. Must also set regtest_bench_file.")
+	benchDir     = flag.String("didchange_dir", "", "If set, run benchmarks in this dir. Must also set didchange_file.")
 	benchFile    = flag.String("didchange_file", "", "The file to modify")
 	benchProfile = flag.String("didchange_cpuprof", "", "file to write cpu profiling data to")
 )

internal/lsp/cache/parsemode_go116.go

@@ -0,0 +1,11 @@
+// Copyright 2022 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+//go:build !go1.17
+// +build !go1.17
+
+package cache
+
+// The parser.SkipObjectResolution mode flag is not supported before Go 1.17.
+const skipObjectResolution = 0

internal/lsp/cache/parsemode_go117.go

@@ -0,0 +1,12 @@
+// Copyright 2022 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+//go:build go1.17
+// +build go1.17
+
+package cache
+
+import "go/parser"
+
+const skipObjectResolution = parser.SkipObjectResolution

internal/lsp/cache/symbols.go

@@ -7,14 +7,15 @@ package cache
 import (
 	"context"
 	"go/ast"
+	"go/parser"
 	"go/token"
 	"go/types"
 	"strings"
 
+	"golang.org/x/tools/internal/lsp/lsppos"
 	"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/span"
 )
 
 type symbolHandle struct {
@@ -54,22 +55,58 @@ func (s *snapshot) buildSymbolHandle(ctx context.Context, fh source.FileHandle)
 	return s.addSymbolHandle(sh)
 }
 
-// symbolize extracts symbols from a file. It does not parse the file through the cache.
+// symbolize extracts symbols from a file. It uses a parsed file already
+// present in the cache but otherwise does not populate the cache.
 func symbolize(ctx context.Context, snapshot *snapshot, fh source.FileHandle) ([]source.Symbol, error) {
-	var w symbolWalker
-	fset := token.NewFileSet() // don't use snapshot.FileSet, as that would needlessly leak memory.
-	data := parseGo(ctx, fset, fh, source.ParseFull)
-	if data.parsed != nil && data.parsed.File != nil {
-		w.curFile = data.parsed
-		w.curURI = protocol.URIFromSpanURI(data.parsed.URI)
-		w.fileDecls(data.parsed.File.Decls)
+	src, err := fh.Read()
+	if err != nil {
+		return nil, err
 	}
+
+	var (
+		file     *ast.File
+		fileDesc *token.File
+	)
+
+	// If the file has already been fully parsed through the cache, we can just
+	// use the result.
+	key := parseKey{file: fh.FileIdentity(), mode: source.ParseFull}
+	if pgh := snapshot.getGoFile(key); pgh != nil {
+		cached := pgh.handle.Cached(snapshot.generation)
+		if cached != nil {
+			cached := cached.(*parseGoData)
+			if cached.parsed != nil {
+				file = cached.parsed.File
+				fileDesc = cached.parsed.Tok
+			}
+		}
+	}
+
+	// Otherwise, we parse the file ourselves. Notably we don't use parseGo here,
+	// so that we can avoid parsing comments and can skip object resolution,
+	// which has a meaningful impact on performance. Neither comments nor objects
+	// are necessary for symbol construction.
+	if file == nil {
+		fset := token.NewFileSet()
+		file, err = parser.ParseFile(fset, fh.URI().Filename(), src, skipObjectResolution)
+		if file == nil {
+			return nil, err
+		}
+		fileDesc = fset.File(file.Package)
+	}
+
+	w := &symbolWalker{
+		mapper: lsppos.NewTokenMapper(src, fileDesc),
+	}
+
+	w.fileDecls(file.Decls)
+
 	return w.symbols, w.firstError
 }
 
 type symbolWalker struct {
-	curFile    *source.ParsedGoFile
-	curURI     protocol.DocumentURI
+	mapper *lsppos.TokenMapper // for computing positions
+
 	symbols    []source.Symbol
 	firstError error
 }
@@ -84,7 +121,7 @@ func (w *symbolWalker) atNode(node ast.Node, name string, kind protocol.SymbolKi
 	}
 	b.WriteString(name)
 
-	rng, err := fileRange(w.curFile, node.Pos(), node.End())
+	rng, err := w.mapper.Range(node.Pos(), node.End())
 	if err != nil {
 		w.error(err)
 		return
@@ -103,14 +140,6 @@ func (w *symbolWalker) error(err error) {
 	}
 }
 
-func fileRange(pgf *source.ParsedGoFile, start, end token.Pos) (protocol.Range, error) {
-	s, err := span.FileSpan(pgf.Tok, pgf.Mapper.Converter, start, end)
-	if err != nil {
-		return protocol.Range{}, nil
-	}
-	return pgf.Mapper.Range(s)
-}
-
 func (w *symbolWalker) fileDecls(decls []ast.Decl) {
 	for _, decl := range decls {
 		switch decl := decl.(type) {