mirror of
https://github.com/golang/go
synced 2024-11-17 07:45:09 -07:00
cmd/api: make NewWatcher populate its own package and import metadata
This partially undoes the optimizations of CL 177597, but makes up some of the difference by caching the package list and import metadata and making the initial calls concurrently, including in TestMain. That reduces the critical path from two sequential 'go list' invocations to just one (run many times concurrently), and eliminates the need for assumptions about the consistency of the 'std' dependency graph across platforms (and hard-coded special cases for packages that violate those assumptions). In the process, this simplifies and fixes TestBenchmark (which has been silently broken since CL 164623). This increases 'time go tool dist test api' on my workstation from 0m8.4s / 0m13.8s / 0m1.7s to 0m10.5s / 0m23.1s / 0m5.1s, compared to 0m12.4s / 0m23.2s / 0m4.7s before CL 177597. (That is, this change retains about half of the wall-time speedup, but almost none of the user-time speedup.) Tested manually using 'go test -race -bench=. cmd/api'. Fixes #37951 Change-Id: Icd537e035e725e1ee7c41d97da5c6651233b927e Reviewed-on: https://go-review.googlesource.com/c/go/+/224619 Run-TryBot: Bryan C. Mills <bcmills@google.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Alexander Rakoczy <alex@golang.org>
This commit is contained in:
Bryan C. Mills
parent
2568d323f6
commit
fcb8f8384a
@ -26,6 +26,7 @@ import (
|
||||
"runtime"
|
||||
"sort"
|
||||
"strings"
|
||||
"sync"
|
||||
)
|
||||
|
||||
func goCmd() string {
|
||||
@ -138,68 +139,37 @@ func main() {
|
||||
c.Compiler = build.Default.Compiler
|
||||
}
|
||||
|
||||
var pkgNames []string
|
||||
if flag.NArg() > 0 {
|
||||
pkgNames = flag.Args()
|
||||
} else {
|
||||
stds, err := exec.Command(goCmd(), "list", "std").Output()
|
||||
if err != nil {
|
||||
log.Fatalf("go list std: %v\n%s", err, stds)
|
||||
}
|
||||
for _, pkg := range strings.Fields(string(stds)) {
|
||||
if !internalPkg.MatchString(pkg) {
|
||||
pkgNames = append(pkgNames, pkg)
|
||||
}
|
||||
}
|
||||
walkers := make([]*Walker, len(contexts))
|
||||
var wg sync.WaitGroup
|
||||
for i, context := range contexts {
|
||||
i, context := i, context
|
||||
wg.Add(1)
|
||||
go func() {
|
||||
defer wg.Done()
|
||||
walkers[i] = NewWalker(context, filepath.Join(build.Default.GOROOT, "src"))
|
||||
}()
|
||||
}
|
||||
wg.Wait()
|
||||
|
||||
importDir, importMap := loadImports()
|
||||
|
||||
// The code below assumes that the import map can vary
|
||||
// by package, so that an import in one package (directory) might mean
|
||||
// something different from the same import in another.
|
||||
// While this can happen in GOPATH mode with vendoring,
|
||||
// it is not possible in the standard library: the one importMap
|
||||
// returned by loadImports applies to all packages.
|
||||
// Construct a per-directory importMap that resolves to
|
||||
// that single map for all packages.
|
||||
importMapForDir := make(map[string]map[string]string)
|
||||
for _, dir := range importDir {
|
||||
importMapForDir[dir] = importMap
|
||||
}
|
||||
var featureCtx = make(map[string]map[string]bool) // feature -> context name -> true
|
||||
for _, context := range contexts {
|
||||
w := NewWalker(context, filepath.Join(build.Default.GOROOT, "src"))
|
||||
w.importDir = importDir
|
||||
w.importMap = importMapForDir
|
||||
for _, w := range walkers {
|
||||
pkgNames := w.stdPackages
|
||||
if flag.NArg() > 0 {
|
||||
pkgNames = flag.Args()
|
||||
}
|
||||
|
||||
for _, name := range pkgNames {
|
||||
// Vendored packages do not contribute to our
|
||||
// public API surface.
|
||||
if strings.HasPrefix(name, "vendor/") {
|
||||
pkg, err := w.Import(name)
|
||||
if _, nogo := err.(*build.NoGoError); nogo {
|
||||
continue
|
||||
}
|
||||
// - Package "unsafe" contains special signatures requiring
|
||||
// extra care when printing them - ignore since it is not
|
||||
// going to change w/o a language change.
|
||||
// - We don't care about the API of commands.
|
||||
if name != "unsafe" && !strings.HasPrefix(name, "cmd/") {
|
||||
if name == "runtime/cgo" && !context.CgoEnabled {
|
||||
// w.Import(name) will return nil
|
||||
continue
|
||||
}
|
||||
pkg, err := w.Import(name)
|
||||
if _, nogo := err.(*build.NoGoError); nogo {
|
||||
continue
|
||||
}
|
||||
if err != nil {
|
||||
log.Fatalf("Import(%q): %v", name, err)
|
||||
}
|
||||
w.export(pkg)
|
||||
if err != nil {
|
||||
log.Fatalf("Import(%q): %v", name, err)
|
||||
}
|
||||
w.export(pkg)
|
||||
}
|
||||
|
||||
ctxName := contextName(context)
|
||||
ctxName := contextName(w.context)
|
||||
for _, f := range w.Features() {
|
||||
if featureCtx[f] == nil {
|
||||
featureCtx[f] = make(map[string]bool)
|
||||
@ -368,23 +338,27 @@ func fileFeatures(filename string) []string {
|
||||
var fset = token.NewFileSet()
|
||||
|
||||
type Walker struct {
|
||||
context *build.Context
|
||||
root string
|
||||
scope []string
|
||||
current *types.Package
|
||||
features map[string]bool // set
|
||||
imported map[string]*types.Package // packages already imported
|
||||
importMap map[string]map[string]string // importer dir -> import path -> canonical path
|
||||
importDir map[string]string // canonical import path -> dir
|
||||
context *build.Context
|
||||
root string
|
||||
scope []string
|
||||
current *types.Package
|
||||
features map[string]bool // set
|
||||
imported map[string]*types.Package // packages already imported
|
||||
stdPackages []string // names, omitting "unsafe", internal, and vendored packages
|
||||
importMap map[string]map[string]string // importer dir -> import path -> canonical path
|
||||
importDir map[string]string // canonical import path -> dir
|
||||
|
||||
}
|
||||
|
||||
func NewWalker(context *build.Context, root string) *Walker {
|
||||
return &Walker{
|
||||
w := &Walker{
|
||||
context: context,
|
||||
root: root,
|
||||
features: map[string]bool{},
|
||||
imported: map[string]*types.Package{"unsafe": types.Unsafe},
|
||||
}
|
||||
w.loadImports()
|
||||
return w
|
||||
}
|
||||
|
||||
func (w *Walker) Features() (fs []string) {
|
||||
@ -455,58 +429,112 @@ func tagKey(dir string, context *build.Context, tags []string) string {
|
||||
return key
|
||||
}
|
||||
|
||||
// loadImports returns information about the packages in the standard library
|
||||
// and the packages they themselves import.
|
||||
// importDir maps expanded import path to the directory containing that package.
|
||||
// importMap maps source import path to expanded import path.
|
||||
type listImports struct {
|
||||
stdPackages []string // names, omitting "unsafe", internal, and vendored packages
|
||||
importDir map[string]string // canonical import path → directory
|
||||
importMap map[string]map[string]string // import path → canonical import path
|
||||
}
|
||||
|
||||
var listCache sync.Map // map[string]listImports, keyed by contextName
|
||||
|
||||
// loadImports populates w with information about the packages in the standard
|
||||
// library and the packages they themselves import in w's build context.
|
||||
//
|
||||
// The source import path and expanded import path are identical except for vendored packages.
|
||||
// For example, on return:
|
||||
//
|
||||
// importMap["math"] = "math"
|
||||
// importDir["math"] = "<goroot>/src/math"
|
||||
// w.importMap["math"] = "math"
|
||||
// w.importDir["math"] = "<goroot>/src/math"
|
||||
//
|
||||
// importMap["golang.org/x/net/route"] = "vendor/golang.org/x/net/route"
|
||||
// importDir["vendor/golang.org/x/net/route"] = "<goroot>/src/vendor/golang.org/x/net/route"
|
||||
// w.importMap["golang.org/x/net/route"] = "vendor/golang.org/x/net/route"
|
||||
// w.importDir["vendor/golang.org/x/net/route"] = "<goroot>/src/vendor/golang.org/x/net/route"
|
||||
//
|
||||
// There are a few imports that only appear on certain platforms,
|
||||
// including it turns out x/net/route, and we add those explicitly.
|
||||
func loadImports() (importDir map[string]string, importMap map[string]string) {
|
||||
out, err := exec.Command(goCmd(), "list", "-e", "-deps", "-json", "std").CombinedOutput()
|
||||
if err != nil {
|
||||
log.Fatalf("loading imports: %v\n%s", err, out)
|
||||
// Since the set of packages that exist depends on context, the result of
|
||||
// loadImports also depends on context. However, to improve test running time
|
||||
// the configuration for each environment is cached across runs.
|
||||
func (w *Walker) loadImports() {
|
||||
if w.context == nil {
|
||||
return // test-only Walker; does not use the import map
|
||||
}
|
||||
|
||||
importDir = make(map[string]string)
|
||||
importMap = make(map[string]string)
|
||||
dec := json.NewDecoder(bytes.NewReader(out))
|
||||
for {
|
||||
var pkg struct {
|
||||
ImportPath, Dir string
|
||||
ImportMap map[string]string
|
||||
}
|
||||
err := dec.Decode(&pkg)
|
||||
if err == io.EOF {
|
||||
break
|
||||
}
|
||||
name := contextName(w.context)
|
||||
|
||||
imports, ok := listCache.Load(name)
|
||||
if !ok {
|
||||
cmd := exec.Command(goCmd(), "list", "-e", "-deps", "-json", "std")
|
||||
cmd.Env = listEnv(w.context)
|
||||
out, err := cmd.CombinedOutput()
|
||||
if err != nil {
|
||||
log.Fatalf("go list: invalid output: %v", err)
|
||||
log.Fatalf("loading imports: %v\n%s", err, out)
|
||||
}
|
||||
|
||||
importDir[pkg.ImportPath] = pkg.Dir
|
||||
for k, v := range pkg.ImportMap {
|
||||
importMap[k] = v
|
||||
var stdPackages []string
|
||||
importMap := make(map[string]map[string]string)
|
||||
importDir := make(map[string]string)
|
||||
dec := json.NewDecoder(bytes.NewReader(out))
|
||||
for {
|
||||
var pkg struct {
|
||||
ImportPath, Dir string
|
||||
ImportMap map[string]string
|
||||
}
|
||||
err := dec.Decode(&pkg)
|
||||
if err == io.EOF {
|
||||
break
|
||||
}
|
||||
if err != nil {
|
||||
log.Fatalf("go list: invalid output: %v", err)
|
||||
}
|
||||
|
||||
// - Package "unsafe" contains special signatures requiring
|
||||
// extra care when printing them - ignore since it is not
|
||||
// going to change w/o a language change.
|
||||
// - internal and vendored packages do not contribute to our
|
||||
// API surface.
|
||||
// - 'go list std' does not include commands, which cannot be
|
||||
// imported anyway.
|
||||
if ip := pkg.ImportPath; ip != "unsafe" && !strings.HasPrefix(ip, "vendor/") && !internalPkg.MatchString(ip) {
|
||||
stdPackages = append(stdPackages, ip)
|
||||
}
|
||||
importDir[pkg.ImportPath] = pkg.Dir
|
||||
if len(pkg.ImportMap) > 0 {
|
||||
importMap[pkg.Dir] = make(map[string]string, len(pkg.ImportMap))
|
||||
}
|
||||
for k, v := range pkg.ImportMap {
|
||||
importMap[pkg.Dir][k] = v
|
||||
}
|
||||
}
|
||||
|
||||
sort.Strings(stdPackages)
|
||||
imports = listImports{
|
||||
stdPackages: stdPackages,
|
||||
importMap: importMap,
|
||||
importDir: importDir,
|
||||
}
|
||||
imports, _ = listCache.LoadOrStore(name, imports)
|
||||
}
|
||||
|
||||
// Fixup for vendor packages listed in args above.
|
||||
fixup := []string{
|
||||
"vendor/golang.org/x/net/route",
|
||||
li := imports.(listImports)
|
||||
w.stdPackages = li.stdPackages
|
||||
w.importDir = li.importDir
|
||||
w.importMap = li.importMap
|
||||
}
|
||||
|
||||
// listEnv returns the process environment to use when invoking 'go list' for
|
||||
// the given context.
|
||||
func listEnv(c *build.Context) []string {
|
||||
if c == nil {
|
||||
return os.Environ()
|
||||
}
|
||||
for _, pkg := range fixup {
|
||||
importDir[pkg] = filepath.Join(build.Default.GOROOT, "src", pkg)
|
||||
importMap[strings.TrimPrefix(pkg, "vendor/")] = pkg
|
||||
|
||||
environ := append(os.Environ(),
|
||||
"GOOS="+c.GOOS,
|
||||
"GOARCH="+c.GOARCH)
|
||||
if c.CgoEnabled {
|
||||
environ = append(environ, "CGO_ENABLED=1")
|
||||
} else {
|
||||
environ = append(environ, "CGO_ENABLED=0")
|
||||
}
|
||||
return
|
||||
return environ
|
||||
}
|
||||
|
||||
// Importing is a sentinel taking the place in Walker.imported
|
||||
@ -538,7 +566,7 @@ func (w *Walker) ImportFrom(fromPath, fromDir string, mode types.ImportMode) (*t
|
||||
dir = filepath.Join(w.root, filepath.FromSlash(name))
|
||||
}
|
||||
if fi, err := os.Stat(dir); err != nil || !fi.IsDir() {
|
||||
log.Fatalf("no source in tree for import %q (from import %s in %s): %v", name, fromPath, fromDir, err)
|
||||
log.Panicf("no source in tree for import %q (from import %s in %s): %v", name, fromPath, fromDir, err)
|
||||
}
|
||||
|
||||
context := w.context
|
||||
|
||||
@ -9,16 +9,36 @@ import (
|
||||
"flag"
|
||||
"fmt"
|
||||
"go/build"
|
||||
"internal/testenv"
|
||||
"io/ioutil"
|
||||
"os"
|
||||
"os/exec"
|
||||
"path/filepath"
|
||||
"sort"
|
||||
"strings"
|
||||
"sync"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func TestMain(m *testing.M) {
|
||||
flag.Parse()
|
||||
for _, c := range contexts {
|
||||
c.Compiler = build.Default.Compiler
|
||||
}
|
||||
|
||||
// Warm up the import cache in parallel.
|
||||
var wg sync.WaitGroup
|
||||
for _, context := range contexts {
|
||||
context := context
|
||||
wg.Add(1)
|
||||
go func() {
|
||||
defer wg.Done()
|
||||
_ = NewWalker(context, filepath.Join(build.Default.GOROOT, "src"))
|
||||
}()
|
||||
}
|
||||
wg.Wait()
|
||||
|
||||
os.Exit(m.Run())
|
||||
}
|
||||
|
||||
var (
|
||||
updateGolden = flag.Bool("updategolden", false, "update golden files")
|
||||
)
|
||||
@ -164,25 +184,12 @@ func TestSkipInternal(t *testing.T) {
|
||||
}
|
||||
|
||||
func BenchmarkAll(b *testing.B) {
|
||||
stds, err := exec.Command(testenv.GoToolPath(b), "list", "std").Output()
|
||||
if err != nil {
|
||||
b.Fatal(err)
|
||||
}
|
||||
b.ResetTimer()
|
||||
pkgNames := strings.Fields(string(stds))
|
||||
|
||||
for _, c := range contexts {
|
||||
c.Compiler = build.Default.Compiler
|
||||
}
|
||||
|
||||
for i := 0; i < b.N; i++ {
|
||||
for _, context := range contexts {
|
||||
w := NewWalker(context, filepath.Join(build.Default.GOROOT, "src"))
|
||||
for _, name := range pkgNames {
|
||||
if name != "unsafe" && !strings.HasPrefix(name, "cmd/") && !internalPkg.MatchString(name) {
|
||||
pkg, _ := w.Import(name)
|
||||
w.export(pkg)
|
||||
}
|
||||
for _, name := range w.stdPackages {
|
||||
pkg, _ := w.Import(name)
|
||||
w.export(pkg)
|
||||
}
|
||||
w.Features()
|
||||
}
|
||||
@ -190,9 +197,6 @@ func BenchmarkAll(b *testing.B) {
|
||||
}
|
||||
|
||||
func TestIssue21181(t *testing.T) {
|
||||
for _, c := range contexts {
|
||||
c.Compiler = build.Default.Compiler
|
||||
}
|
||||
for _, context := range contexts {
|
||||
w := NewWalker(context, "testdata/src/issue21181")
|
||||
pkg, err := w.Import("p")
|
||||
@ -205,9 +209,6 @@ func TestIssue21181(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestIssue29837(t *testing.T) {
|
||||
for _, c := range contexts {
|
||||
c.Compiler = build.Default.Compiler
|
||||
}
|
||||
for _, context := range contexts {
|
||||
w := NewWalker(context, "testdata/src/issue29837")
|
||||
_, err := w.Import("p")
|
||||
|
||||
Loading…
Reference in New Issue
Block a user