Skip to content

Commit 6a70292

Browse files
Bryan C. Millsgopherbot
Bryan C. Mills
authored andcommitted
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 <[email protected]> Auto-Submit: Bryan Mills <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Run-TryBot: Bryan Mills <[email protected]>
1 parent 0e8b605 commit 6a70292

File tree

4 files changed

+91
-46
lines changed

4 files changed

+91
-46
lines changed

src/cmd/compile/internal/importer/gcimporter.go

Lines changed: 43 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -7,38 +7,59 @@ package importer
77

88
import (
99
"bufio"
10+
"bytes"
1011
"fmt"
1112
"go/build"
12-
"internal/goroot"
1313
"internal/pkgbits"
1414
"io"
1515
"os"
16-
"path"
16+
"os/exec"
1717
"path/filepath"
1818
"strings"
19+
"sync"
1920

2021
"cmd/compile/internal/types2"
2122
)
2223

23-
func lookupGorootExport(pkgpath, srcRoot, srcDir string) (string, bool) {
24-
pkgpath = filepath.ToSlash(pkgpath)
25-
m, err := goroot.PkgfileMap()
26-
if err != nil {
27-
return "", false
28-
}
29-
if export, ok := m[pkgpath]; ok {
30-
return export, true
31-
}
32-
vendorPrefix := "vendor"
33-
if strings.HasPrefix(srcDir, filepath.Join(srcRoot, "cmd")) {
34-
vendorPrefix = path.Join("cmd", vendorPrefix)
24+
var exportMap sync.Map // package dir → func() (string, bool)
25+
26+
// lookupGorootExport returns the location of the export data
27+
// (normally found in the build cache, but located in GOROOT/pkg
28+
// in prior Go releases) for the package located in pkgDir.
29+
//
30+
// (We use the package's directory instead of its import path
31+
// mainly to simplify handling of the packages in src/vendor
32+
// and cmd/vendor.)
33+
func lookupGorootExport(pkgDir string) (string, bool) {
34+
f, ok := exportMap.Load(pkgDir)
35+
if !ok {
36+
var (
37+
listOnce sync.Once
38+
exportPath string
39+
)
40+
f, _ = exportMap.LoadOrStore(pkgDir, func() (string, bool) {
41+
listOnce.Do(func() {
42+
cmd := exec.Command("go", "list", "-export", "-f", "{{.Export}}", pkgDir)
43+
cmd.Dir = build.Default.GOROOT
44+
var output []byte
45+
output, err := cmd.Output()
46+
if err != nil {
47+
return
48+
}
49+
50+
exports := strings.Split(string(bytes.TrimSpace(output)), "\n")
51+
if len(exports) != 1 {
52+
return
53+
}
54+
55+
exportPath = exports[0]
56+
})
57+
58+
return exportPath, exportPath != ""
59+
})
3560
}
36-
pkgpath = path.Join(vendorPrefix, pkgpath)
37-
if false { // for debugging
38-
fmt.Fprintln(os.Stderr, "looking up ", pkgpath)
39-
}
40-
export, ok := m[pkgpath]
41-
return export, ok
61+
62+
return f.(func() (string, bool))()
4263
}
4364

4465
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) {
6485
bp, _ := build.Import(path, srcDir, build.FindOnly|build.AllowBinary)
6586
if bp.PkgObj == "" {
6687
var ok bool
67-
if bp.Goroot {
68-
filename, ok = lookupGorootExport(path, bp.SrcRoot, srcDir)
88+
if bp.Goroot && bp.Dir != "" {
89+
filename, ok = lookupGorootExport(bp.Dir)
6990
}
7091
if !ok {
7192
id = path // make sure we have an id to print in error message

src/go/internal/gcimporter/gcimporter.go

Lines changed: 43 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -7,38 +7,62 @@ package gcimporter // import "go/internal/gcimporter"
77

88
import (
99
"bufio"
10+
"bytes"
1011
"fmt"
1112
"go/build"
1213
"go/token"
1314
"go/types"
14-
"internal/goroot"
1515
"internal/pkgbits"
1616
"io"
1717
"os"
18-
"path"
18+
"os/exec"
1919
"path/filepath"
2020
"strings"
21+
"sync"
2122
)
2223

2324
// debugging/development support
2425
const debug = false
2526

26-
func lookupGorootExport(pkgpath, srcRoot, srcDir string) (string, bool) {
27-
pkgpath = filepath.ToSlash(pkgpath)
28-
m, err := goroot.PkgfileMap()
29-
if err != nil {
30-
return "", false
31-
}
32-
if export, ok := m[pkgpath]; ok {
33-
return export, true
27+
var exportMap sync.Map // package dir → func() (string, bool)
28+
29+
// lookupGorootExport returns the location of the export data
30+
// (normally found in the build cache, but located in GOROOT/pkg
31+
// in prior Go releases) for the package located in pkgDir.
32+
//
33+
// (We use the package's directory instead of its import path
34+
// mainly to simplify handling of the packages in src/vendor
35+
// and cmd/vendor.)
36+
func lookupGorootExport(pkgDir string) (string, bool) {
37+
f, ok := exportMap.Load(pkgDir)
38+
if !ok {
39+
var (
40+
listOnce sync.Once
41+
exportPath string
42+
)
43+
f, _ = exportMap.LoadOrStore(pkgDir, func() (string, bool) {
44+
listOnce.Do(func() {
45+
cmd := exec.Command("go", "list", "-export", "-f", "{{.Export}}", pkgDir)
46+
cmd.Dir = build.Default.GOROOT
47+
var output []byte
48+
output, err := cmd.Output()
49+
if err != nil {
50+
return
51+
}
52+
53+
exports := strings.Split(string(bytes.TrimSpace(output)), "\n")
54+
if len(exports) != 1 {
55+
return
56+
}
57+
58+
exportPath = exports[0]
59+
})
60+
61+
return exportPath, exportPath != ""
62+
})
3463
}
35-
vendorPrefix := "vendor"
36-
if strings.HasPrefix(srcDir, filepath.Join(srcRoot, "cmd")) {
37-
vendorPrefix = path.Join("cmd", vendorPrefix)
38-
}
39-
pkgpath = path.Join(vendorPrefix, pkgpath)
40-
export, ok := m[pkgpath]
41-
return export, ok
64+
65+
return f.(func() (string, bool))()
4266
}
4367

4468
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) {
6488
bp, _ := build.Import(path, srcDir, build.FindOnly|build.AllowBinary)
6589
if bp.PkgObj == "" {
6690
var ok bool
67-
if bp.Goroot {
68-
filename, ok = lookupGorootExport(path, bp.SrcRoot, srcDir)
91+
if bp.Goroot && bp.Dir != "" {
92+
filename, ok = lookupGorootExport(bp.Dir)
6993
}
7094
if !ok {
7195
id = path // make sure we have an id to print in error message

src/internal/goroot/importcfg.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,7 @@ func Importcfg() (string, error) {
2424
}
2525
fmt.Fprintf(&icfg, "# import config")
2626
for importPath, export := range m {
27-
if importPath != "unsafe" && export != "" { // unsafe
28-
fmt.Fprintf(&icfg, "\npackagefile %s=%s", importPath, export)
29-
}
27+
fmt.Fprintf(&icfg, "\npackagefile %s=%s", importPath, export)
3028
}
3129
s := icfg.String()
3230
return s, nil
@@ -58,7 +56,9 @@ func PkgfileMap() (map[string]string, error) {
5856
return
5957
}
6058
importPath, export := sp[0], sp[1]
61-
m[importPath] = export
59+
if export != "" {
60+
m[importPath] = export
61+
}
6262
}
6363
stdlibPkgfileMap = m
6464
})

src/internal/testenv/testenv.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -357,7 +357,7 @@ func WriteImportcfg(t testing.TB, dstPath string, additionalPackageFiles map[str
357357
if err != nil {
358358
t.Fatalf("preparing the importcfg failed: %s", err)
359359
}
360-
os.WriteFile(dstPath, []byte(importcfg), 0655)
360+
err = os.WriteFile(dstPath, []byte(importcfg), 0655)
361361
if err != nil {
362362
t.Fatalf("writing the importcfg failed: %s", err)
363363
}

0 commit comments

Comments
 (0)