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>

diff --git a/src/cmd/compile/internal/importer/gcimporter.go b/src/cmd/compile/internal/importer/gcimporter.go
index e479bd12d36fdfb5df12347b9e0dd0e1ebff6a47..5d948f03c8a33f928432f030b90f27227c50689e 100644 (file)
--- 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 614fe52caf86f01e24ade5e9508f4c43a86d9c90..2140a9f98c2c4a2f855f130c92e4aa151aaadd97 100644 (file)
--- 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 89f58c5d3eb8670abb8c7c3d7fe3f07491fc138b..e32407374647e379c3ea4ab8a4cc0ca6c98879a3 100644 (file)
--- 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 6190000d02b0bbbc7702d1133d4c5db476b75e36..6a28b25278e87c4cca993af79398d08149347918 100644 (file)
--- 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)
        }