Skip to content

x/benchmarks/sweet: disable gvisor for arm64 by default #67869

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
haoliu-ampere opened this issue Jun 7, 2024 · 8 comments
Closed

x/benchmarks/sweet: disable gvisor for arm64 by default #67869

haoliu-ampere opened this issue Jun 7, 2024 · 8 comments
Assignees
Labels
help wanted NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@haoliu-ampere
Copy link

Go version

go version go1.22.3 linux/arm64

Output of go env in your module/workspace:

GOARCH='arm64'

What did you do?

Currently, x/benchmarks/sweet fails to run gvisor (which is enabled by default) on linux/arm64: ./sweet run ...

What did you see happen?

gvisor only runs on linux/amd64. (see more: golang/benchmarks@34853f5).

What did you expect to see?

I think it's reasonable to only enable gvisor for amd64. E.g., a simple patch would be:

 sweet/cmd/sweet/benchmark.go | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/sweet/cmd/sweet/benchmark.go b/sweet/cmd/sweet/benchmark.go
index 482d6a7..aaa3457 100644
--- a/sweet/cmd/sweet/benchmark.go
+++ b/sweet/cmd/sweet/benchmark.go
@@ -95,10 +95,13 @@ var benchmarkGroups = func() map[string][]*benchmark {
 		allBenchmarksMap["etcd"],
 		allBenchmarksMap["go-build"],
 		allBenchmarksMap["gopher-lua"],
-		allBenchmarksMap["gvisor"],
 		allBenchmarksMap["markdown"],
 		allBenchmarksMap["tile38"],
 	}
+	if runtime.GOARCH == "amd64" {
+		m["default"] = append(m["default"][:7], m["default"][6:]...)
+		m["default"][6] = allBenchmarksMap["gvisor"]
+	}
 
 	for i := range allBenchmarks {
 		m["all"] = append(m["all"], &allBenchmarks[i])

cc @mknyszek

@gopherbot gopherbot added this to the Unreleased milestone Jun 7, 2024
@mknyszek
Copy link
Contributor

mknyszek commented Jun 7, 2024

Seems fine to me, thanks for trying out the benchmarks on arm64. If you're able, please feel free to send a CL (or a PR) to me. See https://go.dev/doc/contribute. Otherwise I'll try to get to it in the near future, but a contribution would be helpful in moving this along faster, if you need it sooner. Thanks.

@mknyszek mknyszek self-assigned this Jun 7, 2024
@mknyszek mknyszek added help wanted NeedsFix The path to resolution is known, but the work has not been done. labels Jun 7, 2024
@haoliu-ampere
Copy link
Author

Thanks. I'd like to contribute. Let me follow that guide ...

@gabyhelp
Copy link

gabyhelp commented Jun 8, 2024

Similar Issues

(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)

@mauri870
Copy link
Member

@haoliu-ampere Are you planning on sending a CL?

@haoliu-ampere
Copy link
Author

@haoliu-ampere Are you planning on sending a CL?

Yes. I'm still waiting for the CLA (maybe some legal concerns).

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/601075 mentions this issue: sweet: only enable gvisor benchmark by default for amd64

@haoliu-ampere
Copy link
Author

haoliu-ampere commented Jul 31, 2024

Hi @mknyszek,

For https://go.dev/cl/601075, it should have been reviewed and voted. But it seems I cannot trigger the Trybots and Submit, could you please help to trigger the test suite and submit the change? Thanks in advance.

Trybots

After an initial reading of your change, maintainers will trigger trybots, a cluster of servers that will run the full test suite on several different architectures. Most trybots complete in a few minutes, at which point a link will be posted in Gerrit where you can see the results.

Submitting an approved change

When a change is ready, a maintainer will submit the change, which adds it as a commit to the Gerrit repository.

@mknyszek
Copy link
Contributor

Sorry for the delay, I've set CQ+1 and Auto-Submit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

5 participants