Skip to content

Commit

Permalink
internal/testenv: avoid rebuilding all of std in WriteImportcfg
Browse files Browse the repository at this point in the history
Instead, have the caller pass in an explicit list of the packages
(if any) they need.

After golang#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 golang#58248.

Change-Id: I9be6b61ae6e28b75b53af85207c281bb93b9346f
Reviewed-on: https://go-review.googlesource.com/c/go/+/464736
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
  • Loading branch information
Bryan C. Mills authored and johanbrandhorst committed Feb 12, 2023
1 parent 1654166 commit e6d0f8b
Show file tree
Hide file tree
Showing 8 changed files with 65 additions and 130 deletions.
2 changes: 1 addition & 1 deletion src/cmd/internal/archive/archive_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
22 changes: 11 additions & 11 deletions src/cmd/link/link_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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")

Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/objdump/objdump_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down
4 changes: 2 additions & 2 deletions src/cmd/pack/pack_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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")
Expand Down
40 changes: 7 additions & 33 deletions src/go/internal/gcimporter/gcimporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ package gcimporter_test
import (
"bytes"
"fmt"
"internal/goroot"
"internal/testenv"
"os"
"os/exec"
Expand Down Expand Up @@ -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/<filebasename>.
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 {
Expand All @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
8 changes: 4 additions & 4 deletions src/internal/abi/abi_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ package abi_test
import (
"internal/abi"
"internal/testenv"
"os/exec"
"path/filepath"
"strings"
"testing"
Expand Down Expand Up @@ -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")
Expand Down
66 changes: 0 additions & 66 deletions src/internal/goroot/importcfg.go

This file was deleted.

51 changes: 39 additions & 12 deletions src/internal/testenv/testenv.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@
package testenv

import (
"bytes"
"errors"
"flag"
"fmt"
"internal/cfg"
"internal/goroot"
"internal/platform"
"os"
"os/exec"
Expand Down Expand Up @@ -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)
}
}

0 comments on commit e6d0f8b

Please sign in to comment.