golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
124.21k stars 17.7k forks source link

cmd/compile: refactor inline interleaving + very high budget for single-called closures #70504

Closed dr2chase closed 2 hours ago

dr2chase commented 8 hours ago

Go version

go version devel go1.24-326a0ef883 Thu Nov 21 13:23:03 2024 -0500 darwin/arm64

Output of go env in your module/workspace:

AR='ar'
CC='clang'
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_ENABLED='1'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
CXX='clang++'
GCCGO='gccgo'
GO111MODULE=''
GOARCH='arm64'
GOARM64='v8.0'
GOAUTH='netrc'
GOBIN=''
GOCACHE='/Users/drchase/Library/Caches/go-build'
GODEBUG=''
GOENV='/Users/drchase/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFIPS140='off'
GOFLAGS=''
GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/mz/1gp2q1rx7cj2_g_j8sml9kc40095tn/T/go-build2764311795=/tmp/go-build -gno-record-gcc-switches -fno-common'
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMOD='/Users/drchase/work/go/src/go.mod'
GOMODCACHE='/Users/drchase/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/drchase/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/Users/drchase/work/go'
GOSUMDB='sum.golang.org'
GOTELEMETRY='on'
GOTELEMETRYDIR='/Users/drchase/Library/Application Support/go/telemetry'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/Users/drchase/work/go/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='devel go1.24-326a0ef883 Thu Nov 21 13:23:03 2024 -0500'
GOWORK=''
PKG_CONFIG='pkg-config'

What did you do?

Tried to get my CLs mailed, tested, and reviewed before the freeze. They are:

first: https://go.dev/cl/630595 cmd/compile: refactor inline interleaving

second: https://go.dev/cl/630696 cmd/compile: use very high budget for once-called closures

They both pass trybots. I've also benchmarked binary text size and bent performance, and they do no harm.

The purpose of the pair of CLs is to address expected rangefunc performance complaints; without aggressive inlining, memory allocation for nested rangefunc loops is somewhat likely. This change recognizes closures that are only called once and gives them a very large inlining budget.

What did you see happen?

Looks like that will be hard. The most apt reviewers are oversubscribed and may not get to them in time.

What did you expect to see?

A freeze exception, if I could.

gabyhelp commented 8 hours ago

Related Code Changes

(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)

dmitshur commented 2 hours ago

Thanks. It looks like both of those CLs made it in before the freeze started, so a freeze exception isn't needed.