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.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

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

Type

No type

Projects

No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions