go.tools/importer: make loading/parsing concurrent.

1. ParseFiles (in util.go) parses each file in its own goroutine.

2. (*Importer).LoadPackage asynchronously prefetches the
   import graph by scanning the imports of each loaded package
   and calling LoadPackage on each one.

   LoadPackage is now thread-safe and idempotent: it uses a
   condition variable per package; the first goroutine to
   request a package becomes responsible for loading it and
   broadcasts to the others (waiting) when it becomes ready.

ssadump runs 34% faster when loading the oracle.

Also, refactorings:
- delete SourceLoader mechanism; just expose go/build.Context directly.
- CreateSourcePackage now also returns an error directly,
  rather than via PackageInfo.Err, since every client wants that.

R=crawshaw
CC=golang-dev
https://golang.org/cl/13509045

cmd/oracle/main.go

@@ -20,6 +20,7 @@ import (
 	"encoding/json"
 	"flag"
 	"fmt"
+	"go/build"
 	"io"
 	"log"
 	"os"
@@ -120,7 +121,7 @@ func main() {
 	}
 
 	// Ask the oracle.
-	res, err := oracle.Query(args, *modeFlag, *posFlag, ptalog, nil)
+	res, err := oracle.Query(args, *modeFlag, *posFlag, ptalog, &build.Default)
 	if err != nil {
 		fmt.Fprintln(os.Stderr, err)
 		os.Exit(1)

cmd/ssadump/main.go

@@ -8,6 +8,7 @@ package main
 import (
 	"flag"
 	"fmt"
+	"go/build"
 	"log"
 	"os"
 	"runtime"
@@ -66,7 +67,7 @@ func main() {
 	flag.Parse()
 	args := flag.Args()
 
-	impctx := importer.Config{Loader: importer.MakeGoBuildLoader(nil)}
+	impctx := importer.Config{Build: &build.Default}
 
 	var debugMode bool
 	var mode ssa.BuilderMode
@@ -85,7 +86,7 @@ func main() {
 		case 'N':
 			mode |= ssa.NaiveForm
 		case 'G':
-			impctx.Loader = nil
+			impctx.Build = nil
 		case 'L':
 			mode |= ssa.BuildSerially
 		default:

importer/importer.go

@@ -14,19 +14,32 @@ package importer
 import (
 	"fmt"
 	"go/ast"
+	"go/build"
 	"go/token"
 	"os"
+	"strconv"
+	"sync"
 
 	"code.google.com/p/go.tools/go/exact"
 	"code.google.com/p/go.tools/go/types"
 )
 
-// An Importer's methods are not thread-safe.
+// An Importer's methods are not thread-safe unless specified.
 type Importer struct {
 	config   *Config                 // the client configuration
 	Fset     *token.FileSet          // position info for all files seen
-	Packages map[string]*PackageInfo // keys are import paths
+	Packages map[string]*PackageInfo // successfully imported packages keyed by import path
 	errors   map[string]error        // cache of errors by import path
+
+	mu       sync.Mutex          // guards 'packages' map
+	packages map[string]*pkgInfo // all attempted imported packages, keyed by import path
+}
+
+// pkgInfo holds internal per-package information.
+type pkgInfo struct {
+	info  *PackageInfo  // success info
+	err   error         // failure info
+	ready chan struct{} // channel close is notification of ready state
 }
 
 // Config specifies the configuration for the importer.
@@ -38,7 +51,7 @@ type Config struct {
 	// through to the type checker.
 	TypeChecker types.Config
 
-	// If Loader is non-nil, it is used to satisfy imports.
+	// If Build is non-nil, it is used to satisfy imports.
 	//
 	// If it is nil, binary object files produced by the gc
 	// compiler will be loaded instead of source code for all
@@ -47,23 +60,9 @@ type Config struct {
 	// code, so this mode will not yield a whole program.  It is
 	// intended for analyses that perform intraprocedural analysis
 	// of a single package.
-	Loader SourceLoader
+	Build *build.Context
 }
 
-// SourceLoader is the signature of a function that locates, reads and
-// parses a set of source files given an import path.
-//
-// fset is the fileset to which the ASTs should be added.
-// path is the imported path, e.g. "sync/atomic".
-//
-// On success, the function returns files, the set of ASTs produced,
-// or the first error encountered.
-//
-// The MakeGoBuildLoader utility can be used to construct a
-// SourceLoader based on go/build.
-//
-type SourceLoader func(fset *token.FileSet, path string) (files []*ast.File, err error)
-
 // New returns a new, empty Importer using configuration options
 // specified by config.
 //
@@ -71,6 +70,7 @@ func New(config *Config) *Importer {
 	imp := &Importer{
 		config:   config,
 		Fset:     token.NewFileSet(),
+		packages: make(map[string]*pkgInfo),
 		Packages: make(map[string]*PackageInfo),
 		errors:   make(map[string]error),
 	}
@@ -91,8 +91,10 @@ func (imp *Importer) doImport(imports map[string]*types.Package, path string) (p
 	// Load the source/binary for 'path', type-check it, construct
 	// a PackageInfo and update our map (imp.Packages) and the
 	// type-checker's map (imports).
+	// TODO(adonovan): make it the type-checker's job to
+	// consult/update the 'imports' map.  Then we won't need it.
 	var info *PackageInfo
-	if imp.config.Loader != nil {
+	if imp.config.Build != nil {
 		info, err = imp.LoadPackage(path)
 	} else {
 		if info, ok := imp.Packages[path]; ok {
@@ -123,37 +125,80 @@ func (imp *Importer) doImport(imports map[string]*types.Package, path string) (p
 	return nil, err
 }
 
-// LoadPackage loads the package of the specified import-path,
+// imports returns the set of paths imported by the specified files.
+func imports(p string, files []*ast.File) map[string]bool {
+	imports := make(map[string]bool)
+outer:
+	for _, file := range files {
+		for _, decl := range file.Decls {
+			if decl, ok := decl.(*ast.GenDecl); ok {
+				if decl.Tok != token.IMPORT {
+					break outer // stop at the first non-import
+				}
+				for _, spec := range decl.Specs {
+					spec := spec.(*ast.ImportSpec)
+					if path, _ := strconv.Unquote(spec.Path.Value); path != "C" {
+						imports[path] = true
+					}
+				}
+			} else {
+				break outer // stop at the first non-import
+			}
+		}
+	}
+	return imports
+}
+
+// LoadPackage loads the package of the specified import path,
 // performs type-checking, and returns the corresponding
 // PackageInfo.
 //
-// Not idempotent!
-// Precondition: Importer.config.Loader != nil.
+// Precondition: Importer.config.Build != nil.
+// Idempotent and thread-safe.
+//
 // TODO(adonovan): fix: violated in call from CreatePackageFromArgs!
-// Not thread-safe!
 // TODO(adonovan): rethink this API.
 //
 func (imp *Importer) LoadPackage(importPath string) (*PackageInfo, error) {
-	if info, ok := imp.Packages[importPath]; ok {
-		return info, nil // positive cache hit
+	if imp.config.Build == nil {
+		panic("Importer.LoadPackage without a go/build.Config")
 	}
 
-	if err := imp.errors[importPath]; err != nil {
-		return nil, err // negative cache hit
+	imp.mu.Lock()
+	pi, ok := imp.packages[importPath]
+	if !ok {
+		// First request for this pkgInfo:
+		// create a new one in !ready state.
+		pi = &pkgInfo{ready: make(chan struct{})}
+		imp.packages[importPath] = pi
+		imp.mu.Unlock()
+
+		// Load/parse the package.
+		if files, err := loadPackage(imp.config.Build, imp.Fset, importPath); err == nil {
+			// Kick off asynchronous I/O and parsing for the imports.
+			for path := range imports(importPath, files) {
+				go func() { imp.LoadPackage(path) }()
+			}
+
+			// Type-check the package.
+			pi.info, pi.err = imp.CreateSourcePackage(importPath, files)
+		} else {
+			pi.err = err
+		}
+
+		close(pi.ready) // enter ready state and wake up waiters
+	} else {
+		imp.mu.Unlock()
+
+		<-pi.ready // wait for ready condition.
 	}
 
-	if imp.config.Loader == nil {
-		panic("Importer.LoadPackage without a SourceLoader")
+	// Invariant: pi is ready.
+
+	if pi.err != nil {
+		return nil, pi.err
 	}
-	files, err := imp.config.Loader(imp.Fset, importPath)
-	if err != nil {
-		return nil, err
-	}
-	info := imp.CreateSourcePackage(importPath, files)
-	if info.Err != nil {
-		return nil, info.Err
-	}
-	return info, nil
+	return pi.info, nil
 }
 
 // CreateSourcePackage invokes the type-checker on files and returns a
@@ -165,14 +210,10 @@ func (imp *Importer) LoadPackage(importPath string) (*PackageInfo, error) {
 // importPath is the full name under which this package is known, such
 // as appears in an import declaration. e.g. "sync/atomic".
 //
-// The ParseFiles utility may be helpful for parsing a set of Go
-// source files.
+// Postcondition: for result (info, err), info.Err == err.
 //
-// The result is always non-nil; the presence of errors is indicated
-// by the PackageInfo.Err field.
-//
-func (imp *Importer) CreateSourcePackage(importPath string, files []*ast.File) *PackageInfo {
-	pkgInfo := &PackageInfo{
+func (imp *Importer) CreateSourcePackage(importPath string, files []*ast.File) (*PackageInfo, error) {
+	info := &PackageInfo{
 		Files: files,
 		Info: types.Info{
 			Types:      make(map[ast.Expr]types.Type),
@@ -183,7 +224,7 @@ func (imp *Importer) CreateSourcePackage(importPath string, files []*ast.File) *
 			Selections: make(map[*ast.SelectorExpr]*types.Selection),
 		},
 	}
-	pkgInfo.Pkg, pkgInfo.Err = imp.config.TypeChecker.Check(importPath, imp.Fset, files, &pkgInfo.Info)
-	imp.Packages[importPath] = pkgInfo
-	return pkgInfo
+	info.Pkg, info.Err = imp.config.TypeChecker.Check(importPath, imp.Fset, files, &info.Info)
+	imp.Packages[importPath] = info
+	return info, info.Err
 }

importer/source_test.go

@@ -239,7 +239,7 @@ func TestEnclosingFunction(t *testing.T) {
 			"900", "func@2.27"},
 	}
 	for _, test := range tests {
-		imp := importer.New(new(importer.Config)) // (NB: no Loader)
+		imp := importer.New(new(importer.Config)) // (NB: no go/build.Config)
 		f, start, end := findInterval(t, imp.Fset, test.input, test.substr)
 		if f == nil {
 			continue
@@ -249,9 +249,9 @@ func TestEnclosingFunction(t *testing.T) {
 			t.Errorf("EnclosingFunction(%q) not exact", test.substr)
 			continue
 		}
-		info := imp.CreateSourcePackage("main", []*ast.File{f})
-		if info.Err != nil {
-			t.Error(info.Err.Error())
+		info, err := imp.CreateSourcePackage("main", []*ast.File{f})
+		if err != nil {
+			t.Error(err.Error())
 			continue
 		}
 

importer/util.go

@@ -16,6 +16,7 @@ import (
 	"os"
 	"path/filepath"
 	"strings"
+	"sync"
 )
 
 // CreatePackageFromArgs builds an initial Package from a list of
@@ -30,7 +31,7 @@ import (
 func CreatePackageFromArgs(imp *Importer, args []string) (info *PackageInfo, rest []string, err error) {
 	switch {
 	case len(args) == 0:
-		return nil, nil, errors.New("No *.go source files nor package name was specified.")
+		return nil, nil, errors.New("no *.go source files nor package name was specified.")
 
 	case strings.HasSuffix(args[0], ".go"):
 		// % tool a.go b.go ...
@@ -42,8 +43,7 @@ func CreatePackageFromArgs(imp *Importer, args []string) (info *PackageInfo, res
 		files, err = ParseFiles(imp.Fset, ".", args[:i]...)
 		rest = args[i:]
 		if err == nil {
-			info = imp.CreateSourcePackage("main", files)
-			err = info.Err
+			info, err = imp.CreateSourcePackage("main", files)
 		}
 
 	default:
@@ -57,59 +57,57 @@ func CreatePackageFromArgs(imp *Importer, args []string) (info *PackageInfo, res
 	return
 }
 
-// MakeGoBuildLoader returns an implementation of the SourceLoader
-// function prototype that locates packages using the go/build
-// libraries.  It may return nil upon gross misconfiguration
-// (e.g. os.Getwd() failed).
-//
-// ctxt specifies the go/build.Context to use; if nil, the default
-// Context is used.
-//
-func MakeGoBuildLoader(ctxt *build.Context) SourceLoader {
-	srcDir, err := os.Getwd()
+var cwd string
+
+func init() {
+	var err error
+	cwd, err = os.Getwd()
 	if err != nil {
-		return nil // serious misconfiguration
+		panic("getcwd failed: " + err.Error())
 	}
-	if ctxt == nil {
-		ctxt = &build.Default
+}
+
+// loadPackage ascertains which files belong to package path, then
+// loads, parses and returns them.
+func loadPackage(ctxt *build.Context, fset *token.FileSet, path string) (files []*ast.File, err error) {
+	// TODO(adonovan): fix: Do we need cwd? Shouldn't
+	// ImportDir(path) / $GOROOT suffice?
+	bp, err := ctxt.Import(path, cwd, 0)
+	if _, ok := err.(*build.NoGoError); ok {
+		return nil, nil // empty directory
 	}
-	return func(fset *token.FileSet, path string) (files []*ast.File, err error) {
-		// TODO(adonovan): fix: Do we need cwd? Shouldn't
-		// ImportDir(path) / $GOROOT suffice?
-		bp, err := ctxt.Import(path, srcDir, 0)
-		if _, ok := err.(*build.NoGoError); ok {
-			return nil, nil // empty directory
-		}
-		if err != nil {
-			return // import failed
-		}
-		files, err = ParseFiles(fset, bp.Dir, bp.GoFiles...)
-		if err != nil {
-			return nil, err
-		}
-		return
+	if err != nil {
+		return // import failed
 	}
+	return ParseFiles(fset, bp.Dir, bp.GoFiles...)
 }
 
 // ParseFiles parses the Go source files files within directory dir
 // and returns their ASTs, or the first parse error if any.
 //
-// This utility function is provided to facilitate implementing a
-// SourceLoader.
-//
-func ParseFiles(fset *token.FileSet, dir string, files ...string) (parsed []*ast.File, err error) {
-	for _, file := range files {
-		var f *ast.File
+func ParseFiles(fset *token.FileSet, dir string, files ...string) ([]*ast.File, error) {
+	var wg sync.WaitGroup
+	n := len(files)
+	parsed := make([]*ast.File, n, n)
+	errors := make([]error, n, n)
+	for i, file := range files {
 		if !filepath.IsAbs(file) {
 			file = filepath.Join(dir, file)
 		}
-		f, err = parser.ParseFile(fset, file, nil, parser.DeclarationErrors)
-		if err != nil {
-			return // parsing failed
-		}
-		parsed = append(parsed, f)
+		wg.Add(1)
+		go func(i int, file string) {
+			parsed[i], errors[i] = parser.ParseFile(fset, file, nil, parser.DeclarationErrors)
+			wg.Done()
+		}(i, file)
 	}
-	return
+	wg.Wait()
+
+	for _, err := range errors {
+		if err != nil {
+			return nil, err
+		}
+	}
+	return parsed, nil
 }
 
 // ---------- Internal helpers ----------

oracle/oracle.go

@@ -128,11 +128,10 @@ func Query(args []string, mode, pos string, ptalog io.Writer, buildContext *buil
 		return nil, fmt.Errorf("invalid mode type: %q", mode)
 	}
 
-	var loader importer.SourceLoader
-	if minfo.needs&WholeSource != 0 {
-		loader = importer.MakeGoBuildLoader(buildContext)
+	if minfo.needs&WholeSource == 0 {
+		buildContext = nil
 	}
-	imp := importer.New(&importer.Config{Loader: loader})
+	imp := importer.New(&importer.Config{Build: buildContext})
 	o := &oracle{
 		prog:   ssa.NewProgram(imp.Fset, 0),
 		timers: make(map[string]time.Duration),

pointer/pointer_test.go

@@ -12,6 +12,7 @@ import (
 	"bytes"
 	"fmt"
 	"go/ast"
+	"go/build"
 	"go/parser"
 	"go/token"
 	"io/ioutil"
@@ -162,7 +163,7 @@ func findProbe(prog *ssa.Program, probes []probe, e *expectation) *probe {
 }
 
 func doOneInput(input, filename string) bool {
-	impctx := &importer.Config{Loader: importer.MakeGoBuildLoader(nil)}
+	impctx := &importer.Config{Build: &build.Default}
 	imp := importer.New(impctx)
 
 	// Parsing.
@@ -175,9 +176,9 @@ func doOneInput(input, filename string) bool {
 	}
 
 	// Type checking.
-	info := imp.CreateSourcePackage("main", []*ast.File{f})
-	if info.Err != nil {
-		fmt.Println(info.Err.Error())
+	info, err := imp.CreateSourcePackage("main", []*ast.File{f})
+	if err != nil {
+		fmt.Println(err.Error())
 		return false
 	}
 

ssa/builder_test.go

@@ -46,9 +46,9 @@ func main() {
 		return
 	}
 
-	info := imp.CreateSourcePackage("main", []*ast.File{f})
-	if info.Err != nil {
-		t.Error(info.Err.Error())
+	info, err := imp.CreateSourcePackage("main", []*ast.File{f})
+	if err != nil {
+		t.Error(err.Error())
 		return
 	}
 

ssa/example_test.go

@@ -5,12 +5,14 @@
 package ssa_test
 
 import (
-	"code.google.com/p/go.tools/importer"
-	"code.google.com/p/go.tools/ssa"
 	"fmt"
 	"go/ast"
+	"go/build"
 	"go/parser"
 	"os"
+
+	"code.google.com/p/go.tools/importer"
+	"code.google.com/p/go.tools/ssa"
 )
 
 // This program demonstrates how to run the SSA builder on a "Hello,
@@ -41,7 +43,7 @@ func main() {
 }
 `
 	// Construct an importer.  Imports will be loaded as if by 'go build'.
-	imp := importer.New(&importer.Config{Loader: importer.MakeGoBuildLoader(nil)})
+	imp := importer.New(&importer.Config{Build: &build.Default})
 
 	// Parse the input file.
 	file, err := parser.ParseFile(imp.Fset, "hello.go", hello, parser.DeclarationErrors)
@@ -51,9 +53,9 @@ func main() {
 	}
 
 	// Create a "main" package containing one file.
-	info := imp.CreateSourcePackage("main", []*ast.File{file})
-	if info.Err != nil {
-		fmt.Print(info.Err.Error()) // type error
+	info, err := imp.CreateSourcePackage("main", []*ast.File{file})
+	if err != nil {
+		fmt.Print(err.Error()) // type error
 		return
 	}
 

ssa/interp/interp_test.go

@@ -165,9 +165,7 @@ func run(t *testing.T, dir, input string) bool {
 		inputs = append(inputs, dir+i)
 	}
 
-	imp := importer.New(&importer.Config{
-		Loader: importer.MakeGoBuildLoader(nil),
-	})
+	imp := importer.New(&importer.Config{Build: &build.Default})
 	files, err := importer.ParseFiles(imp.Fset, ".", inputs...)
 	if err != nil {
 		t.Errorf("ssa.ParseFiles(%s) failed: %s", inputs, err.Error())
@@ -186,9 +184,9 @@ func run(t *testing.T, dir, input string) bool {
 	}()
 
 	hint = fmt.Sprintf("To dump SSA representation, run:\n%% go build code.google.com/p/go.tools/cmd/ssadump; ./ssadump -build=CFP %s\n", input)
-	info := imp.CreateSourcePackage("main", files)
-	if info.Err != nil {
-		t.Errorf("importer.CreateSourcePackage(%s) failed: %s", inputs, info.Err.Error())
+	info, err := imp.CreateSourcePackage("main", files)
+	if err != nil {
+		t.Errorf("importer.CreateSourcePackage(%s) failed: %s", inputs, err)
 		return false
 	}
 

ssa/source_test.go

@@ -46,9 +46,9 @@ func TestObjValueLookup(t *testing.T) {
 		}
 	}
 
-	info := imp.CreateSourcePackage("main", []*ast.File{f})
-	if info.Err != nil {
-		t.Error(info.Err.Error())
+	info, err := imp.CreateSourcePackage("main", []*ast.File{f})
+	if err != nil {
+		t.Error(err.Error())
 		return
 	}
 
@@ -197,9 +197,9 @@ func TestValueForExpr(t *testing.T) {
 		return
 	}
 
-	info := imp.CreateSourcePackage("main", []*ast.File{f})
-	if info.Err != nil {
-		t.Error(info.Err.Error())
+	info, err := imp.CreateSourcePackage("main", []*ast.File{f})
+	if err != nil {
+		t.Error(err.Error())
 		return
 	}
 

ssa/stdlib_test.go

@@ -53,8 +53,8 @@ func allPackages() []string {
 
 func TestStdlib(t *testing.T) {
 	ctxt := build.Default
-	ctxt.CgoEnabled = false
-	impctx := importer.Config{Loader: importer.MakeGoBuildLoader(&ctxt)}
+	ctxt.CgoEnabled = false // mutating a global!
+	impctx := importer.Config{Build: &ctxt}
 
 	// Load, parse and type-check the program.
 	t0 := time.Now()