bytes: Test killed with quit on wasip1-wasm-wazero

[gopherbot]

#!watchflakes
post <- goos == "wasip1" && pkg == "bytes" && `Test killed with quit`

Issue created automatically to collect these failures.

Example (log):

SIGQUIT: quit
PC=0x467a61 m=0 sigcode=0

goroutine 1 [running]:
	goroutine running on other thread; stack unavailable

goroutine 2 [running]:
	goroutine running on other thread; stack unavailable
created by runtime.init.6
	/usr/local/go/src/runtime/proc.go:293 +0x25
...
r12    0x0
r13    0x3
r14    0x8e69a0
r15    0xc00004f800
rip    0x467a61
rflags 0x286
cs     0x33
fs     0x0
gs     0x0
*** Test killed with quit: ran too long (4m0s).

— watchflakes

[gopherbot]

Found new dashboard test flakes for:

#!watchflakes
post <- pkg == "bytes" && test == ""

2023-05-27 02:24 wasip1-wasm-wazero go@7ad92e95 bytes (log)

SIGQUIT: quit
PC=0x467a61 m=0 sigcode=0

goroutine 1 [running]:
	goroutine running on other thread; stack unavailable

goroutine 2 [running]:
	goroutine running on other thread; stack unavailable
created by runtime.init.6
	/usr/local/go/src/runtime/proc.go:293 +0x25
...
r12    0x0
r13    0x3
r14    0x8e69a0
r15    0xc00004f800
rip    0x467a61
rflags 0x286
cs     0x33
fs     0x0
gs     0x0
*** Test killed with quit: ran too long (4m0s).

2023-05-31 16:25 wasip1-wasm-wazero go@d329fc5b bytes (log)

SIGQUIT: quit
PC=0x467a61 m=0 sigcode=0

goroutine 1 [running]:
	goroutine running on other thread; stack unavailable

goroutine 2 [running]:
	goroutine running on other thread; stack unavailable
created by runtime.init.6
	/usr/local/go/src/runtime/proc.go:293 +0x25
...
r12    0x0
r13    0x0
r14    0x8e69a0
r15    0xc000610800
rip    0x467a61
rflags 0x286
cs     0x33
fs     0x0
gs     0x0
*** Test killed with quit: ran too long (4m0s).

— watchflakes

[bcmills]

attn @golang/wasm

[codefromthecrypt]

wazero runs this package, but breaks compilation from run.. Also, it uses the short flag...

$ cd $(gotip env GOROOT)
$ GOOS=wasip1 GOARCH=wasm gotip test -v -c -o bytes.test ./src/bytes
$ wazero run -mount=/:/ -env PWD=$PWD bytes.test -- -test.short -test.v

If the runner is not using the -test.short flag, the problem could be this. I noticed TestCompareBytes
takes a long time (not a short test)

=== RUN   TestCompareBytes
--- PASS: TestCompareBytes (47.26s)

[codefromthecrypt]

TL;DR; we may want to skip heavy, optimizable tests on wazero until its optimizing compiler is finished. Specifically, skip TestCompareBytes and cite tetratelabs/wazero#1496 which should remove the skip.

TestCompareBytes is a heavy test with optimizable code. wasmtime and wasmer have optimizers on by default. While I'm not testing on the same arch as the runner, there's a significant difference on this test when not optimized. I'm using wasmer because its optimization is simpler to disable.

$ wasmer  run --dir=/ --env PWD="$PWD" --env PATH="$PATH" --singlepass bytes.test -- -test.v  -test.run TestCompareBytes
=== RUN   TestCompareBytes
--- PASS: TestCompareBytes (26.72s)
PASS

$ wasmer  run --dir=/ --env PWD="$PWD" --env PATH="$PATH" bytes.test -- -test.v  -test.run TestCompareBytes
=== RUN   TestCompareBytes
--- PASS: TestCompareBytes (16.36s)
PASS

[johanbrandhorst]

I'd prefer not to add runtime specific checks into tests. It seems like the suggestion is that this test sometimes might just take too long to run on wazero (especially on machines that are already running other tests)? I think we can wait and revisit this once wazero has an optimizing compiler in place.

[bcmills]

@codefromthecrypt, by default the Go builders run the test with -short.

But it looks like TestCompareBytes intentionally runs more slowly on builders:
https://cs.opensource.google/go/go/+/master:src/bytes/compare_test.go;l=76;drc=bfa25c3f6c72829cd36f5701418f726702de9c06

Given that we now have much better longtest builder coverage than we did when that check was added, I suggest that we simply remove the || testenv.Builder() != "" from that condition, which will cause it to be skipped on the wasip1 builders (and any other builder than runs with -short too).

[gopherbot]

Change https://go.dev/cl/507416 mentions this issue: bytes: remove builders check from compare test

[codefromthecrypt]

thanks for thinking this through, folks!