From e6d0f8b2be22ed54d81fca41cf234418f831ec57 Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Thu, 2 Feb 2023 10:42:46 -0500 Subject: [PATCH] internal/testenv: avoid rebuilding all of std in WriteImportcfg MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead, have the caller pass in an explicit list of the packages (if any) they need. After #47257, a builder running a test does not necessarily have the entire standard library already cached, especially when running tests in sharded mode. testenv.WriteImportcfg used to write an importcfg for the entire standard library — which required rebuilding the entire standard library — even though most tests need only a tiny subset. This reduces the time to test internal/abi with a cold build cache on my workstation from ~16s to ~0.05s. It somewhat increases the time for 'go test go/internal/gcimporter' with a cold cache, from ~43s to ~54s, presumably due to decreased parallelism in rebuilding the standard library and increased overhead in re-resolving the import map. However, 'go test -short' running time remains stable (~5.5s before and after). Fixes #58248. Change-Id: I9be6b61ae6e28b75b53af85207c281bb93b9346f Reviewed-on: https://go-review.googlesource.com/c/go/+/464736 Run-TryBot: Bryan Mills Reviewed-by: Cherry Mui Reviewed-by: Than McIntosh TryBot-Result: Gopher Robot Auto-Submit: Bryan Mills --- src/cmd/internal/archive/archive_test.go | 2 +- src/cmd/link/link_test.go | 22 +++---- src/cmd/objdump/objdump_test.go | 2 +- src/cmd/pack/pack_test.go | 4 +- src/go/internal/gcimporter/gcimporter_test.go | 40 ++--------- src/internal/abi/abi_test.go | 8 +-- src/internal/goroot/importcfg.go | 66 ------------------- src/internal/testenv/testenv.go | 51 ++++++++++---- 8 files changed, 65 insertions(+), 130 deletions(-) delete mode 100644 src/internal/goroot/importcfg.go diff --git a/src/cmd/internal/archive/archive_test.go b/src/cmd/internal/archive/archive_test.go index 0e2c7bca75e0ee..10a3d6ebeb65e4 100644 --- a/src/cmd/internal/archive/archive_test.go +++ b/src/cmd/internal/archive/archive_test.go @@ -113,7 +113,7 @@ func buildGoobj(t *testing.T) goobjPaths { go2src := filepath.Join("testdata", "go2.go") importcfgfile := filepath.Join(buildDir, "importcfg") - testenv.WriteImportcfg(t, importcfgfile, nil) + testenv.WriteImportcfg(t, importcfgfile, nil, go1src, go2src) out, err := testenv.Command(t, gotool, "tool", "compile", "-importcfg="+importcfgfile, "-p=p", "-o", go1obj, go1src).CombinedOutput() if err != nil { diff --git a/src/cmd/link/link_test.go b/src/cmd/link/link_test.go index 4dca2e20d61c5a..b4ef9ada17c682 100644 --- a/src/cmd/link/link_test.go +++ b/src/cmd/link/link_test.go @@ -49,15 +49,16 @@ func main() {} ` tmpdir := t.TempDir() + main := filepath.Join(tmpdir, "main.go") - importcfgfile := filepath.Join(tmpdir, "importcfg") - testenv.WriteImportcfg(t, importcfgfile, nil) - - err := os.WriteFile(filepath.Join(tmpdir, "main.go"), []byte(source), 0666) + err := os.WriteFile(main, []byte(source), 0666) if err != nil { t.Fatalf("failed to write main.go: %v\n", err) } + importcfgfile := filepath.Join(tmpdir, "importcfg") + testenv.WriteImportcfg(t, importcfgfile, nil, main) + cmd := testenv.Command(t, testenv.GoToolPath(t), "tool", "compile", "-importcfg="+importcfgfile, "-p=main", "main.go") cmd.Dir = tmpdir out, err := cmd.CombinedOutput() @@ -101,11 +102,10 @@ func TestIssue28429(t *testing.T) { } } - importcfgfile := filepath.Join(tmpdir, "importcfg") - testenv.WriteImportcfg(t, importcfgfile, nil) - // Compile a main package. write("main.go", "package main; func main() {}") + importcfgfile := filepath.Join(tmpdir, "importcfg") + testenv.WriteImportcfg(t, importcfgfile, nil, filepath.Join(tmpdir, "main.go")) runGo("tool", "compile", "-importcfg="+importcfgfile, "-p=main", "main.go") runGo("tool", "pack", "c", "main.a", "main.o") @@ -243,7 +243,7 @@ void foo() { cflags := strings.Fields(runGo("env", "GOGCCFLAGS")) importcfgfile := filepath.Join(tmpdir, "importcfg") - testenv.WriteImportcfg(t, importcfgfile, nil) + testenv.WriteImportcfg(t, importcfgfile, nil, "runtime") // Compile, assemble and pack the Go and C code. runGo("tool", "asm", "-p=main", "-gensymabis", "-o", "symabis", "x.s") @@ -787,10 +787,10 @@ func TestIndexMismatch(t *testing.T) { mObj := filepath.Join(tmpdir, "main.o") exe := filepath.Join(tmpdir, "main.exe") - importcfgFile := filepath.Join(tmpdir, "stdlib.importcfg") - testenv.WriteImportcfg(t, importcfgFile, nil) + importcfgFile := filepath.Join(tmpdir, "runtime.importcfg") + testenv.WriteImportcfg(t, importcfgFile, nil, "runtime") importcfgWithAFile := filepath.Join(tmpdir, "witha.importcfg") - testenv.WriteImportcfg(t, importcfgWithAFile, map[string]string{"a": aObj}) + testenv.WriteImportcfg(t, importcfgWithAFile, map[string]string{"a": aObj}, "runtime") // Build a program with main package importing package a. cmd := testenv.Command(t, testenv.GoToolPath(t), "tool", "compile", "-importcfg="+importcfgFile, "-p=a", "-o", aObj, aSrc) diff --git a/src/cmd/objdump/objdump_test.go b/src/cmd/objdump/objdump_test.go index 69b4cf4e212ef9..226e74d81e8e1e 100644 --- a/src/cmd/objdump/objdump_test.go +++ b/src/cmd/objdump/objdump_test.go @@ -299,7 +299,7 @@ func TestDisasmGoobj(t *testing.T) { tmp := t.TempDir() importcfgfile := filepath.Join(tmp, "hello.importcfg") - testenv.WriteImportcfg(t, importcfgfile, nil) + testenv.WriteImportcfg(t, importcfgfile, nil, "testdata/fmthello.go") hello := filepath.Join(tmp, "hello.o") args := []string{"tool", "compile", "-p=main", "-importcfg=" + importcfgfile, "-o", hello} diff --git a/src/cmd/pack/pack_test.go b/src/cmd/pack/pack_test.go index be75738093512b..5534a10b37f3cd 100644 --- a/src/cmd/pack/pack_test.go +++ b/src/cmd/pack/pack_test.go @@ -210,7 +210,7 @@ func TestHello(t *testing.T) { } importcfgfile := filepath.Join(dir, "hello.importcfg") - testenv.WriteImportcfg(t, importcfgfile, nil) + testenv.WriteImportcfg(t, importcfgfile, nil, hello) goBin := testenv.GoToolPath(t) run(goBin, "tool", "compile", "-importcfg="+importcfgfile, "-p=main", "hello.go") @@ -284,7 +284,7 @@ func TestLargeDefs(t *testing.T) { goBin := testenv.GoToolPath(t) run(goBin, "tool", "compile", "-importcfg="+importcfgfile, "-p=large", "large.go") run(packPath(t), "grc", "large.a", "large.o") - testenv.WriteImportcfg(t, importcfgfile, map[string]string{"large": filepath.Join(dir, "large.o")}) + testenv.WriteImportcfg(t, importcfgfile, map[string]string{"large": filepath.Join(dir, "large.o")}, "runtime") run(goBin, "tool", "compile", "-importcfg="+importcfgfile, "-p=main", "main.go") run(goBin, "tool", "link", "-importcfg="+importcfgfile, "-L", ".", "-o", "a.out", "main.o") out := run("./a.out") diff --git a/src/go/internal/gcimporter/gcimporter_test.go b/src/go/internal/gcimporter/gcimporter_test.go index 3270f3d68292a5..800c372971acc3 100644 --- a/src/go/internal/gcimporter/gcimporter_test.go +++ b/src/go/internal/gcimporter/gcimporter_test.go @@ -7,7 +7,6 @@ package gcimporter_test import ( "bytes" "fmt" - "internal/goroot" "internal/testenv" "os" "os/exec" @@ -36,7 +35,7 @@ func TestMain(m *testing.M) { // compile runs the compiler on filename, with dirname as the working directory, // and writes the output file to outdirname. // compile gives the resulting package a packagepath of testdata/. -func compile(t *testing.T, dirname, filename, outdirname string, packagefiles map[string]string) string { +func compile(t *testing.T, dirname, filename, outdirname string, packageFiles map[string]string, pkgImports ...string) string { // filename must end with ".go" basename, ok := strings.CutSuffix(filepath.Base(filename), ".go") if !ok { @@ -46,16 +45,9 @@ func compile(t *testing.T, dirname, filename, outdirname string, packagefiles ma outname := filepath.Join(outdirname, objname) importcfgfile := os.DevNull - if len(packagefiles) > 0 { + if len(packageFiles) > 0 || len(pkgImports) > 0 { importcfgfile = filepath.Join(outdirname, basename) + ".importcfg" - importcfg := new(bytes.Buffer) - fmt.Fprintf(importcfg, "# import config") - for k, v := range packagefiles { - fmt.Fprintf(importcfg, "\npackagefile %s=%s\n", k, v) - } - if err := os.WriteFile(importcfgfile, importcfg.Bytes(), 0655); err != nil { - t.Fatal(err) - } + testenv.WriteImportcfg(t, importcfgfile, packageFiles, pkgImports...) } pkgpath := path.Join("testdata", basename) @@ -118,16 +110,7 @@ func TestImportTestdata(t *testing.T) { tmpdir := mktmpdir(t) defer os.RemoveAll(tmpdir) - packageFiles := map[string]string{} - for _, pkg := range wantImports { - export, _ := FindPkg(pkg, "testdata") - if export == "" { - t.Fatalf("no export data found for %s", pkg) - } - packageFiles[pkg] = export - } - - compile(t, "testdata", testfile, filepath.Join(tmpdir, "testdata"), packageFiles) + compile(t, "testdata", testfile, filepath.Join(tmpdir, "testdata"), nil, wantImports...) path := "./testdata/" + strings.TrimSuffix(testfile, ".go") if pkg := testPath(t, path, tmpdir); pkg != nil { @@ -188,11 +171,7 @@ func TestImportTypeparamTests(t *testing.T) { // Compile and import, and compare the resulting package with the package // that was type-checked directly. - pkgFiles, err := goroot.PkgfileMap() - if err != nil { - t.Fatal(err) - } - compile(t, rootDir, entry.Name(), filepath.Join(tmpdir, "testdata"), pkgFiles) + compile(t, rootDir, entry.Name(), filepath.Join(tmpdir, "testdata"), nil, filename) pkgName := strings.TrimSuffix(entry.Name(), ".go") imported := importPkg(t, "./testdata/"+pkgName, tmpdir) checked := checkFile(t, filename, src) @@ -569,13 +548,8 @@ func TestIssue13566(t *testing.T) { t.Fatal(err) } - jsonExport, _ := FindPkg("encoding/json", "testdata") - if jsonExport == "" { - t.Fatalf("no export data found for encoding/json") - } - - compile(t, "testdata", "a.go", testoutdir, map[string]string{"encoding/json": jsonExport}) - compile(t, testoutdir, bpath, testoutdir, map[string]string{"testdata/a": filepath.Join(testoutdir, "a.o")}) + compile(t, "testdata", "a.go", testoutdir, nil, "encoding/json") + compile(t, testoutdir, bpath, testoutdir, map[string]string{"testdata/a": filepath.Join(testoutdir, "a.o")}, "encoding/json") // import must succeed (test for issue at hand) pkg := importPkg(t, "./testdata/b", tmpdir) diff --git a/src/internal/abi/abi_test.go b/src/internal/abi/abi_test.go index f0d8dceb3ec9c1..44b9e78a304181 100644 --- a/src/internal/abi/abi_test.go +++ b/src/internal/abi/abi_test.go @@ -7,7 +7,6 @@ package abi_test import ( "internal/abi" "internal/testenv" - "os/exec" "path/filepath" "strings" "testing" @@ -42,18 +41,19 @@ func TestFuncPCCompileError(t *testing.T) { symabi := filepath.Join(tmpdir, "symabi") obj := filepath.Join(tmpdir, "x.o") + // Write an importcfg file for the dependencies of the package. importcfgfile := filepath.Join(tmpdir, "hello.importcfg") - testenv.WriteImportcfg(t, importcfgfile, nil) + testenv.WriteImportcfg(t, importcfgfile, nil, "internal/abi") // parse assembly code for symabi. - cmd := exec.Command(testenv.GoToolPath(t), "tool", "asm", "-gensymabis", "-o", symabi, asmSrc) + cmd := testenv.Command(t, testenv.GoToolPath(t), "tool", "asm", "-gensymabis", "-o", symabi, asmSrc) out, err := cmd.CombinedOutput() if err != nil { t.Fatalf("go tool asm -gensymabis failed: %v\n%s", err, out) } // compile go code. - cmd = exec.Command(testenv.GoToolPath(t), "tool", "compile", "-importcfg="+importcfgfile, "-p=p", "-symabis", symabi, "-o", obj, goSrc) + cmd = testenv.Command(t, testenv.GoToolPath(t), "tool", "compile", "-importcfg="+importcfgfile, "-p=p", "-symabis", symabi, "-o", obj, goSrc) out, err = cmd.CombinedOutput() if err == nil { t.Fatalf("go tool compile did not fail") diff --git a/src/internal/goroot/importcfg.go b/src/internal/goroot/importcfg.go deleted file mode 100644 index e32407374647e3..00000000000000 --- a/src/internal/goroot/importcfg.go +++ /dev/null @@ -1,66 +0,0 @@ -// Copyright 2022 The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -package goroot - -import ( - "bytes" - "fmt" - "os/exec" - "strings" - "sync" -) - -// Importcfg returns an importcfg file to be passed to the -// Go compiler that contains the cached paths for the .a files for the -// standard library. -func Importcfg() (string, error) { - var icfg bytes.Buffer - - m, err := PkgfileMap() - if err != nil { - return "", err - } - fmt.Fprintf(&icfg, "# import config") - for importPath, export := range m { - fmt.Fprintf(&icfg, "\npackagefile %s=%s", importPath, export) - } - s := icfg.String() - return s, nil -} - -var ( - stdlibPkgfileMap map[string]string - stdlibPkgfileErr error - once sync.Once -) - -// PkgfileMap returns a map of package paths to the location on disk -// of the .a file for the package. -// The caller must not modify the map. -func PkgfileMap() (map[string]string, error) { - once.Do(func() { - m := make(map[string]string) - output, err := exec.Command("go", "list", "-export", "-e", "-f", "{{.ImportPath}} {{.Export}}", "std", "cmd").Output() - if err != nil { - stdlibPkgfileErr = err - } - for _, line := range strings.Split(string(output), "\n") { - if line == "" { - continue - } - sp := strings.SplitN(line, " ", 2) - if len(sp) != 2 { - stdlibPkgfileErr = fmt.Errorf("determining pkgfile map: invalid line in go list output: %q", line) - return - } - importPath, export := sp[0], sp[1] - if export != "" { - m[importPath] = export - } - } - stdlibPkgfileMap = m - }) - return stdlibPkgfileMap, stdlibPkgfileErr -} diff --git a/src/internal/testenv/testenv.go b/src/internal/testenv/testenv.go index 6a28b25278e87c..82fdfb6ff6e6b3 100644 --- a/src/internal/testenv/testenv.go +++ b/src/internal/testenv/testenv.go @@ -11,11 +11,11 @@ package testenv import ( + "bytes" "errors" "flag" "fmt" "internal/cfg" - "internal/goroot" "internal/platform" "os" "os/exec" @@ -347,18 +347,45 @@ func SkipIfOptimizationOff(t testing.TB) { } // WriteImportcfg writes an importcfg file used by the compiler or linker to -// dstPath containing entries for the packages in std and cmd in addition -// to the package to package file mappings in additionalPackageFiles. -func WriteImportcfg(t testing.TB, dstPath string, additionalPackageFiles map[string]string) { - importcfg, err := goroot.Importcfg() - for k, v := range additionalPackageFiles { - importcfg += fmt.Sprintf("\npackagefile %s=%s", k, v) +// dstPath containing entries for the file mappings in packageFiles, as well +// as for the packages transitively imported by the package(s) in pkgs. +// +// pkgs may include any package pattern that is valid to pass to 'go list', +// so it may also be a list of Go source files all in the same directory. +func WriteImportcfg(t testing.TB, dstPath string, packageFiles map[string]string, pkgs ...string) { + t.Helper() + + icfg := new(bytes.Buffer) + icfg.WriteString("# import config\n") + for k, v := range packageFiles { + fmt.Fprintf(icfg, "packagefile %s=%s\n", k, v) } - if err != nil { - t.Fatalf("preparing the importcfg failed: %s", err) + + if len(pkgs) > 0 { + // Use 'go list' to resolve any missing packages and rewrite the import map. + cmd := Command(t, GoToolPath(t), "list", "-export", "-deps", "-f", `{{if ne .ImportPath "command-line-arguments"}}{{if .Export}}{{.ImportPath}}={{.Export}}{{end}}{{end}}`) + cmd.Args = append(cmd.Args, pkgs...) + cmd.Stderr = new(strings.Builder) + out, err := cmd.Output() + if err != nil { + t.Fatalf("%v: %v\n%s", cmd, err, cmd.Stderr) + } + + for _, line := range strings.Split(string(out), "\n") { + if line == "" { + continue + } + importPath, export, ok := strings.Cut(line, "=") + if !ok { + t.Fatalf("invalid line in output from %v:\n%s", cmd, line) + } + if packageFiles[importPath] == "" { + fmt.Fprintf(icfg, "packagefile %s=%s\n", importPath, export) + } + } } - err = os.WriteFile(dstPath, []byte(importcfg), 0655) - if err != nil { - t.Fatalf("writing the importcfg failed: %s", err) + + if err := os.WriteFile(dstPath, icfg.Bytes(), 0666); err != nil { + t.Fatal(err) } }