cmd/guru: parallelize loop in globalReferrersPkgLevel

This change parallelizes the outer loop in globalReferrersPkgLevel,
which loops over packages to inspect.

There is also an easily parallelizable inner loop.
However, parallelizing it adds complication
(deffiles needs a mutex, inQueryPackage requires a wait group)
and offers only a 2% speed-up.

Benchmarks for this change, looking for encoding/json.MarshalIndent:

name       old time/op       new time/op       delta
Referrers        5.31s ± 2%        4.67s ± 3%  -11.95%  (p=0.000 n=10+10)

name       old user-time/op  new user-time/op  delta
Referrers        15.9s ± 2%        16.5s ± 3%   +3.71%  (p=0.000 n=10+10)

name       old sys-time/op   new sys-time/op   delta
Referrers        15.7s ± 3%        16.1s ± 3%   +2.73%  (p=0.011 n=10+10)


Fixes golang/go#24272
Updates golang/go#25017


This work supported by Sourcegraph.

Change-Id: I5dcda9017103cdff59d0ffdf5e87d2c2c955a33a
Reviewed-on: https://go-review.googlesource.com/108878
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
This commit is contained in:
Josh Bleecher Snyder 2018-04-23 11:53:40 -07:00
parent 1e1ec013b9
commit d4c6246f3e
2 changed files with 175 additions and 164 deletions

View File

@ -75,7 +75,7 @@ type Query struct {
PTALog io.Writer // (optional) pointer-analysis log file PTALog io.Writer // (optional) pointer-analysis log file
Reflection bool // model reflection soundly (currently slow). Reflection bool // model reflection soundly (currently slow).
// result-printing function // result-printing function, safe for concurrent use
Output func(*token.FileSet, QueryResult) Output func(*token.FileSet, QueryResult)
} }

View File

@ -394,21 +394,26 @@ func globalReferrersPkgLevel(q *Query, obj types.Object, fset *token.FileSet) er
namebytes := []byte(name) // byte slice version of query object name, for early filtering namebytes := []byte(name) // byte slice version of query object name, for early filtering
objpos := fset.Position(obj.Pos()) // position of query object, used to prevent re-emitting original decl objpos := fset.Position(obj.Pos()) // position of query object, used to prevent re-emitting original decl
var files []string // reusable list of files sema := make(chan struct{}, 20) // counting semaphore to limit I/O concurrency
var pkgnames []string // reusable list of names the package is imported under var wg sync.WaitGroup
for u := range users { for u := range users {
u := u
wg.Add(1)
go func() {
defer wg.Done()
uIsXTest := strings.HasSuffix(u, "!test") // indicates whether this package is the special defpkg xtest package uIsXTest := strings.HasSuffix(u, "!test") // indicates whether this package is the special defpkg xtest package
u = strings.TrimSuffix(u, "!test") u = strings.TrimSuffix(u, "!test")
// Resolve package. // Resolve package.
sema <- struct{}{} // acquire token
pkg, err := q.Build.Import(u, cwd, build.IgnoreVendor) pkg, err := q.Build.Import(u, cwd, build.IgnoreVendor)
<-sema // release token
if err != nil { if err != nil {
continue return
} }
files = files[:0]
// If we're not in the query package, // If we're not in the query package,
// the object is in another package regardless, // the object is in another package regardless,
// so we want to process all files. // so we want to process all files.
@ -417,6 +422,7 @@ func globalReferrersPkgLevel(q *Query, obj types.Object, fset *token.FileSet) er
// part of that query package; // part of that query package;
// that set depends on whether the query package itself is an xtest. // that set depends on whether the query package itself is an xtest.
inQueryPkg := u == defpkg && isxtest == uIsXTest inQueryPkg := u == defpkg && isxtest == uIsXTest
var files []string
if !inQueryPkg || !isxtest { if !inQueryPkg || !isxtest {
files = append(files, pkg.GoFiles...) files = append(files, pkg.GoFiles...)
files = append(files, pkg.TestGoFiles...) files = append(files, pkg.TestGoFiles...)
@ -427,10 +433,10 @@ func globalReferrersPkgLevel(q *Query, obj types.Object, fset *token.FileSet) er
} }
if len(files) == 0 { if len(files) == 0 {
continue return
} }
var deffiles map[string]*ast.File // set of files that are part of this package, for inQueryPkg only var deffiles map[string]*ast.File
if inQueryPkg { if inQueryPkg {
deffiles = make(map[string]*ast.File) deffiles = make(map[string]*ast.File)
} }
@ -439,7 +445,9 @@ func globalReferrersPkgLevel(q *Query, obj types.Object, fset *token.FileSet) er
if !buildutil.IsAbsPath(q.Build, file) { if !buildutil.IsAbsPath(q.Build, file) {
file = buildutil.JoinPath(q.Build, pkg.Dir, file) file = buildutil.JoinPath(q.Build, pkg.Dir, file)
} }
sema <- struct{}{} // acquire token
src, err := readFile(q.Build, file) src, err := readFile(q.Build, file)
<-sema // release token
if err != nil { if err != nil {
continue continue
} }
@ -472,7 +480,7 @@ func globalReferrersPkgLevel(q *Query, obj types.Object, fset *token.FileSet) er
// pkgnames is the set of names by which defpkg is imported in this file. // pkgnames is the set of names by which defpkg is imported in this file.
// (Multiple imports in the same file are legal but vanishingly rare.) // (Multiple imports in the same file are legal but vanishingly rare.)
pkgnames = pkgnames[:0] pkgnames := make([]string, 0, 1)
var isdotimport bool var isdotimport bool
for _, imp := range f.Imports { for _, imp := range f.Imports {
path, err := strconv.Unquote(imp.Path.Value) path, err := strconv.Unquote(imp.Path.Value)
@ -583,8 +591,11 @@ func globalReferrersPkgLevel(q *Query, obj types.Object, fset *token.FileSet) er
} }
deffiles = nil // allow GC deffiles = nil // allow GC
} }
}()
} }
wg.Wait()
return nil return nil
} }