bytes,strings: tests appear to take ~100x as long on riscv

[mvdan]

I was poking at https://build.golang.org/, and I wondered why the builds for the riscv machines were lagging behind. I understand they may be slower machines, but still, they shouldn't be that slow.

I took a peek at some of the recent logs, such as:

https://build.golang.org/log/c0751a2f215266bf2e29d8987f7a274a4774894d
https://build.golang.org/log/f7b5e9a48eba8ab84a1ac5f77e87dec51f28ef3b
https://build.golang.org/log/ba7516c48279b409253d9742a2efb1fa2e2fa59d

What jumped out to me is how the bytes and strings packages take 100-300s to test, which sounds like something is very wrong. Perhaps they are running huge inputs through unoptimized functions like IndexByte? On most other builders, these packages take 1-2s.

For the rest of the packages, testing on riscv is somewhere in between about as fast and 10x slower. For example, cmd/go includes lots of process execution, filesystem I/O, and Go builds, and it goes from ~30s to ~300s, which is still reasonable. I haven't found a single other package that gets close to being 100x as slow.

Also, at an intuitive level, it does not make any sense that testing cmd/go takes about as long as testing bytes; the former has significantly more tests, even in short mode, and they tend to be integration tests too.

It's unclear to me if we can truly blame the two packages, or if this problem is only occurring on riscv; I haven't done further research.

cc @4a6f656c @rhysh

[ianlancetaylor]

For the strings package essentially all the extra time is taken by TestCompareStrings, which in my testing on a gomote takes 120.50 seconds on riscv64 and 0.50s on amd64. That test boils down to a bunch of calls to runtime_cmpstring in the internal/bytealg package. For amd64 this function is written in assembler. For riscv64 the function is written in Go. Still, the difference in time shouldn't be that large.

The inner loop of the function on riscv64 is

   11b84:	00150513          	addi	a0,a0,1
   11b88:	04555663          	bge	a0,t0,11bd4 <runtime.cmpstring+0x94>
   11b8c:	08657a63          	bgeu	a0,t1,11c20 <runtime.cmpstring+0xe0>
   11b90:	00a485b3          	add	a1,s1,a0
   11b94:	0005c583          	lbu	a1,0(a1)
   11b98:	06757e63          	bgeu	a0,t2,11c14 <runtime.cmpstring+0xd4>
   11b9c:	00a40633          	add	a2,s0,a0
   11ba0:	00064603          	lbu	a2,0(a2)
   11ba4:	00c5ee63          	bltu	a1,a2,11bc0 <runtime.cmpstring+0x80>
   11ba8:	fcb67ee3          	bgeu	a2,a1,11b84 <runtime.cmpstring+0x44>

That code is not optimal, but I have no idea why it would be so very slow.

[mengzhuo]

I can submit a CL for this if you want :)

[ianlancetaylor]

@mengzhuo I don't know what the fix is, do you? If you do know, by all means send a CL. Thanks.

[gopherbot]

Change https://golang.org/cl/379074 mentions this issue: internal/bytealg: improve Compare for riscv64

[gopherbot]

Change https://golang.org/cl/380075 mentions this issue: internal/bytealg: optimise memequal on riscv64

[gopherbot]

Change https://golang.org/cl/380076 mentions this issue: internal/bytealg: optimise compare on riscv64

[mengzhuo]

Updates: after Joel's CLs had merged:

bytes	23.651s
strings	32.728s

[mvdan]

That seems within the 10x slow-down range of some other std packages being tested, so I think we could close this issue if the purpose was just to avoid the testing slowness. Happy to leave it open if anyone thinks there's still improvements to be made for riscv in that package.

[mvdan]

I actually found another similar case. index/suffixarray takes about a second on most amd64 builders (e.g. 0.2s on linux-amd64-fedora, 0.2s on linux-amd64-sid) but it takes much longer on riscv64:

• 255s - https://build.golang.org/log/18f68c7653b868c06c394f7920ca8fade094f08d
• 128s - https://build.golang.org/log/d8826bf2cde61de342c60e66b7774c045a8d9e78

[mengzhuo]

Some other pacakges that slow

crypto/ed25519/internal/edwards25519 39.730s
cmd/go 281.076s
cmd/api 55.184s
image/gif 118.533s

https://build.golang.org/log/185c9831ce42fda738be8ab272cb157aa050e3e4