go/loader: reduce contention for findPackage mutex
...by making the cache non-blocking and duplicate-suppressing.
(The bottleneck was introduced while adding vendoring support.)
This reduces the 95th percentile map shard time for type-checking 120K
packages to 8min from 20min.
Also: move the I/O-limiting counting semaphore and the NoGoError check
from FindPackage to findPackage so that all implementations benefit.
Change-Id: I43527122262cf80475dd3212d78c340e1c71e36c
Reviewed-on: https://go-review.googlesource.com/18580
Reviewed-by: Robert Griesemer <gri@golang.org>
diff --git a/go/loader/loader.go b/go/loader/loader.go
index 19e2bbd..b0f8336 100644
--- a/go/loader/loader.go
+++ b/go/loader/loader.go
@@ -100,10 +100,10 @@
// 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 @@
// 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 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 @@
// 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 @@
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 @@
// 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
}
