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>
This commit is contained in:
parent
b81ea3a5e1
commit
5a2875abe7
|
@ -100,10 +100,10 @@ type Config struct {
|
||||||
|
|
||||||
// FindPackage is called during Load to create the build.Package
|
// FindPackage is called during Load to create the build.Package
|
||||||
// for a given import path from a given directory.
|
// for a given import path from a given directory.
|
||||||
// If FindPackage is nil, a default implementation
|
// If FindPackage is nil, (*build.Context).Import is used.
|
||||||
// based on ctxt.Import is used. A client may use this hook to
|
// A client may use this hook to adapt to a proprietary build
|
||||||
// adapt to a proprietary build system that does not follow the
|
// system that does not follow the "go build" layout
|
||||||
// "go build" layout conventions, for example.
|
// conventions, for example.
|
||||||
//
|
//
|
||||||
// It must be safe to call concurrently from multiple goroutines.
|
// It must be safe to call concurrently from multiple goroutines.
|
||||||
FindPackage func(ctxt *build.Context, fromDir, importPath string, mode build.ImportMode) (*build.Package, error)
|
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.
|
// findpkg is a memoization of FindPackage.
|
||||||
findpkgMu sync.Mutex // guards findpkg
|
findpkgMu sync.Mutex // guards findpkg
|
||||||
findpkg map[findpkgKey]findpkgValue
|
findpkg map[findpkgKey]*findpkgValue
|
||||||
|
|
||||||
importedMu sync.Mutex // guards imported
|
importedMu sync.Mutex // guards imported
|
||||||
imported map[string]*importInfo // all imported packages (incl. failures) by import path
|
imported map[string]*importInfo // all imported packages (incl. failures) by import path
|
||||||
|
@ -403,6 +403,7 @@ type findpkgKey struct {
|
||||||
}
|
}
|
||||||
|
|
||||||
type findpkgValue struct {
|
type findpkgValue struct {
|
||||||
|
ready chan struct{} // closed to broadcast readiness
|
||||||
bp *build.Package
|
bp *build.Package
|
||||||
err error
|
err error
|
||||||
}
|
}
|
||||||
|
@ -470,15 +471,7 @@ func (conf *Config) Load() (*Program, error) {
|
||||||
|
|
||||||
// Install default FindPackage hook using go/build logic.
|
// Install default FindPackage hook using go/build logic.
|
||||||
if conf.FindPackage == nil {
|
if conf.FindPackage == nil {
|
||||||
conf.FindPackage = func(ctxt *build.Context, path, fromDir string, mode build.ImportMode) (*build.Package, error) {
|
conf.FindPackage = (*build.Context).Import
|
||||||
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
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
prog := &Program{
|
prog := &Program{
|
||||||
|
@ -491,7 +484,7 @@ func (conf *Config) Load() (*Program, error) {
|
||||||
imp := importer{
|
imp := importer{
|
||||||
conf: conf,
|
conf: conf,
|
||||||
prog: prog,
|
prog: prog,
|
||||||
findpkg: make(map[findpkgKey]findpkgValue),
|
findpkg: make(map[findpkgKey]*findpkgValue),
|
||||||
imported: make(map[string]*importInfo),
|
imported: make(map[string]*importInfo),
|
||||||
start: time.Now(),
|
start: time.Now(),
|
||||||
graph: make(map[string]map[string]bool),
|
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
|
// findPackage locates the package denoted by the importPath in the
|
||||||
// specified directory.
|
// specified directory.
|
||||||
func (imp *importer) findPackage(importPath, fromDir string, mode build.ImportMode) (*build.Package, error) {
|
func (imp *importer) findPackage(importPath, fromDir string, mode build.ImportMode) (*build.Package, error) {
|
||||||
// TODO(adonovan): opt: non-blocking duplicate-suppressing cache.
|
// We use a non-blocking duplicate-suppressing cache (gopl.io §9.7)
|
||||||
// i.e. don't hold the lock around FindPackage.
|
// to avoid holding the lock around FindPackage.
|
||||||
key := findpkgKey{importPath, fromDir, mode}
|
key := findpkgKey{importPath, fromDir, mode}
|
||||||
imp.findpkgMu.Lock()
|
imp.findpkgMu.Lock()
|
||||||
defer imp.findpkgMu.Unlock()
|
|
||||||
v, ok := imp.findpkg[key]
|
v, ok := imp.findpkg[key]
|
||||||
if !ok {
|
if ok {
|
||||||
bp, err := imp.conf.FindPackage(imp.conf.build(), importPath, fromDir, mode)
|
// cache hit
|
||||||
v = findpkgValue{bp, err}
|
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.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
|
return v.bp, v.err
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in New Issue