Skip to content

x/tools/gopls/internal/analysis/modernize: after refactoring the code using the fmtappendf analyzer, performance may decrease #73666

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
cuishuang opened this issue May 11, 2025 · 3 comments
Labels
gopls Issues related to the Go language server, gopls. ToolProposal Issues describing a requested change to a Go tool or command-line program. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@cuishuang
Copy link
Contributor

Go version

go1.24.2

Output of go env in your module/workspace:

AR='ar'
CC='clang'
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_ENABLED='0'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
CXX='clang++'
GCCGO='gccgo'
GO111MODULE='auto'
GOARCH='arm64'
GOARM64='v8.0'
GOAUTH='netrc'
GOBIN=''
GOCACHE='/Users/xxx/Library/Caches/go-build'
GOCACHEPROG=''
GODEBUG=''
GOENV='/Users/xxx/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFIPS140='off'
GOFLAGS=''
GOGCCFLAGS='-fPIC -arch arm64 -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/9t/839s3jmj73bcgyp5x_xh3gw00000gn/T/go-build2973783802=/tmp/go-build -gno-record-gcc-switches -fno-common'
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMOD='/Users/xxxx/20250428/tools/gopls/go.mod'
GOMODCACHE='/Users/xxx/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/xxx/go'
GOPRIVATE=''
GOPROXY='https://goproxy.cn,direct'
GOROOT='/usr/local/go'
GOSUMDB='off'
GOTELEMETRY='local'
GOTELEMETRYDIR='/Users/xxx/Library/Application Support/go/telemetry'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/local/go/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.24.2'
GOWORK=''
PKG_CONFIG='pkg-config'

What did you do?

benchmark_test.go:

package main

import (
	"fmt"
	"testing"
)

var demoStr = "testStr"

// Benchmark using fmt.Sprintf + conversion to []byte
func BenchmarkSprintfToByte(b *testing.B) {
	b.ReportAllocs()
	for i := 0; i < b.N; i++ {
		_ = []byte(fmt.Sprintf("%s/", demoStr))
	}
}

// Benchmark using fmt.Appendf (Go 1.21+)
func BenchmarkFmtAppendf(b *testing.B) {
	b.ReportAllocs()
	for i := 0; i < b.N; i++ {
		_ = fmt.Appendf(nil, "%s/", demoStr)
	}
}

// Benchmark using a plain append for one alloc
func BenchmarkAppendLiteral(b *testing.B) {
	b.ReportAllocs()
	for i := 0; i < b.N; i++ {
		_ = append([]byte(demoStr), '/')
	}
}

What did you see happen?

go test -bench=. -benchmem -count=5

The result is:

goos: darwin
goarch: arm64
pkg: demo/fmtappendf
cpu: Apple M1
BenchmarkSprintfToByte-8        20321730                58.71 ns/op           24 B/op          2 allocs/op
BenchmarkSprintfToByte-8        19925031                59.58 ns/op           24 B/op          2 allocs/op
BenchmarkSprintfToByte-8        20412472                59.30 ns/op           24 B/op          2 allocs/op
BenchmarkSprintfToByte-8        20003860                61.53 ns/op           24 B/op          2 allocs/op
BenchmarkSprintfToByte-8        19751610                58.90 ns/op           24 B/op          2 allocs/op
BenchmarkFmtAppendf-8           18380124                66.07 ns/op           24 B/op          2 allocs/op
BenchmarkFmtAppendf-8           18335096                69.05 ns/op           24 B/op          2 allocs/op
BenchmarkFmtAppendf-8           16943139                64.83 ns/op           24 B/op          2 allocs/op
BenchmarkFmtAppendf-8           19060880                63.74 ns/op           24 B/op          2 allocs/op
BenchmarkFmtAppendf-8           18375620                63.24 ns/op           24 B/op          2 allocs/op
BenchmarkAppendLiteral-8        407714352                2.922 ns/op           0 B/op          0 allocs/op
BenchmarkAppendLiteral-8        412019996                2.927 ns/op           0 B/op          0 allocs/op
BenchmarkAppendLiteral-8        406827756                2.946 ns/op           0 B/op          0 allocs/op
BenchmarkAppendLiteral-8        394574599                3.014 ns/op           0 B/op          0 allocs/op
BenchmarkAppendLiteral-8        374828496                2.947 ns/op           0 B/op          0 allocs/op
PASS
ok      demo/fmtappendf 21.413s

What did you expect to see?

Using fmt.Appendf seems to cause a slight performance decrease. Should we refactor it to use the BenchmarkAppendLiteral style, which has excellent performance?

@gopherbot gopherbot added Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels May 11, 2025
@gopherbot gopherbot added this to the Unreleased milestone May 11, 2025
@gabyhelp gabyhelp added the ToolProposal Issues describing a requested change to a Go tool or command-line program. label May 11, 2025
@xieyuschen
Copy link
Member

@cuishuang thanks for reporting the issue, you can try to make the demoStr longer and then the result differs. Because fmt.Appendf is used to replace fmt.Sprintf, so BenchmarkAppendLiteral is not helpful here.

+func init() {
+	for range 10 {
+		demoStr += demoStr
+	}
+}

Then the result will be:

goos: linux
goarch: amd64
pkg: mod.com
cpu: AMD Ryzen 7 9700X 8-Core Processor             
BenchmarkSprintfToByte-16        1000000              1073 ns/op            8216 B/op          2 allocs/op
BenchmarkSprintfToByte-16        1000000              1077 ns/op            8217 B/op          2 allocs/op
BenchmarkSprintfToByte-16        1000000              1068 ns/op            8216 B/op          2 allocs/op
BenchmarkSprintfToByte-16        1000000              1032 ns/op            8216 B/op          2 allocs/op
BenchmarkSprintfToByte-16        1219204              1027 ns/op            8215 B/op          2 allocs/op
BenchmarkFmtAppendf-16           1228786               989.5 ns/op          8214 B/op          2 allocs/op
BenchmarkFmtAppendf-16           1212967              1005 ns/op            8215 B/op          2 allocs/op
BenchmarkFmtAppendf-16           1000000              1012 ns/op            8214 B/op          2 allocs/op
BenchmarkFmtAppendf-16           1000000              1009 ns/op            8214 B/op          2 allocs/op
BenchmarkFmtAppendf-16           1220061              1002 ns/op            8215 B/op          2 allocs/op
PASS
ok      mod.com 15.248s

@cuishuang
Copy link
Contributor Author

@cuishuang thanks for reporting the issue, you can try to make the demoStr longer and then the result differs. Because fmt.Appendf is used to replace fmt.Sprintf, so BenchmarkAppendLiteral is not helpful here.

+func init() {

  • for range 10 {
  • demoStr += demoStr
    
  • }
    +}
    Then the result will be:
goos: linux
goarch: amd64
pkg: mod.com
cpu: AMD Ryzen 7 9700X 8-Core Processor             
BenchmarkSprintfToByte-16        1000000              1073 ns/op            8216 B/op          2 allocs/op
BenchmarkSprintfToByte-16        1000000              1077 ns/op            8217 B/op          2 allocs/op
BenchmarkSprintfToByte-16        1000000              1068 ns/op            8216 B/op          2 allocs/op
BenchmarkSprintfToByte-16        1000000              1032 ns/op            8216 B/op          2 allocs/op
BenchmarkSprintfToByte-16        1219204              1027 ns/op            8215 B/op          2 allocs/op
BenchmarkFmtAppendf-16           1228786               989.5 ns/op          8214 B/op          2 allocs/op
BenchmarkFmtAppendf-16           1212967              1005 ns/op            8215 B/op          2 allocs/op
BenchmarkFmtAppendf-16           1000000              1012 ns/op            8214 B/op          2 allocs/op
BenchmarkFmtAppendf-16           1000000              1009 ns/op            8214 B/op          2 allocs/op
BenchmarkFmtAppendf-16           1220061              1002 ns/op            8215 B/op          2 allocs/op
PASS
ok      mod.com 15.248s

Indeed, thanks for your reply and guidance!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gopls Issues related to the Go language server, gopls. ToolProposal Issues describing a requested change to a Go tool or command-line program. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

4 participants