cmd/bent: remove required dependencies
Make rsync optional with fallback to cp.
Remove use of /usr/bin/time and replace with measuring time directly
from Go.
For golang/go#49207
Change-Id: Ief5a7a90f9460ddec1d5a51c99d4a534e38a5d9c
Reviewed-on: https://go-review.googlesource.com/c/benchmarks/+/361734
Trust: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: David Chase <drchase@google.com>
diff --git a/cmd/bent/README.md b/cmd/bent/README.md
index 2d55f62..213e5db 100644
--- a/cmd/bent/README.md
+++ b/cmd/bent/README.md
@@ -6,13 +6,14 @@
Installation:
```go get github.com/dr2chase/bent```
-Requires commands `rsync` and `/usr/bin/time`.
-Also depends on burntsushi/toml, and expects that Docker is installed and available on the command line.
+Depends on burntsushi/toml, and expects that Docker is installed and available on the command line.
You can avoid the need for Docker with the `-U` command line flag, if you're okay with running benchmarks outside containers.
Alternately, if you wish to only run those benchmarks that can be compiled into a container (this is platform-dependent)
use the -S flag.
+Install `rsync` for slightly improved copy performance.
+
Initial usage :
```
diff --git a/cmd/bent/bent.go b/cmd/bent/bent.go
index 4081800..01e6ec1 100644
--- a/cmd/bent/bent.go
+++ b/cmd/bent/bent.go
@@ -30,7 +30,7 @@
type BenchStat struct {
Name string
- RealTime, UserTime, SysTime int64 // nanoseconds, -1 if missing.
+ RealTime, UserTime, SysTime time.Duration
}
type Benchmark struct {
@@ -81,6 +81,7 @@
var wikiTable = false // emit the tests in a form usable in a wiki table
var explicitAll counterFlag // Include "-a" on "go test -c" test build ; repeating flag causes multiple rebuilds, useful for build benchmarking.
var shuffle = 2 // Dimensionality of (build) shuffling; 0 = none, 1 = per-benchmark, configuration ordering, 2 = bench, config pairs, 3 = across repetitions.
+var haveRsync = true
//go:embed scripts/*
var scripts embed.FS
@@ -187,20 +188,10 @@
flag.Parse()
- // Fail early if either of these commands is missing.
- _, errTime := exec.LookPath("/usr/bin/time")
_, errRsync := exec.LookPath("rsync")
- if errTime != nil && errRsync != nil {
- fmt.Println("This program needs /usr/bin/time and rsync commands to run")
- os.Exit(1)
- }
if errRsync != nil {
- fmt.Println("This program needs the rsync command to run")
- os.Exit(1)
- }
- if errTime != nil {
- fmt.Println("This program needs the /usr/bin/time command to run")
- os.Exit(1)
+ haveRsync = false
+ fmt.Println("Warning: using cp instead of rsync")
}
if requireSandbox {
@@ -567,7 +558,12 @@
config.Disabled = true
}
- cp := exec.Command("rsync", "-a", from+"/", to)
+ var cp *exec.Cmd
+ if haveRsync {
+ cp = exec.Command("rsync", "-a", from+"/", to)
+ } else {
+ cp = exec.Command("cp", "-a", from+"/.", to)
+ }
s, _ = config.runBinary("", cp, false)
if s != "" {
fmt.Println("Error copying directory tree, ", from, to)
diff --git a/cmd/bent/configuration.go b/cmd/bent/configuration.go
index 89f1f25..301c5ec 100644
--- a/cmd/bent/configuration.go
+++ b/cmd/bent/configuration.go
@@ -16,9 +16,9 @@
"os/exec"
"path"
"runtime"
- "strconv"
"strings"
"sync"
+ "time"
)
// Configuration is a structure that holds all the variables necessary to
@@ -172,8 +172,7 @@
}
}
- // Prefix with time for build benchmarking:
- cmd := exec.Command("/usr/bin/time", "-p", gocmd, "test", "-vet=off", "-c")
+ cmd := exec.Command(gocmd, "test", "-vet=off", "-c")
compileTo := path.Join(dirs.wd, dirs.testBinDir, config.benchName(bench))
cmd.Args = append(cmd.Args, "-o", compileTo)
cmd.Args = append(cmd.Args, bench.BuildFlags...)
@@ -206,7 +205,9 @@
defer cleanup(gopath)
+ start := time.Now()
output, err := cmd.CombinedOutput()
+ realTime := time.Since(start)
if err != nil {
s := ""
switch e := err.(type) {
@@ -220,12 +221,13 @@
return s + "(" + bench.Name + ")\n"
}
soutput := string(output)
- // Capture times from the end of the output.
- rbt := extractTime(soutput, "real")
- ubt := extractTime(soutput, "user")
- sbt := extractTime(soutput, "sys")
- config.buildStats = append(config.buildStats,
- BenchStat{Name: bench.Name, RealTime: rbt, UserTime: ubt, SysTime: sbt})
+ bs := BenchStat{
+ Name: bench.Name,
+ RealTime: realTime,
+ UserTime: cmd.ProcessState.UserTime(),
+ SysTime: cmd.ProcessState.SystemTime(),
+ }
+ config.buildStats = append(config.buildStats, bs)
// Report and record build stats to testbin
@@ -239,7 +241,7 @@
buf.WriteString(s)
}
s := fmt.Sprintf("Benchmark%s 1 %d build-real-ns/op %d build-user-ns/op %d build-sys-ns/op\n",
- strings.Title(bench.Name), rbt, ubt, sbt)
+ strings.Title(bench.Name), bs.RealTime.Nanoseconds(), bs.UserTime.Nanoseconds(), bs.SysTime.Nanoseconds())
if verbose > 0 {
fmt.Print(s)
}
@@ -365,30 +367,3 @@
}
return "", rc
}
-
-// extractTime extracts a time (from /usr/bin/time -p) based on the tag
-// and returns the time converted to nanoseconds. Missing times and bad
-// data result in NaN.
-func extractTime(output, label string) int64 {
- // find tag in first column
- li := strings.LastIndex(output, label)
- if li < 0 {
- return -1
- }
- output = output[li+len(label):]
- // lose intervening white space
- li = strings.IndexAny(output, "0123456789-.eEdD")
- if li < 0 {
- return -1
- }
- output = output[li:]
- li = strings.IndexAny(output, "\n\r\t ")
- if li >= 0 { // failing to find EOL is a special case of done.
- output = output[:li]
- }
- x, err := strconv.ParseFloat(output, 64)
- if err != nil {
- return -1
- }
- return int64(x * 1000 * 1000 * 1000)
-}
