golang / golang/go

x/tools/go/analysis/passes/modernize: slicesbackward: rewrite is significantly slower and has no per-site opt-out

Open
#80,971 4 comments 0 reactions 1 assignee Claimed by @madelinekalil View on GitHub
Tools
Dominant language
Go
Stars
139k
Forks
19.4k
PR merge metrics
PR metrics pending

Description

### Go version

```console
$ go version
go version go1.27.0 darwin/arm64
```

### Output of `go env` in your module/workspace

```console
GOARCH='arm64'
GOARM64='v8.0'
GOEXPERIMENT=''
GOOS='darwin'
GOROOT='/Users/marten/bin/go1.27ex'
GOVERSION='go1.27.0'
```

### What did you do?

I'm updating [quic-go](https://github.com/quic-go/quic-go) to Go 1.27. Running `go fix` rewrites reverse index loops using the new `slicesbackward` analyzer:

```diff
-for i := len(values) - 1; i >= 0; i-- {
- sum += values[i]
+for _, value := range slices.Backward(values) {
+ sum += value
}
```

Some of these loops are in hot paths. I reduced the performance difference to this benchmark:

```go
package backward_test

import (
"slices"
"testing"
)

func BenchmarkBackwardIterationForLoop(b *testing.B) {
values := make([]int, 64)
for i := range values {
values[i] = i
}
b.ReportAllocs()
var sum int
for b.Loop() {
for i := len(values) - 1; i >= 0; i-- {
sum += values[i]
}
}
}

func BenchmarkBackwardIterationSlicesBackward(b *testing.B) {
values := make([]int, 64)
for i := range values {
values[i] = i
}
b.ReportAllocs()
var sum int
for b.Loop() {
for _, value := range slices.Backward(values) {
sum += value
}
}
}
```

I ran it with:

```console
go test -run '^$' -bench '^BenchmarkBackwardIteration' -benchmem -benchtime=1s -count=10
```

Comparing the results with `benchstat`:

```text
name for loop slices.Backward delta
BackwardIteration-16 89.2ns ± 1% 104.8ns ± 0% +17.49% (p=0.000 n=9+8)
```

Both variants perform zero allocations.

### What did you see happen?

The code produced by `go fix` is approximately 17.5% slower.

#69015 already tracks the general performance difference between iterators and equivalent loops. This issue is specifically about `go fix` automatically applying such a transformation while that performance difference still exists.

There is also no way to suppress this particular fix at the source location. The only available control is:

```console
go fix -slicesbackward=false
```

This disables the analyzer for the entire invocation. In quic-go, the cleanest workaround is to run `go fix` separately for the affected package and disable `slicesbackward` for that entire package. The other workaround would be to rewrite the loop into a less idiomatic form that the analyzer no longer recognizes.

### What did you expect to see?

I don't expect `go fix` to automatically replace a straightforward loop with code that is significantly slower.

Ideally, the compiler would eliminate the iterator overhead, as tracked in #69015. Until both forms have comparable performance, `slicesbackward` should avoid being applied unconditionally.

At minimum, `go fix` should provide a way to suppress an individual suggested fix at a source location, analogous to a `//nolint:` directive. This would allow performance-sensitive code to remain unchanged without disabling the analyzer for an entire package or invocation.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.