Skip to content
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

runtime: Redundant memory loads in the for loop in function retake() #59477

Closed
nngffhj opened this issue Apr 7, 2023 · 2 comments
Closed

runtime: Redundant memory loads in the for loop in function retake() #59477

nngffhj opened this issue Apr 7, 2023 · 2 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. Performance
Milestone

Comments

@nngffhj
Copy link

nngffhj commented Apr 7, 2023

What version of Go are you using (go version)?

$ go version
go version go1.20.3 linux/amd64

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/czhong4/.cache/go-build"
GOENV="/home/czhong4/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/czhong4/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/czhong4/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/czhong4/gocode/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/czhong4/gocode/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.20.3"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/czhong4/gocode/go/src/go.mod"
GOWORK=""
CGO_CFLAGS="-O2 -g"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-O2 -g"
CGO_FFLAGS="-O2 -g"
CGO_LDFLAGS="-O2 -g"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build822587075=/tmp/go-build -gno-record-gcc-switches"

What did you do?

I ran Go programs with a tool that detects redundant loads.

What did you expect to see?

Few redundant loads.

What did you see instead?

There are about 20000-30000 redundant loads in a hello world program at instruction

cmp    %rax,0xed8ed(%rip)        # 52c3f8 <runtime.allp+0x8>

that does the loop bounds checking at source line

...
for i := 0; i < len(allp); i++ {
...

in function runtime.retake(). It loads the length of allp from memory in each loop. If we store len(allp) in a variable and only update that variable after acquiring allpLock, those redundant loads can be eliminated.

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Apr 7, 2023
@gopherbot
Copy link

Change https://go.dev/cl/482936 mentions this issue: runtime: eliminate redundant loads in function runtime.retake()

@mknyszek mknyszek added this to the Backlog milestone Apr 7, 2023
@mknyszek
Copy link
Contributor

mknyszek commented Apr 7, 2023

Thanks for your change. I don't think this function runs frequently enough for this optimization to show any meaningful difference in real applications, and in my opinion it makes the code less clear.

(In the future, please feel free to send small changes like this without filing an issue for it.)

@mknyszek mknyszek closed this as not planned Won't fix, can't repro, duplicate, stale Apr 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. Performance
Projects
None yet
Development

No branches or pull requests

3 participants