Skip to content

Commit

Permalink
sweet: reenable gVisor benchmark
Browse files Browse the repository at this point in the history
The issue preventing the gVisor benchmark from working has magically
been resolved by us doing nothing. Yay! Reenable the benchmark.

... Except the startup benchmark, which does `runsc run`, doesn't work
on the builders. Womp womp. Disable just that benchmark for now; any
coverage here is useful.

Fixes #51445.
Updates #67508.

Change-Id: Ic0bd03038ef392640bf39c801cff2395678a8216
Cq-Include-Trybots: luci.golang.try:x_benchmarks-gotip-linux-amd64-longtest
Reviewed-on: https://go-review.googlesource.com/c/benchmarks/+/586097
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Michael Pratt <[email protected]>
  • Loading branch information
mknyszek committed May 21, 2024
1 parent f862e5e commit 34853f5
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 15 deletions.
12 changes: 8 additions & 4 deletions sweet/benchmarks/gvisor/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,17 @@ type benchmark interface {

func main1() error {
benchmarks := []benchmark{
startup{},
// TODO(go.dev/issue/67508): Disable the startup benchmark because it doesn't work
// on the builders.
// startup{},
systemCall{500000},
httpServer{20 * time.Second},
}
if cliCfg.short {
benchmarks = []benchmark{
startup{},
// TODO(go.dev/issue/67508): Disable the startup benchmark because it doesn't work
// on the builders.
// startup{},
systemCall{500},
httpServer{1 * time.Second},
}
Expand All @@ -62,8 +66,8 @@ func main1() error {
var buf bytes.Buffer
if err := bench.run(&cliCfg, &buf); err != nil {
if buf.Len() != 0 {
fmt.Fprintln(os.Stderr, "=== Benchmark stdout+stderr ===")
fmt.Fprintln(os.Stderr, buf.String())
fmt.Fprintf(os.Stderr, "=== Benchmark %s stdout+stderr ===", bench.name())
fmt.Fprintf(os.Stderr, "%s\n", buf.String())
}
return err
}
Expand Down
9 changes: 1 addition & 8 deletions sweet/cmd/sweet/benchmark.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,19 +95,12 @@ var benchmarkGroups = func() map[string][]*benchmark {
allBenchmarksMap["etcd"],
allBenchmarksMap["go-build"],
allBenchmarksMap["gopher-lua"],
// TODO(go.dev/issue/51445): Enable once gVisor builds with Go 1.19.
// allBenchmarksMap["gvisor"],
allBenchmarksMap["gvisor"],
allBenchmarksMap["markdown"],
allBenchmarksMap["tile38"],
}

for i := range allBenchmarks {
switch allBenchmarks[i].name {
case "gvisor":
// TODO(go.dev/issue/51445): Include in "all"
// once gVisor builds with Go 1.19.
continue
}
m["all"] = append(m["all"], &allBenchmarks[i])
}

Expand Down
3 changes: 1 addition & 2 deletions sweet/cmd/sweet/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -200,8 +200,7 @@ func TestSweetEndToEnd(t *testing.T) {
{"bleve-index", 1},
{"gopher-lua", 1},
{"markdown", 1},
// TODO(go.dev/issue/51445): Enable once gVisor builds with Go 1.19.
// {"gvisor", 1},
{"gvisor", 1},
} {
sema.Acquire(context.Background(), shard.weight)
wg.Add(1)
Expand Down
2 changes: 1 addition & 1 deletion sweet/harnesses/gvisor.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ func (h GVisor) Get(gcfg *common.GetConfig) error {
gcfg.SrcDir,
"https://github.com/google/gvisor",
"go",
"adc7bb5e1baf4a7489e428e1fad756e5e2aa3410", // release-20220228.0-4233-gadc7bb5e1
"b75aeea", // release-20240513.0-37-g4f08fc481
)
}

Expand Down

2 comments on commit 34853f5

@hliu-ampere
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @mknyszek

The gVisor benchmark fails on ARM platforms due to missing corresponding workloads:

=== Benchmark GVisorSyscall stdout+stderr ===Running scope as unit: test-syscall.scope
*** Warning: sandbox network isn't supported with --rootless, switching to host ***
starting container: starting root container: starting sandbox: creating process: failed to load <workdir>/gvisor/myconfig/assets/syscall/bin/linux-arm64/workload: no such file or directory

error: exit status 128

To unzip assets-v0.3.0.zip:

$ unzip ./assets-v0.3.0.zip 'gvisor/*'
Archive:  ./assets-v0.3.0.zip
...
   creating: gvisor/http/bin/linux-amd64/
  inflating: gvisor/http/bin/linux-amd64/workload 
  ...
   creating: gvisor/syscall/bin/linux-amd64/
  inflating: gvisor/syscall/bin/linux-amd64/workload

There are only workload for linux-amd64, and no linux-arm64.

Could you also fix this problem, or do you know why there is no such workload?

@mknyszek
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do not use GitHub for code review. Please do not comment on commits; this makes the conversation far less visible to others. Please file an issue at https://github.com/golang/go/issues.

Also, we do not currently run the benchmark suite on linux/arm64. We do not have immediate plans to start running it on linux/arm64, though we would like to eventually. We are aware there are issues with building and running on linux/arm64.

Please sign in to comment.