sweet: implement misc UX improvements
- Give a better error message when running sweet from somewhere other
than x/benchmarks/sweet.
- Reference the actual flag names when referring to them in the run
subcommand's error messages.
- Document where the sweet command expects to run from by default.
- Make -short imply -count=1 if -count is not set.
- Print the number of benchmark runs that will happen.
- Clean up the README a bit.
Change-Id: I5adc5af69b9f4e86e3a79f9bd2735353c6834e0b
Reviewed-on: https://go-review.googlesource.com/c/benchmarks/+/449296
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
diff --git a/sweet/README.md b/sweet/README.md
index 0ce6560..5f350b9 100644
--- a/sweet/README.md
+++ b/sweet/README.md
@@ -12,15 +12,11 @@
### Supported Platforms
-* Linux
-* TODO(mknyszek): Support more.
+* linux/amd64
### Dependencies
The `sweet` tool only depends on having a stable version of Go and `git`.
-If you're testing out a development version of Go with Sweet, don't build sweet
-itself with it! Use what's installed on your system instead, or some known-stable
-version.
Some benchmarks, however, have various requirements for building. Notably
they are:
@@ -78,14 +74,23 @@
`-shell` will cause the tool to print each action it performs as a shell
command. Note that while the shell commands are valid for many systems, they
may depend on tools being available on your system that `sweet` does not
-require (e.g. `git`).
+require.
+
+Note that by default `sweet run` expects to be executed in
+`/path/to/x/benchmarks/sweet`, that is, the root of the Sweet subdirectory in
+the `x/benchmarks` repository.
+To execute it from somewhere else, point `-bench-dir` at
+`/path/to/x/benchmarks/sweet/benchmarks`.
## Tips and Rules of Thumb
+* If you're not confident if your experimental Go toolchain will work with all
+ the benchmarks, try the `-short` flag to run to get much faster feedback on
+ whether each benchmark builds and runs.
* You can expect the benchmarks to take a few hours to run with the default
settings on a somewhat sizable Linux box.
* If a benchmark fails to build, run with `-shell` and copy and re-run the
- command to get output.
+ last command to get full output.
TODO(mknyszek): Dump the output to the terminal.
* If a benchmark fails to run, the output should have been captured in the
corresponding results file (e.g. if biogo-igor failed, check
@@ -97,7 +102,7 @@
These benchmarks generally try to stress the Go runtime in interesting ways, and
some may end up with very large heaps. Therefore, it's recommended to run the
-suite on a system with at least 30 GiB of RAM available to minimize the chance
+suite on a system with at least 16 GiB of RAM available to minimize the chance
that results are lost due to an out-of-memory error.
## Configuration Format
@@ -144,7 +149,7 @@
### Tips for Reducing Noise
-* Sweet should be run on a system where a [perflock
+* Sweet should be run on a dedicated machine where a [perflock
daemon](https://github.com/aclements/perflock) is running (to avoid noise due
to CPU throttling).
* Avoid running these benchmarks in cloud environments if possible. Generally
@@ -153,14 +158,3 @@
for more details. Try to use dedicated hardware instead.
*Do not* compare results produced by separate invocations of the `sweet` tool.
-
-### Caveats
-
-With the current release there are a few notable caveats when it comes to the
-results and the noisiness of the benchmarks that will addressed with future
-releases. Notably:
-
-* Time for the tile38 benchmarks is quite noisy, but at least consistent.
-* RSS numbers for the gopher-lua and biogo-igor benchmarks are quite noisy.
-* Peak RSS is currently not very reliable.
-* The gVisor startup benchmarks are very noisy.
diff --git a/sweet/cmd/sweet/run.go b/sweet/cmd/sweet/run.go
index 27bcc80..0e69f4a 100644
--- a/sweet/cmd/sweet/run.go
+++ b/sweet/cmd/sweet/run.go
@@ -6,6 +6,7 @@
import (
"archive/zip"
+ "errors"
"flag"
"fmt"
"io"
@@ -39,12 +40,15 @@
const (
runLongDesc = `Execute benchmarks in the suite against GOROOTs provided in TOML configuration
-files.`
+files. Note: by default, this command expects to run from /path/to/x/benchmarks/sweet.`
runUsage = `Usage: %s run [flags] <config> [configs...]
`
)
-const pgoCountDefaultMax = 5
+const (
+ countDefault = 10
+ pgoCountDefaultMax = 5
+)
type runCfg struct {
count int
@@ -142,7 +146,7 @@
func (c *runCmd) SetFlags(f *flag.FlagSet) {
f.StringVar(&c.runCfg.resultsDir, "results", "./results", "location to write benchmark results to")
f.StringVar(&c.runCfg.benchDir, "bench-dir", "./benchmarks", "the benchmarks directory in the sweet source")
- f.StringVar(&c.runCfg.assetsDir, "assets-dir", "", "the directory containing uncompressed assets for sweet benchmarks (overrides -cache)")
+ f.StringVar(&c.runCfg.assetsDir, "assets-dir", "", "a directory containing uncompressed assets for sweet benchmarks, usually for debugging Sweet (overrides -cache)")
f.StringVar(&c.runCfg.workDir, "work-dir", "", "work directory for benchmarks (default: temporary directory)")
f.StringVar(&c.runCfg.assetsCache, "cache", bootstrap.CacheDefault(), "cache location for assets")
f.BoolVar(&c.runCfg.dumpCore, "dump-core", false, "whether to dump core files for each benchmark process when it completes a benchmark")
@@ -152,12 +156,12 @@
f.StringVar(&c.runCfg.perfFlags, "perf-flags", "", "the flags to pass to Linux perf if -perf is set")
f.BoolVar(&c.pgo, "pgo", false, "perform PGO testing; for each config, collect profiles from a baseline run which are used to feed into a generated PGO config")
f.IntVar(&c.runCfg.pgoCount, "pgo-count", 0, "the number of times to run profiling runs for -pgo; defaults to the value of -count if <=5, or 5 if higher")
- f.IntVar(&c.runCfg.count, "count", 10, "the number of times to run each benchmark")
+ f.IntVar(&c.runCfg.count, "count", 0, fmt.Sprintf("the number of times to run each benchmark (default %d)", countDefault))
f.BoolVar(&c.quiet, "quiet", false, "whether to suppress activity output on stderr (no effect on -shell)")
f.BoolVar(&c.printCmd, "shell", false, "whether to print the commands being executed to stdout")
f.BoolVar(&c.stopOnError, "stop-on-error", false, "whether to stop running benchmarks if an error occurs or a benchmark fails")
- f.BoolVar(&c.short, "short", false, "whether to run a short version of the benchmarks for testing")
+ f.BoolVar(&c.short, "short", false, "whether to run a short version of the benchmarks for testing (changes -count to 1)")
f.Var(&c.toRun, "run", "benchmark group or comma-separated list of benchmarks to run")
}
@@ -170,6 +174,13 @@
log.SetCommandTrace(c.printCmd)
log.SetActivityLog(!c.quiet)
+ if c.runCfg.count == 0 {
+ if c.short {
+ c.runCfg.count = 1
+ } else {
+ c.runCfg.count = countDefault
+ }
+ }
if c.runCfg.pgoCount == 0 {
c.runCfg.pgoCount = c.runCfg.count
if c.runCfg.pgoCount > pgoCountDefaultMax {
@@ -189,23 +200,23 @@
// benchmarks potentially changing their current working directory.
c.workDir, err = filepath.Abs(c.workDir)
if err != nil {
- return fmt.Errorf("creating absolute path from provided work root: %w", err)
+ return fmt.Errorf("creating absolute path from provided work root (-work-dir): %w", err)
}
c.benchDir, err = filepath.Abs(c.benchDir)
if err != nil {
- return fmt.Errorf("creating absolute path from benchmarks path: %w", err)
+ return fmt.Errorf("creating absolute path from benchmarks path (-bench-dir): %w", err)
}
c.resultsDir, err = filepath.Abs(c.resultsDir)
if err != nil {
- return fmt.Errorf("creating absolute path from results path: %w", err)
+ return fmt.Errorf("creating absolute path from results path (-results): %w", err)
}
if c.assetsDir != "" {
c.assetsDir, err = filepath.Abs(c.assetsDir)
if err != nil {
- return fmt.Errorf("creating absolute path from assets path: %w", err)
+ return fmt.Errorf("creating absolute path from assets path (-assets-dir): %w", err)
}
if info, err := os.Stat(c.assetsDir); os.IsNotExist(err) {
- return fmt.Errorf("assets not found at %q: did you mean to specify assets-dir?", c.assetsDir)
+ return fmt.Errorf("assets not found at %q: did you forget to run `sweet get`?", c.assetsDir)
} else if err != nil {
return fmt.Errorf("stat assets %q: %v", c.assetsDir, err)
} else if info.Mode()&os.ModeDir == 0 {
@@ -214,18 +225,18 @@
c.assetsFS = os.DirFS(c.assetsDir)
} else {
if c.assetsCache == "" {
- return fmt.Errorf("missing assets cache and assets directory: cannot proceed without assets")
+ return fmt.Errorf("missing assets cache (-cache) and assets directory (-assets-dir): cannot proceed without assets")
}
c.assetsCache, err = filepath.Abs(c.assetsCache)
if err != nil {
- return fmt.Errorf("creating absolute path from assets cache path: %w", err)
+ return fmt.Errorf("creating absolute path from assets cache path (-cache): %w", err)
}
if info, err := os.Stat(c.assetsCache); os.IsNotExist(err) {
- return fmt.Errorf("assets not found at %q: forgot to run `sweet get`?", c.assetsDir)
+ return fmt.Errorf("assets not found at %q (-assets-dir): did you forget to run `sweet get`?", c.assetsDir)
} else if err != nil {
return fmt.Errorf("stat assets %q: %v", c.assetsDir, err)
} else if info.Mode()&os.ModeDir == 0 {
- return fmt.Errorf("%q is not a directory", c.assetsDir)
+ return fmt.Errorf("%q (-assets-dir) is not a directory", c.assetsDir)
}
assetsFile, err := bootstrap.CachedAssets(c.assetsCache, common.Version)
if err == bootstrap.ErrNotInCache {
@@ -247,6 +258,26 @@
return err
}
}
+ // Validate c.benchDir and provide helpful error messages..
+ if fi, err := os.Stat(c.benchDir); errors.Is(err, fs.ErrNotExist) {
+ return fmt.Errorf("benchmarks directory (-bench-dir) does not exist; did you mean to run this command from x/benchmarks/sweet?")
+ } else if err != nil {
+ return fmt.Errorf("checking benchmarks directory (-bench-dir): %w", err)
+ } else {
+ if !fi.IsDir() {
+ return fmt.Errorf("-bench-dir is not a directory; did you mean to run this command from x/benchmarks/sweet?")
+ }
+ var missing []string
+ for _, b := range allBenchmarks {
+ fi, err := os.Stat(filepath.Join(c.benchDir, b.name))
+ if err != nil || !fi.IsDir() {
+ missing = append(missing, b.name)
+ }
+ }
+ if len(missing) != 0 {
+ return fmt.Errorf("benchmarks directory (-bench-dir) is missing benchmarks (%s); did you mean to run this command from x/benchmarks/sweet?", strings.Join(missing, ", "))
+ }
+ }
log.Printf("Work directory: %s", c.workDir)
// Parse and validate all input TOML configs.
@@ -331,7 +362,12 @@
if len(unknown) != 0 {
return fmt.Errorf("unknown benchmarks: %s", strings.Join(unknown, ", "))
}
- log.Printf("Benchmarks: %s", strings.Join(benchmarkNames(benchmarks), " "))
+ countString := fmt.Sprintf("%d runs", c.runCfg.count)
+ if c.pgo {
+ countString += fmt.Sprintf(", %d pgo runs", c.runCfg.pgoCount)
+ }
+ countString += fmt.Sprintf(" per config (%d)", len(configs))
+ log.Printf("Benchmarks: %s (%s)", strings.Join(benchmarkNames(benchmarks), " "), countString)
// Check prerequisites for each benchmark.
for _, b := range benchmarks {
