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

cmd/compile: Autotmp* variables causing unnecessary escape #58111

Closed
ar8327 opened this issue Jan 27, 2023 · 4 comments
Closed

cmd/compile: Autotmp* variables causing unnecessary escape #58111

ar8327 opened this issue Jan 27, 2023 · 4 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge Performance

Comments

@ar8327
Copy link

ar8327 commented Jan 27, 2023

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

$ go version
go version go1.20rc3 darwin/arm64

Does this issue reproduce with the latest release?

Yes. From 1.18 to latest
Don't see the problem on 1.17.13 and older

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

go env Output
$ go env
GO111MODULE="on"
GOARCH="arm64"
GOBIN=""
GOCACHE="/Users/ar8327/Library/Caches/go-build"
GOENV="/Users/ar8327/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="arm64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/ar8327/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/ar8327/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/Users/ar8327/go/go1.20rc3"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/Users/ar8327/go/go1.20rc3/pkg/tool/darwin_arm64"
GOVCS=""
GOVERSION="go1.20rc3"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/ar8327/GolandProjects/playground/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 -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/j9/bzrxk4410vv1zxk0p7c4z34w0000gn/T/go-build446591197=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

Run following benchmark

func asyncLog(v string) {
	go syncLog(v)
}

func syncLog(v string) {
	// Do nothing
}

func BenchmarkAsyncLog(b *testing.B) {
	for i := 0; i < b.N; i++ {
		asyncLog("xx")
	}
}

What did you expect to see?

I expect no variable escape and no memory allocation at benchmark

Before go 1.18, compiled asm was like:

        0x0000 00000 (go.go:7)  TEXT    "".asyncLog(SB), ABIInternal, $40-16
        0x000f 00015 (go.go:7)  SUBQ    $40, SP
        0x0013 00019 (go.go:7)  MOVQ    BP, 32(SP)
        0x0018 00024 (go.go:7)  LEAQ    32(SP), BP
        0x001d 00029 (go.go:8)  MOVL    $16, (SP)
        0x0024 00036 (go.go:8)  LEAQ    "".syncLog·f(SB), AX
        0x002b 00043 (go.go:8)  MOVQ    AX, 8(SP)
        0x0030 00048 (go.go:8)  MOVQ    "".v+48(SP), AX
        0x0035 00053 (go.go:8)  MOVQ    AX, 16(SP)
        0x003a 00058 (go.go:8)  MOVQ    "".v+56(SP), AX
        0x003f 00063 (go.go:8)  MOVQ    AX, 24(SP)
        0x0044 00068 (go.go:8)  CALL    runtime.newproc(SB)
        0x0049 00073 (go.go:9)  MOVQ    32(SP), BP
        0x004e 00078 (go.go:9)  ADDQ    $40, SP
        0x0052 00082 (go.go:9)  RET

Variables were allocated on caller's stack frame

What did you see instead?

        0x0006 00006 (go.go:3)  SUBQ    $32, SP
        0x000a 00010 (go.go:3)  MOVQ    BP, 24(SP)
        0x000f 00015 (go.go:3)  LEAQ    24(SP), BP
        0x0014 00020 (go.go:3)  MOVQ    AX, .v+40(FP)
        0x0019 00025 (go.go:4)  MOVQ    AX, ..autotmp_3+16(SP)
        0x001e 00030 (go.go:4)  MOVQ    BX, .v+48(SP)
        0x0023 00035 (go.go:4)  LEAQ    type.noalg.struct { F uintptr; ..autotmp_1 interface {} }(SB), AX
        0x002a 00042 (go.go:4)  CALL    runtime.newobject(SB)
        0x002f 00047 (go.go:4)  LEAQ    .asyncLog.func1(SB), CX
        0x0036 00054 (go.go:4)  MOVQ    CX, (AX)
        0x0039 00057 (go.go:4)  MOVQ    ..autotmp_3+16(SP), CX
        0x003e 00062 (go.go:4)  MOVQ    CX, 8(AX)
        0x0042 00066 (go.go:4)  CMPL    runtime.writeBarrier(SB), $0
        0x0049 00073 (go.go:4)  JNE     86
        0x004b 00075 (go.go:4)  MOVQ    .v+48(SP), CX
        0x0050 00080 (go.go:4)  MOVQ    CX, 16(AX)
        0x0054 00084 (go.go:4)  JMP     101
        0x0056 00086 (go.go:4)  LEAQ    16(AX), DI
        0x005a 00090 (go.go:4)  MOVQ    .v+48(SP), CX
        0x005f 00095 (go.go:4)  NOP
        0x0060 00096 (go.go:4)  CALL    runtime.gcWriteBarrierCX(SB)
        0x0065 00101 (go.go:4)  CALL    runtime.newproc(SB)
        0x006a 00106 (go.go:5)  MOVQ    24(SP), BP
        0x006f 00111 (go.go:5)  ADDQ    $32, SP
        0x0073 00115 (go.go:5)  RET
        

.asyncLog.func1 STEXT size=76 args=0x0 locals=0x18 funcid=0x15 align=0x0
        0x0000 00000 (go.go:4)  TEXT    .asyncLog.func1(SB), WRAPPER|NEEDCTXT|ABIInternal, $24-0
        0x0006 00006 (go.go:4)  SUBQ    $24, SP
        0x000a 00010 (go.go:4)  MOVQ    BP, 16(SP)
        0x000f 00015 (go.go:4)  LEAQ    16(SP), BP
        0x001d 00029 (go.go:4)  NOP
        0x001d 00029 (go.go:4)  MOVQ    8(DX), AX
        0x0021 00033 (go.go:4)  MOVQ    16(DX), BX
        0x0025 00037 (go.go:4)  CALL    .syncLog(SB)
        0x002a 00042 (go.go:4)  MOVQ    16(SP), BP
        0x002f 00047 (go.go:4)  ADDQ    $24, SP
        0x0033 00051 (go.go:4)  RET

@ar8327 ar8327 changed the title go/compiler: Autotmp* variables causing unnecessary escape cmd/compile: Autotmp* variables causing unnecessary escape Jan 27, 2023
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jan 27, 2023
@randall77
Copy link
Contributor

This was part of the switch to regabi, we had to build a closure for anything go'd. I believe the CL is https://go-review.googlesource.com/c/go/+/298669
So this is definitely expected. I don't think there's a fundamental reason why this allocation is necessary, but fixing it will certainly be complicated.
In any case, there are more allocations going on when going a function (a runtime.g and a stack, at least). This allocation is minor in comparison.
@thanm @cherrymui @dr2chase

@thanm
Copy link
Contributor

thanm commented Jan 27, 2023

Yes, this is expected behavior. Doing things this way made things much simpler for defer and go() handling with the register ABI. As Keith points out, the closure allocation is small potatoes compared with the other things that have to happen to create a new goroutine.

@ar8327
Copy link
Author

ar8327 commented Jan 28, 2023

Yes. I've just run a few tests. Performance difference is marginal, about 2% between 1.17.13 and 1.20 rc3.

@randall77
Copy link
Contributor

Ok, I'm going to close this issue, as I don't see anything actionable.

@golang golang locked and limited conversation to collaborators Jan 28, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge Performance
Projects
None yet
Development

No branches or pull requests

4 participants