Skip to content

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

Description

@marten-seemann

Go version

$ go version
go version go1.27.0 darwin/arm64

Output of go env in your module/workspace

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 to Go 1.27. Running go fix rewrites reverse index loops using the new slicesbackward analyzer:

-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:

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:

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

Comparing the results with benchstat:

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:

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:<check> directive. This would allow performance-sensitive code to remain unchanged without disabling the analyzer for an entire package or invocation.

Metadata

Metadata

Assignees

Labels

ToolsThis label describes issues relating to any tools in the x/tools repository.

Type

No type

Projects

No projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions