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

x/net/http2: improve frame decoding performance by reusing frames #68352

Open
YHM404 opened this issue Jul 9, 2024 · 5 comments
Open

x/net/http2: improve frame decoding performance by reusing frames #68352

YHM404 opened this issue Jul 9, 2024 · 5 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Performance
Milestone

Comments

@YHM404
Copy link

YHM404 commented Jul 9, 2024

Proposal Details

I noticed a issue about SetReuseFrames #18502

I would like to propose the addition of a configuration option to enable or disable SetReuseFrames in the HTTP2 server and HTTP2 transport.

SetReuseFrames has been shown to improve the performance of go-grpc, so I believe it can similarly enhance the performance of HTTP server/client.

By adding this configuration option, users can choose to enable frame reuse based on their performance needs and use cases.

Thank you for considering this feature request.

@YHM404 YHM404 added the Proposal label Jul 9, 2024
@seankhliao seankhliao changed the title x/net/http2: Adding a Configuration Option to SetReuseFrames proposal: x/net/http2: Adding a Configuration Option to SetReuseFrames Jul 9, 2024
@gopherbot gopherbot added this to the Proposal milestone Jul 9, 2024
@gabyhelp
Copy link

gabyhelp commented Jul 9, 2024

Related Issues and Documentation

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

@ianlancetaylor
Copy link
Contributor

CC @neild @bradfitz

@neild
Copy link
Contributor

neild commented Jul 9, 2024

There's no need for a user-configurable option here. If there are performance improvements that can be made to frame decoding in the HTTP/2 implementation, we can just make them.

@seankhliao seankhliao changed the title proposal: x/net/http2: Adding a Configuration Option to SetReuseFrames proposal: x/net/http2: add config option for setting Server/Transport SetReuseFrames Jul 9, 2024
@YHM404
Copy link
Author

YHM404 commented Jul 10, 2024

There's no need for a user-configurable option here. If there are performance improvements that can be made to frame decoding in the HTTP/2 implementation, we can just make them.

I will conduct a performance comparison and, if everything looks good, I will submit a pull request.

@ianlancetaylor ianlancetaylor added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. and removed Proposal labels Jul 10, 2024
@ianlancetaylor ianlancetaylor modified the milestones: Proposal, Backlog Jul 10, 2024
@seankhliao seankhliao changed the title proposal: x/net/http2: add config option for setting Server/Transport SetReuseFrames x/net/http2: improve frame decoding performance by reusing frames Jul 10, 2024
@YHM404
Copy link
Author

YHM404 commented Jul 10, 2024

I have conducted a simple benchmark, and the results show that SetReuseFrames significantly improves the performance of parseDataFrame.

Running tool: /usr/local/bin/go test -benchmem -run=^$ -bench ^BenchmarkParseDataFramesWithoutReuse$ golang.org/x/net/http2

goos: darwin
goarch: amd64
pkg: golang.org/x/net/http2
cpu: Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
BenchmarkParseDataFramesWithoutReuse-12    	31313877	        39.03 ns/op	      48 B/op	       1 allocs/op
Running tool: /usr/local/bin/go test -benchmem -run=^$ -bench ^BenchmarkParseDataFrames$ golang.org/x/net/http2

goos: darwin
goarch: amd64
pkg: golang.org/x/net/http2
cpu: Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
BenchmarkParseDataFrames-12    	143745076	         8.167 ns/op	       0 B/op	       0 allocs/op
PASS
ok  	golang.org/x/net/http2	2.413s

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Performance
Projects
None yet
Development

No branches or pull requests

6 participants