sweet: don't fetch large github repos for go-build benchmark if short
Currently the go-build benchmark fetches k8s and istio even if -short is
set. It won't build these repositories in this case, just fetch them. It
appears that fetching them is somewhat flaky, probably GitHub not liking
the fact that we're cloning such large repositories so often. (It's hard
to say why exactly it's failing, but every recently failure in the Sweet
integration test (#56958) is in exactly one of two places: cloning k8s
or istio from GitHub. It's also clear this cloning step is unnecessary
for this test.)
This CL modifies the harness' Get method to accept a struct (aligning
with the other harness methods) that now includes a Short parameter, the
same parameter passed to Build and Run. Now that "short" is plumbed down
into Get, we can skip cloning these repositories.
Fixes #56958.
Change-Id: Ia9e3749ac82608bc24a4a4791955d3a1f546faa1
Reviewed-on: https://go-review.googlesource.com/c/benchmarks/+/498284
Run-TryBot: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
diff --git a/sweet/cmd/sweet/benchmark.go b/sweet/cmd/sweet/benchmark.go
index 5cfec49..3c33803 100644
--- a/sweet/cmd/sweet/benchmark.go
+++ b/sweet/cmd/sweet/benchmark.go
@@ -183,7 +183,11 @@
// multiple times, this will already be done.
_, err := os.Stat(srcDir)
if os.IsNotExist(err) {
- if err := b.harness.Get(srcDir); err != nil {
+ gcfg := &common.GetConfig{
+ SrcDir: srcDir,
+ Short: r.short,
+ }
+ if err := b.harness.Get(gcfg); err != nil {
return fmt.Errorf("retrieving source for %s: %v", b.name, err)
}
}
diff --git a/sweet/common/harness.go b/sweet/common/harness.go
index 525235a..01e7724 100644
--- a/sweet/common/harness.go
+++ b/sweet/common/harness.go
@@ -6,6 +6,21 @@
import "os"
+type GetConfig struct {
+ // SrcDir is the path to the directory that the harness should write
+ // benchmark source into. This is then fed into the BuildConfig.
+ //
+ // The harness should not write benchmark source code from the Sweet
+ // repository here; it need only collect source code it needs to fetch
+ // from a remote source.
+ SrcDir string
+
+ // Short indicates whether or not to run a short version of the benchmarks
+ // for testing. Guaranteed to be the same as BuildConfig.Short and
+ // RunConfig.Short.
+ Short bool
+}
+
type BuildConfig struct {
// BinDir is the path to the directory where all built binaries should be
// placed.
@@ -22,8 +37,8 @@
BenchDir string
// Short indicates whether or not to run a short version of the benchmarks
- // for testing. Guaranteed to be the same as RunConfig.Short for any
- // RunConfig.
+ // for testing. Guaranteed to be the same as GetConfig.Short and
+ // RunConfig.Short.
Short bool
}
@@ -57,8 +72,8 @@
Results *os.File
// Short indicates whether or not to run a short version of the benchmarks
- // for testing. Guaranteed to be the same as BuildConfig.Short for any
- // BuildConfig.
+ // for testing. Guaranteed to be the same as GetConfig.Short and
+ // BuildConfig.Short.
Short bool
}
@@ -70,7 +85,7 @@
CheckPrerequisites() error
// Get retrieves the source code for a benchmark and places it in srcDir.
- Get(srcDir string) error
+ Get(g *GetConfig) error
// Build builds a benchmark and places the binaries in binDir.
Build(cfg *Config, b *BuildConfig) error
diff --git a/sweet/generators/gvisor.go b/sweet/generators/gvisor.go
index b8ca1b1..1727fd2 100644
--- a/sweet/generators/gvisor.go
+++ b/sweet/generators/gvisor.go
@@ -74,7 +74,7 @@
if err := os.MkdirAll(srcDir, os.ModePerm); err != nil {
return err
}
- if err := (harnesses.GVisor{}).Get(srcDir); err != nil {
+ if err := (harnesses.GVisor{}).Get(&common.GetConfig{SrcDir: srcDir}); err != nil {
return err
}
diff --git a/sweet/generators/tile38.go b/sweet/generators/tile38.go
index d16c58e..4bf3aca 100644
--- a/sweet/generators/tile38.go
+++ b/sweet/generators/tile38.go
@@ -78,7 +78,7 @@
if err := os.MkdirAll(srcDir, os.ModePerm); err != nil {
return err
}
- if err := (harnesses.Tile38{}).Get(srcDir); err != nil {
+ if err := (harnesses.Tile38{}).Get(&common.GetConfig{SrcDir: srcDir}); err != nil {
return err
}
diff --git a/sweet/harnesses/etcd.go b/sweet/harnesses/etcd.go
index fb55c2a..dd15ae8 100644
--- a/sweet/harnesses/etcd.go
+++ b/sweet/harnesses/etcd.go
@@ -19,7 +19,7 @@
return nil
}
-func (h Etcd) Get(srcDir string) error {
+func (h Etcd) Get(gcfg *common.GetConfig) error {
// Build against the latest alpha.
//
// Because of the way etcd is released (as a binary blob),
@@ -27,7 +27,7 @@
// Improving performance of these versions doesn't really matter.
// Instead, try to track something close to HEAD.
return gitShallowClone(
- srcDir,
+ gcfg.SrcDir,
"https://github.com/etcd-io/etcd",
"v3.6.0-alpha.0",
)
diff --git a/sweet/harnesses/go-build.go b/sweet/harnesses/go-build.go
index 33cc67b..485ae73 100644
--- a/sweet/harnesses/go-build.go
+++ b/sweet/harnesses/go-build.go
@@ -20,42 +20,48 @@
clone func(outDir string) error
}
-var buildBenchmarks = []*buildBenchmark{
- {
- name: "kubernetes",
- pkg: "cmd/kubelet",
- clone: func(outDir string) error {
- return gitShallowClone(
- outDir,
- "https://github.com/kubernetes/kubernetes",
- "v1.22.1",
- )
+var (
+ buildBenchmarks = []*buildBenchmark{
+ {
+ name: "kubernetes",
+ pkg: "cmd/kubelet",
+ clone: func(outDir string) error {
+ return gitShallowClone(
+ outDir,
+ "https://github.com/kubernetes/kubernetes",
+ "v1.22.1",
+ )
+ },
},
- },
- {
- name: "istio",
- pkg: "istioctl/cmd/istioctl",
- clone: func(outDir string) error {
- return gitShallowClone(
- outDir,
- "https://github.com/istio/istio",
- "1.11.1",
- )
+ {
+ name: "istio",
+ pkg: "istioctl/cmd/istioctl",
+ clone: func(outDir string) error {
+ return gitShallowClone(
+ outDir,
+ "https://github.com/istio/istio",
+ "1.11.1",
+ )
+ },
},
- },
- {
- name: "pkgsite",
- pkg: "cmd/frontend",
- clone: func(outDir string) error {
- return gitCloneToCommit(
- outDir,
- "https://go.googlesource.com/pkgsite",
- "master",
- "0a8194a898a1ceff6a0b29e3419650daf43d8567",
- )
+ {
+ name: "pkgsite",
+ pkg: "cmd/frontend",
+ clone: func(outDir string) error {
+ return gitCloneToCommit(
+ outDir,
+ "https://go.googlesource.com/pkgsite",
+ "master",
+ "0a8194a898a1ceff6a0b29e3419650daf43d8567",
+ )
+ },
},
- },
-}
+ }
+ // For short mode, only build pkgsite. It's the smallest of
+ // the set, and it's hosted on go.googlesource.com, so fetching
+ // source is less likely to be rate-limited causing CI failures.
+ buildBenchmarksShort = []*buildBenchmark{buildBenchmarks[2]}
+)
type GoBuild struct{}
@@ -63,10 +69,10 @@
return nil
}
-func (h GoBuild) Get(srcDir string) error {
+func (h GoBuild) Get(gcfg *common.GetConfig) error {
// Clone the sources that we're going to build.
- for _, bench := range buildBenchmarks {
- if err := bench.clone(filepath.Join(srcDir, bench.name)); err != nil {
+ for _, bench := range goBuildBenchmarks(gcfg.Short) {
+ if err := bench.clone(filepath.Join(gcfg.SrcDir, bench.name)); err != nil {
return err
}
}
@@ -77,11 +83,8 @@
// Local copy of config for updating GOROOT.
cfg := pcfg.Copy()
- benchmarks := buildBenchmarks
- if bcfg.Short {
- // Do only the pkgsite benchmark.
- benchmarks = []*buildBenchmark{buildBenchmarks[2]}
- }
+ // Get the benchmarks we're going to build.
+ benchmarks := goBuildBenchmarks(bcfg.Short)
// cfg.GoRoot is our source toolchain. We need to rebuild cmd/compile
// and cmd/link with cfg.BuildEnv to apply any configured build options
@@ -131,11 +134,7 @@
cfg := pcfg.Copy()
cfg.GoRoot = filepath.Join(rcfg.BinDir, "goroot") // see Build, above.
- benchmarks := buildBenchmarks
- if rcfg.Short {
- // Do only the pkgsite benchmark.
- benchmarks = []*buildBenchmark{buildBenchmarks[2]}
- }
+ benchmarks := goBuildBenchmarks(rcfg.Short)
for _, bench := range benchmarks {
cmd := exec.Command(
filepath.Join(rcfg.BinDir, "go-build-bench"),
@@ -155,3 +154,10 @@
}
return nil
}
+
+func goBuildBenchmarks(short bool) []*buildBenchmark {
+ if short {
+ return buildBenchmarksShort
+ }
+ return buildBenchmarks
+}
diff --git a/sweet/harnesses/gvisor.go b/sweet/harnesses/gvisor.go
index 85be166..31d20c8 100644
--- a/sweet/harnesses/gvisor.go
+++ b/sweet/harnesses/gvisor.go
@@ -24,9 +24,9 @@
return nil
}
-func (h GVisor) Get(srcDir string) error {
+func (h GVisor) Get(gcfg *common.GetConfig) error {
return gitCloneToCommit(
- srcDir,
+ gcfg.SrcDir,
"https://github.com/google/gvisor",
"go",
"adc7bb5e1baf4a7489e428e1fad756e5e2aa3410", // release-20220228.0-4233-gadc7bb5e1
diff --git a/sweet/harnesses/local.go b/sweet/harnesses/local.go
index 5982f6f..d3a0022 100644
--- a/sweet/harnesses/local.go
+++ b/sweet/harnesses/local.go
@@ -23,7 +23,7 @@
return nil
}
-func (h *localBenchHarness) Get(_ string) error {
+func (h *localBenchHarness) Get(_ *common.GetConfig) error {
return nil
}
diff --git a/sweet/harnesses/tile38.go b/sweet/harnesses/tile38.go
index 228789a..4b84c7a 100644
--- a/sweet/harnesses/tile38.go
+++ b/sweet/harnesses/tile38.go
@@ -22,9 +22,9 @@
return nil
}
-func (h Tile38) Get(srcDir string) error {
+func (h Tile38) Get(gcfg *common.GetConfig) error {
return gitShallowClone(
- srcDir,
+ gcfg.SrcDir,
"https://github.com/tidwall/tile38",
"1.29.1",
)
