cmd/bench: move toolchain selection closer to execution
Right now we "iterate" over toolchains (GOROOTs) at the outer-most part
of the tool, but bringing that in closer lets us do things like only
build the benchmarking tools once.
This change also introduces abstractions around the Go tool from the
Sweet tool to simplify and deduplicate some code. For instance, building
bent currently fails with the baseline GOROOT because the GOROOT
environment variable isn't set correctly, but the "gotest" benchmarks
do.
For golang/go#49207.
Change-Id: I6816e1112174f951d3bc22c2b1033b8e98dc0327
Reviewed-on: https://go-review.googlesource.com/c/benchmarks/+/378214
Reviewed-by: Michael Pratt <mpratt@google.com>
Trust: Michael Knyszek <mknyszek@google.com>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
diff --git a/cmd/bench/bent.go b/cmd/bench/bent.go
index 6e20dda..74471e8 100644
--- a/cmd/bench/bent.go
+++ b/cmd/bench/bent.go
@@ -18,23 +18,25 @@
// TODO(prattmic): refactor bent to export Todo so we can directly build this
// in Go.
var configurationTmpl = template.Must(template.New("configuration").Parse(`
+{{- range . -}}
[[Configurations]]
- Name = "Benchmark"
- Root = "{{.}}"
+ Name = "{{.Name}}"
+ Root = "{{.GOROOT}}"
+
+{{end -}}
`))
-func writeConfiguration(filename, goroot string) error {
+func writeBentConfiguration(filename string, tcs []*toolchain) error {
var buf bytes.Buffer
- if err := configurationTmpl.Execute(&buf, goroot); err != nil {
+ if err := configurationTmpl.Execute(&buf, tcs); err != nil {
return fmt.Errorf("error generating configuration: %w", err)
}
- log.Printf("bent configuration for GOROOT %s:\n%s", goroot, buf.String())
+ log.Printf("bent configuration:\n%s", buf.String())
if err := os.WriteFile(filename, buf.Bytes(), 0644); err != nil {
return fmt.Errorf("error creating configurations.toml: %w", err)
}
-
return nil
}
@@ -60,39 +62,34 @@
return os.RemoveAll(dir)
}
-func bent(goroot string) (err error) {
+func bent(tcs []*toolchain) (err error) {
dir, err := os.MkdirTemp("", "bent")
if err != nil {
return fmt.Errorf("error creating temporary directory: %w", err)
}
defer func() {
- err = removeAllIncludingReadonly(dir)
- if err != nil {
+ r := removeAllIncludingReadonly(dir)
+ if r != nil && err == nil {
err = fmt.Errorf("error removing temporary directory: %w", err)
+ } else if r != nil {
+ log.Printf("error removing temporary directory: %v", err)
}
}()
log.Printf("Bent temporary directory: %s", dir)
- bentPath := filepath.Join(dir, "bent")
-
log.Printf("Building bent...")
- // Build bent itself. N.B. we don't need to do this with the goroot
- // under test since we aren't testing bent itself, but we are sure that
- // this toolchain exists.
- //
- // TODO(prattmic): do this only once on first call?
- cmd := goCommand(goroot, "build", "-o", bentPath, "golang.org/x/benchmarks/cmd/bent")
- cmd.Stdout = os.Stdout
- cmd.Stderr = os.Stderr
- if err := cmd.Run(); err != nil {
- return fmt.Errorf("error building bent: %w", err)
+ // Build bent itself. Just pick any toolchain we have; it doesn't matter which.
+ // We're not benchmarking bent itself, it's just a driver.
+ bentPath := filepath.Join(dir, "bent")
+ if err := tcs[0].BuildPackage("golang.org/x/benchmarks/cmd/bent", bentPath); err != nil {
+ return fmt.Errorf("building bent: %w", err)
}
log.Printf("Initializing bent...")
// Initialize scratch dir for bent.
- cmd = exec.Command(bentPath, "-I")
+ cmd := exec.Command(bentPath, "-I")
cmd.Dir = dir
cmd.Stdout = os.Stdout
cmd.Stderr = os.Stderr
@@ -101,13 +98,15 @@
}
confFile := filepath.Join(dir, "configurations.toml")
- if err := writeConfiguration(confFile, goroot); err != nil {
+ if err := writeBentConfiguration(confFile, tcs); err != nil {
return fmt.Errorf("error writing configuration: %w", err)
}
log.Printf("Running bent...")
// Finally we can actually run the benchmarks.
+ // N.B. bent prints the "toolchain" tag to indicate which toolchain is being used.
+ // It's passed to bent via the TOML configuration.
cmd = exec.Command(bentPath, "-C", confFile, "-B", filepath.Join(dir, "benchmarks-50.toml"))
cmd.Dir = dir
cmd.Stdout = os.Stdout
@@ -115,6 +114,5 @@
if err := cmd.Run(); err != nil {
return fmt.Errorf("error running bent -I: %w", err)
}
-
return nil
}
diff --git a/cmd/bench/gotest.go b/cmd/bench/gotest.go
index a98ce20..4044b6b 100644
--- a/cmd/bench/gotest.go
+++ b/cmd/bench/gotest.go
@@ -5,30 +5,18 @@
package main
import (
+ "fmt"
"log"
- "os"
- "strings"
)
-func goTest(goroot string) error {
- log.Printf("Running Go test benchmarks for GOROOT %s", goroot)
-
- cmd := goCommand(goroot, "test", "-v", "-run=none", "-bench=.", "-count=5", "golang.org/x/benchmarks/...")
- cmd.Stdout = os.Stdout
- cmd.Stderr = os.Stderr
-
- env := os.Environ()
- needGOROOT := true
- for i := range env {
- if strings.HasPrefix(env[i], "GOROOT=") {
- env[i] = "GOROOT=" + goroot
- needGOROOT = false
+func goTest(tcs []*toolchain) error {
+ for _, tc := range tcs {
+ log.Printf("Running Go test benchmarks for %s", tc.Name)
+ fmt.Printf("toolchain: %s\n", tc.Name)
+ err := tc.Do("test", "-v", "-run=none", "-bench=.", "-count=5", "golang.org/x/benchmarks/...")
+ if err != nil {
+ return fmt.Errorf("error running gotest with toolchain %s: %w", tc.Name, err)
}
}
- if needGOROOT {
- env = append(env, "GOROOT="+goroot)
- }
- cmd.Env = env
-
- return cmd.Run()
+ return nil
}
diff --git a/cmd/bench/main.go b/cmd/bench/main.go
index 7a3863b..e3bb6f2 100644
--- a/cmd/bench/main.go
+++ b/cmd/bench/main.go
@@ -17,6 +17,8 @@
"os/exec"
"path/filepath"
"strings"
+
+ "golang.org/x/benchmarks/sweet/common"
)
var wait = flag.Bool("wait", true, "wait for system idle before starting benchmarking")
@@ -35,27 +37,37 @@
return strings.TrimSpace(string(b)), nil
}
-func goCommand(goroot string, args ...string) *exec.Cmd {
- bin := filepath.Join(goroot, "bin/go")
- cmd := exec.Command(bin, args...)
- return cmd
+type toolchain struct {
+ *common.Go
+ Name string
}
-func run(goroot string) error {
- log.Printf("GOROOT under test: %s", goroot)
+func toolchainFromGOROOT(name, goroot string) *toolchain {
+ return &toolchain{
+ Go: &common.Go{
+ Tool: filepath.Join(goroot, "bin", "go"),
+ // Update the GOROOT so the wrong one doesn't propagate from
+ // the environment.
+ Env: common.NewEnvFromEnviron().MustSet("GOROOT=" + goroot),
+ PassOutput: true,
+ },
+ Name: name,
+ }
+}
+func run(tcs []*toolchain) error {
+ // Because each of the functions below is responsible for running
+ // benchmarks under each toolchain itself, it is also responsible
+ // for ensuring that the benchmark tag "toolchain" is printed.
pass := true
-
- if err := goTest(goroot); err != nil {
+ if err := goTest(tcs); err != nil {
pass = false
log.Printf("Error running Go tests: %v", err)
}
-
- if err := bent(goroot); err != nil {
+ if err := bent(tcs); err != nil {
pass = false
log.Printf("Error running bent: %v", err)
}
-
if !pass {
return fmt.Errorf("benchmarks failed")
}
@@ -73,29 +85,22 @@
}
}
- goroot, err := determineGOROOT()
+ // Find the toolchain under test.
+ gorootExperiment, err := determineGOROOT()
if err != nil {
log.Fatalf("Unable to determine GOROOT: %v", err)
}
+ toolchains := []*toolchain{toolchainFromGOROOT("experiment", gorootExperiment)}
- fmt.Println("toolchain: experiment")
-
- pass := true
- if err := run(goroot); err != nil {
- pass = false
+ // Find the baseline toolchain, if applicable.
+ gorootBaseline := os.Getenv("BENCH_BASELINE_GOROOT")
+ if gorootBaseline != "" {
+ toolchains = append(toolchains, toolchainFromGOROOT("baseline", gorootBaseline))
}
- baseline := os.Getenv("BENCH_BASELINE_GOROOT")
- if baseline != "" {
- fmt.Println("toolchain: baseline")
-
- if err := run(baseline); err != nil {
- pass = false
- }
- }
-
- if !pass {
- log.Printf("FAIL")
+ // Run benchmarks against the toolchains.
+ if err := run(toolchains); err != nil {
+ log.Print("FAIL")
os.Exit(1)
}
}
diff --git a/cmd/bent/bent.go b/cmd/bent/bent.go
index 4bf0797..964b759 100644
--- a/cmd/bent/bent.go
+++ b/cmd/bent/bent.go
@@ -998,6 +998,7 @@
cmd.Args = append(cmd.Args, moreArgs...)
config.say("shortname: " + b.Name + "\n")
+ config.say("toolchain: " + config.Name + "\n")
s, rc = todo.Configurations[j].runBinary(dirs.wd, cmd, false)
} else {
// docker run --net=none -e GOROOT=... -w /src/github.com/minio/minio/cmd $D /testbin/cmd_Config.test -test.short -test.run=Nope -test.v -test.bench=Benchmark'(Get|Put|List)'
@@ -1020,6 +1021,7 @@
cmd.Args = append(cmd.Args, config.RunFlags...)
cmd.Args = append(cmd.Args, moreArgs...)
config.say("shortname: " + b.Name + "\n")
+ config.say("toolchain: " + config.Name + "\n")
s, rc = todo.Configurations[j].runBinary(dirs.wd, cmd, false)
}
if s != "" {
diff --git a/sweet/common/gotool.go b/sweet/common/gotool.go
index 7caaabc..633092f 100644
--- a/sweet/common/gotool.go
+++ b/sweet/common/gotool.go
@@ -8,13 +8,15 @@
"fmt"
"os"
"os/exec"
+ "path/filepath"
"golang.org/x/benchmarks/sweet/common/log"
)
type Go struct {
- Tool string
- Env *Env
+ Tool string
+ Env *Env
+ PassOutput bool
}
func SystemGoTool() (*Go, error) {
@@ -28,37 +30,41 @@
}, nil
}
-func (g *Go) run(args ...string) error {
+func (g *Go) Do(args ...string) error {
cmd := exec.Command(g.Tool, args...)
cmd.Env = g.Env.Collapse()
+ if g.PassOutput {
+ cmd.Stdout = os.Stdout
+ cmd.Stderr = os.Stderr
+ }
log.TraceCommand(cmd, false)
return cmd.Run()
}
+func (g *Go) GOROOT() string {
+ return filepath.Dir(filepath.Dir(g.Tool))
+}
+
func (g *Go) BuildPackage(pkg, out string) error {
if pkg[0] == '/' || pkg[0] == '.' {
return fmt.Errorf("path used as package in go build")
}
- return g.build(pkg, out)
+ return g.Do("build", "-o", out, pkg)
}
func (g *Go) BuildPath(path, out string) error {
if path[0] != '/' && path[0] != '.' {
path = "./" + path
}
- return g.build(path, out)
-}
-
-func (g *Go) build(pkgOrPath, out string) (err error) {
cwd, err := os.Getwd()
if err != nil {
return fmt.Errorf("failed to get current working directory: %v", err)
}
defer chdir(cwd)
- if err := chdir(pkgOrPath); err != nil {
+ if err := chdir(path); err != nil {
return fmt.Errorf("failed to enter build directory: %v", err)
}
- return g.run("build", "-o", out)
+ return g.Do("build", "-o", out)
}
func chdir(path string) error {
