diff --git a/go/loader/loader.go b/go/loader/loader.go index 19e2bbd4..b0f8336c 100644 --- a/go/loader/loader.go +++ b/go/loader/loader.go @@ -100,10 +100,10 @@ type Config struct { // FindPackage is called during Load to create the build.Package // for a given import path from a given directory. - // If FindPackage is nil, a default implementation - // based on ctxt.Import is used. A client may use this hook to - // adapt to a proprietary build system that does not follow the - // "go build" layout conventions, for example. + // If FindPackage is nil, (*build.Context).Import is used. + // A client may use this hook to adapt to a proprietary build + // system that does not follow the "go build" layout + // conventions, for example. // // It must be safe to call concurrently from multiple goroutines. FindPackage func(ctxt *build.Context, fromDir, importPath string, mode build.ImportMode) (*build.Package, error) @@ -382,7 +382,7 @@ type importer struct { // findpkg is a memoization of FindPackage. findpkgMu sync.Mutex // guards findpkg - findpkg map[findpkgKey]findpkgValue + findpkg map[findpkgKey]*findpkgValue importedMu sync.Mutex // guards imported imported map[string]*importInfo // all imported packages (incl. failures) by import path @@ -403,8 +403,9 @@ type findpkgKey struct { } type findpkgValue struct { - bp *build.Package - err error + ready chan struct{} // closed to broadcast readiness + bp *build.Package + err error } // importInfo tracks the success or failure of a single import. @@ -470,15 +471,7 @@ func (conf *Config) Load() (*Program, error) { // Install default FindPackage hook using go/build logic. if conf.FindPackage == nil { - conf.FindPackage = func(ctxt *build.Context, path, fromDir string, mode build.ImportMode) (*build.Package, error) { - ioLimit <- true - bp, err := ctxt.Import(path, fromDir, mode) - <-ioLimit - if _, ok := err.(*build.NoGoError); ok { - return bp, nil // empty directory is not an error - } - return bp, err - } + conf.FindPackage = (*build.Context).Import } prog := &Program{ @@ -491,7 +484,7 @@ func (conf *Config) Load() (*Program, error) { imp := importer{ conf: conf, prog: prog, - findpkg: make(map[findpkgKey]findpkgValue), + findpkg: make(map[findpkgKey]*findpkgValue), imported: make(map[string]*importInfo), start: time.Now(), graph: make(map[string]map[string]bool), @@ -803,16 +796,32 @@ func (imp *importer) doImport(from *PackageInfo, to string) (*types.Package, err // findPackage locates the package denoted by the importPath in the // specified directory. func (imp *importer) findPackage(importPath, fromDir string, mode build.ImportMode) (*build.Package, error) { - // TODO(adonovan): opt: non-blocking duplicate-suppressing cache. - // i.e. don't hold the lock around FindPackage. + // We use a non-blocking duplicate-suppressing cache (gopl.io ยง9.7) + // to avoid holding the lock around FindPackage. key := findpkgKey{importPath, fromDir, mode} imp.findpkgMu.Lock() - defer imp.findpkgMu.Unlock() v, ok := imp.findpkg[key] - if !ok { - bp, err := imp.conf.FindPackage(imp.conf.build(), importPath, fromDir, mode) - v = findpkgValue{bp, err} + if ok { + // cache hit + imp.findpkgMu.Unlock() + + <-v.ready // wait for entry to become ready + } else { + // Cache miss: this goroutine becomes responsible for + // populating the map entry and broadcasting its readiness. + v = &findpkgValue{ready: make(chan struct{})} imp.findpkg[key] = v + imp.findpkgMu.Unlock() + + ioLimit <- true + v.bp, v.err = imp.conf.FindPackage(imp.conf.build(), importPath, fromDir, mode) + <-ioLimit + + if _, ok := v.err.(*build.NoGoError); ok { + v.err = nil // empty directory is not an error + } + + close(v.ready) // broadcast ready condition } return v.bp, v.err }