internal/lsp/cache: clarify buildPackageHandle

Before, buildPackageHandle and buildKey were mutually recursive.
Together they performed a sequential recursion over Metadata.Deps,
calling GetFile and parseGoHandle for every file, and then
finally (in postorder) binding a Handle for the type checking step.

This change inlines buildKey to make the recursion more obvious,
performs the recursion over dependencies first, followed by
the reading of Go source files for this package, in parallel.
(The IWL benchmark reports improvement but its variance is so
high I'm not sure I trust it.) Other opportunities for parallelism
are pointed out in new comments.

The Bind operation for typechecking calls dep.check for each
dependency in a separate goroutine. It no longer waits for
each one since it is only prefetching the information
that will be required during import processing, which will
block until the information becomes available.

Before, both reading and parsing appear to occur twice:
once in buildPackageHandle and once in doTypeCheck.
(Perhaps the second was a cache hits, but there's no need
to rely on a cache.)
Now, only file reading (GetFile) occurs in buildPackageHandle,
since that's all that's needed for the packageKey.
And parsing only occurs in doTypeCheck. The source.FileHandles
are plumbed through as parameters.

Also:
- move parseGoHandles to a local function, since it exists only
  for buildPackageKey. It no longer parses, it only reads.
- lots of TODO comments for possible optimizations,
  and typical measured times of various operations.
- remove obsolete comment re: Bind and finalizers.

Change-Id: Iad049884607b73eaa6701bdf7771f96b042142d5
Reviewed-on: https://go-review.googlesource.com/c/tools/+/411913
Run-TryBot: Alan Donovan <[email protected]>
gopls-CI: kokoro <[email protected]>
Auto-Submit: Alan Donovan <[email protected]>
Reviewed-by: Robert Findley <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>

Author: adonovan

internal/lsp/cache/cache.go

@@ -101,7 +101,7 @@ func (c *Cache) getFile(ctx context.Context, uri span.URI) (*fileHandle, error)
 		return fh, nil
 	}
 
-	fh, err := readFile(ctx, uri, fi)
+	fh, err := readFile(ctx, uri, fi) // ~25us
 	if err != nil {
 		return nil, err
 	}
@@ -126,7 +126,7 @@ func readFile(ctx context.Context, uri span.URI, fi os.FileInfo) (*fileHandle, e
 	_ = ctx
 	defer done()
 
-	data, err := ioutil.ReadFile(uri.Filename())
+	data, err := ioutil.ReadFile(uri.Filename()) // ~20us
 	if err != nil {
 		return &fileHandle{
 			modTime: fi.ModTime(),