Skip to content

x/net/http2: optimize typeFrameParser using an array #73613

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
evanj opened this issue May 6, 2025 · 3 comments
Closed

x/net/http2: optimize typeFrameParser using an array #73613

evanj opened this issue May 6, 2025 · 3 comments
Labels
FixPending Issues that have a fix which has not yet been reviewed or submitted. Implementation Issues describing a semantics-preserving change to the Go implementation. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@evanj
Copy link
Contributor

evanj commented May 6, 2025

Go version

go version go1.24.2 darwin/arm64

Output of go env in your module/workspace:

AR='ar'
CC='cc'
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_ENABLED='1'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
CXX='c++'
GCCGO='gccgo'
GO111MODULE=''
GOARCH='arm64'
GOARM64='v8.0'
GOAUTH='netrc'
GOBIN=''
GOCACHE='/Users/evan.jones/Library/Caches/go-build'
GOCACHEPROG=''
GODEBUG=''
GOENV='/Users/evan.jones/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/g1/97d8s0r57hj4nv4_qd3fqcrm0000gp/T/go-build274632297=/tmp/go-build -gno-record-gcc-switches -fno-common'
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMOD='/Users/evan.jones/go-net/go.mod'
GOMODCACHE='/Users/evan.jones/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/evan.jones/go'
GOPRIVATE=''
GOPROXY=''
GOROOT='/opt/homebrew/Cellar/go/1.24.2/libexec'
GOSUMDB='sum.golang.org'
GOTELEMETRY='on'
GOTELEMETRYDIR='/Users/evan.jones/Library/Application Support/go/telemetry'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/opt/homebrew/Cellar/go/1.24.2/libexec/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.24.2'
GOWORK=''
PKG_CONFIG='pkg-config'

What did you do?

I have been looking at gRPC overhead using the Go profiler for production services at Datadog. I noticed that http2.typeFrameParser shows up as 0.1% of total CPU for a high QPS service. I have submitted a gerrit code review to optimize this function by replacing the map lookup with an array.

Suggested micro-optimization: https://go-review.googlesource.com/c/net/+/670415

What did you see happen?

The attached screenshot shows the pprof output for a production service, focused on this function. The "0.16% of profile" suggests that this function is 0.16% of all CPU used for this service. The output shows that all the time is the map lookup since the function does not do anything else.

Suggested micro-optimization: https://go-review.googlesource.com/c/net/+/670415

Image

What did you expect to see?

This function will use less CPU with an array lookup.

@gabyhelp
Copy link

gabyhelp commented May 6, 2025

Related Issues

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

@gabyhelp gabyhelp added the Implementation Issues describing a semantics-preserving change to the Go implementation. label May 6, 2025
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/670415 mentions this issue: http2: use an array instead of a map in typeFrameParser

@cherrymui cherrymui added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. FixPending Issues that have a fix which has not yet been reviewed or submitted. labels May 8, 2025
@cherrymui cherrymui added this to the Unreleased milestone May 8, 2025
@cherrymui
Copy link
Member

cc @neild @tombergan

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FixPending Issues that have a fix which has not yet been reviewed or submitted. Implementation Issues describing a semantics-preserving change to the Go implementation. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

4 participants