diff options
author | Bryan C. Mills <bcmills@google.com> | 2022-12-01 13:39:39 -0500 |
---|---|---|
committer | Gopher Robot <gobot@golang.org> | 2022-12-01 22:04:03 +0000 |
commit | 6a70292d1cb3464e5b2c2c03341e5148730a1889 (patch) | |
tree | d4f3d338b461bdd862b4e1efb81cdf95e46ee58e | |
parent | 0e8b6056c99daebcc3c571316a9551c9fab03a00 (diff) | |
download | go-6a70292d1cb3464e5b2c2c03341e5148730a1889.tar.gz go-6a70292d1cb3464e5b2c2c03341e5148730a1889.zip |
go/internal/gcimporter: load cached export data for packages individually
Previously, we were using internal/goroot.PkgfileMap to locate
cached export data. However, PkgfileMap regenerates export data
for the entire standard library, whereas gcimporter may only need
a single package.
Under the new approach, we load the export data (still using
'go list -export') for each GOROOT package individually, avoiding work
to rebuild export data for packages that are not needed.
This is a tradeoff: if most packages in GOROOT are actually needed, we
end up making many more calls to 'go list', and may build packages
sequentially instead of in parallel (but with lower latency to start
using the export data from the earlier packages).
On my workstation, starting from a clean cache for each run,
this reduces the wall time of
'go test go/internal/gcimporter -run=TestImportedTypes'
from 22s real time (2m10s user time) to 6s real (27s user),
and only increases 'go test go/internal/gcimporter' from
28s real (2m16s user) to 30s real (2m19s user).
Updates #56967.
Updates #47257.
Change-Id: I22556acdd9b1acc56533ed4c2728ea29b585c073
Reviewed-on: https://go-review.googlesource.com/c/go/+/454497
Reviewed-by: Michael Matloob <matloob@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
-rw-r--r-- | src/cmd/compile/internal/importer/gcimporter.go | 65 | ||||
-rw-r--r-- | src/go/internal/gcimporter/gcimporter.go | 62 | ||||
-rw-r--r-- | src/internal/goroot/importcfg.go | 8 | ||||
-rw-r--r-- | src/internal/testenv/testenv.go | 2 |
4 files changed, 91 insertions, 46 deletions
diff --git a/src/cmd/compile/internal/importer/gcimporter.go b/src/cmd/compile/internal/importer/gcimporter.go index e479bd12d3..5d948f03c8 100644 --- a/src/cmd/compile/internal/importer/gcimporter.go +++ b/src/cmd/compile/internal/importer/gcimporter.go @@ -7,38 +7,59 @@ package importer import ( "bufio" + "bytes" "fmt" "go/build" - "internal/goroot" "internal/pkgbits" "io" "os" - "path" + "os/exec" "path/filepath" "strings" + "sync" "cmd/compile/internal/types2" ) -func lookupGorootExport(pkgpath, srcRoot, srcDir string) (string, bool) { - pkgpath = filepath.ToSlash(pkgpath) - m, err := goroot.PkgfileMap() - if err != nil { - return "", false - } - if export, ok := m[pkgpath]; ok { - return export, true - } - vendorPrefix := "vendor" - if strings.HasPrefix(srcDir, filepath.Join(srcRoot, "cmd")) { - vendorPrefix = path.Join("cmd", vendorPrefix) +var exportMap sync.Map // package dir → func() (string, bool) + +// lookupGorootExport returns the location of the export data +// (normally found in the build cache, but located in GOROOT/pkg +// in prior Go releases) for the package located in pkgDir. +// +// (We use the package's directory instead of its import path +// mainly to simplify handling of the packages in src/vendor +// and cmd/vendor.) +func lookupGorootExport(pkgDir string) (string, bool) { + f, ok := exportMap.Load(pkgDir) + if !ok { + var ( + listOnce sync.Once + exportPath string + ) + f, _ = exportMap.LoadOrStore(pkgDir, func() (string, bool) { + listOnce.Do(func() { + cmd := exec.Command("go", "list", "-export", "-f", "{{.Export}}", pkgDir) + cmd.Dir = build.Default.GOROOT + var output []byte + output, err := cmd.Output() + if err != nil { + return + } + + exports := strings.Split(string(bytes.TrimSpace(output)), "\n") + if len(exports) != 1 { + return + } + + exportPath = exports[0] + }) + + return exportPath, exportPath != "" + }) } - pkgpath = path.Join(vendorPrefix, pkgpath) - if false { // for debugging - fmt.Fprintln(os.Stderr, "looking up ", pkgpath) - } - export, ok := m[pkgpath] - return export, ok + + return f.(func() (string, bool))() } var pkgExts = [...]string{".a", ".o"} // a file from the build cache will have no extension @@ -64,8 +85,8 @@ func FindPkg(path, srcDir string) (filename, id string) { bp, _ := build.Import(path, srcDir, build.FindOnly|build.AllowBinary) if bp.PkgObj == "" { var ok bool - if bp.Goroot { - filename, ok = lookupGorootExport(path, bp.SrcRoot, srcDir) + if bp.Goroot && bp.Dir != "" { + filename, ok = lookupGorootExport(bp.Dir) } if !ok { id = path // make sure we have an id to print in error message diff --git a/src/go/internal/gcimporter/gcimporter.go b/src/go/internal/gcimporter/gcimporter.go index 614fe52caf..2140a9f98c 100644 --- a/src/go/internal/gcimporter/gcimporter.go +++ b/src/go/internal/gcimporter/gcimporter.go @@ -7,38 +7,62 @@ package gcimporter // import "go/internal/gcimporter" import ( "bufio" + "bytes" "fmt" "go/build" "go/token" "go/types" - "internal/goroot" "internal/pkgbits" "io" "os" - "path" + "os/exec" "path/filepath" "strings" + "sync" ) // debugging/development support const debug = false -func lookupGorootExport(pkgpath, srcRoot, srcDir string) (string, bool) { - pkgpath = filepath.ToSlash(pkgpath) - m, err := goroot.PkgfileMap() - if err != nil { - return "", false - } - if export, ok := m[pkgpath]; ok { - return export, true +var exportMap sync.Map // package dir → func() (string, bool) + +// lookupGorootExport returns the location of the export data +// (normally found in the build cache, but located in GOROOT/pkg +// in prior Go releases) for the package located in pkgDir. +// +// (We use the package's directory instead of its import path +// mainly to simplify handling of the packages in src/vendor +// and cmd/vendor.) +func lookupGorootExport(pkgDir string) (string, bool) { + f, ok := exportMap.Load(pkgDir) + if !ok { + var ( + listOnce sync.Once + exportPath string + ) + f, _ = exportMap.LoadOrStore(pkgDir, func() (string, bool) { + listOnce.Do(func() { + cmd := exec.Command("go", "list", "-export", "-f", "{{.Export}}", pkgDir) + cmd.Dir = build.Default.GOROOT + var output []byte + output, err := cmd.Output() + if err != nil { + return + } + + exports := strings.Split(string(bytes.TrimSpace(output)), "\n") + if len(exports) != 1 { + return + } + + exportPath = exports[0] + }) + + return exportPath, exportPath != "" + }) } - vendorPrefix := "vendor" - if strings.HasPrefix(srcDir, filepath.Join(srcRoot, "cmd")) { - vendorPrefix = path.Join("cmd", vendorPrefix) - } - pkgpath = path.Join(vendorPrefix, pkgpath) - export, ok := m[pkgpath] - return export, ok + + return f.(func() (string, bool))() } var pkgExts = [...]string{".a", ".o"} // a file from the build cache will have no extension @@ -64,8 +88,8 @@ func FindPkg(path, srcDir string) (filename, id string) { bp, _ := build.Import(path, srcDir, build.FindOnly|build.AllowBinary) if bp.PkgObj == "" { var ok bool - if bp.Goroot { - filename, ok = lookupGorootExport(path, bp.SrcRoot, srcDir) + if bp.Goroot && bp.Dir != "" { + filename, ok = lookupGorootExport(bp.Dir) } if !ok { id = path // make sure we have an id to print in error message diff --git a/src/internal/goroot/importcfg.go b/src/internal/goroot/importcfg.go index 89f58c5d3e..e324073746 100644 --- a/src/internal/goroot/importcfg.go +++ b/src/internal/goroot/importcfg.go @@ -24,9 +24,7 @@ func Importcfg() (string, error) { } fmt.Fprintf(&icfg, "# import config") for importPath, export := range m { - if importPath != "unsafe" && export != "" { // unsafe - fmt.Fprintf(&icfg, "\npackagefile %s=%s", importPath, export) - } + fmt.Fprintf(&icfg, "\npackagefile %s=%s", importPath, export) } s := icfg.String() return s, nil @@ -58,7 +56,9 @@ func PkgfileMap() (map[string]string, error) { return } importPath, export := sp[0], sp[1] - m[importPath] = export + if export != "" { + m[importPath] = export + } } stdlibPkgfileMap = m }) diff --git a/src/internal/testenv/testenv.go b/src/internal/testenv/testenv.go index 6190000d02..6a28b25278 100644 --- a/src/internal/testenv/testenv.go +++ b/src/internal/testenv/testenv.go @@ -357,7 +357,7 @@ func WriteImportcfg(t testing.TB, dstPath string, additionalPackageFiles map[str if err != nil { t.Fatalf("preparing the importcfg failed: %s", err) } - os.WriteFile(dstPath, []byte(importcfg), 0655) + err = os.WriteFile(dstPath, []byte(importcfg), 0655) if err != nil { t.Fatalf("writing the importcfg failed: %s", err) } |