Skip to content

x/net/http2: increased memory use with ForceAttemptHTTP2 #64164

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
rawat-divyanshu opened this issue Nov 15, 2023 · 4 comments
Closed

x/net/http2: increased memory use with ForceAttemptHTTP2 #64164

rawat-divyanshu opened this issue Nov 15, 2023 · 4 comments
Labels
FrozenDueToAge WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Milestone

Comments

@rawat-divyanshu
Copy link

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

$ go version
go version go1.21.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

What did you do?

In my golang application we were using the http Client with custom TLS config, and as per the golang's x/net/http package if we provide a custom TLS Config then the support for http/2 will be disabled. So for enabling the http/2 support with custom TLS config I enabled the flag ForceAttemptHTTP2 flag, but with that I'm seeing a 200MB of memory spike in my application.

I'm running this application on a 800 Hosts setup. But as of now none of the Hosts has a http/2 support enabled still I'm seeing this memory spike.

On profiling my application I'm seeing the top contributor for my memory is bytes.growSlice which increased from 8% to whooping 30%. Profile details are as below:

  1. Profile with above changes:
Screenshot 2023-11-15 at 2 52 37 PM Screenshot 2023-11-15 at 2 53 18 PM Screenshot 2023-11-15 at 3 00 53 PM
  1. Profile without changes:
Screenshot 2023-11-15 at 2 56 12 PM Screenshot 2023-11-15 at 2 56 27 PM

What did you expect to see?

Since I enabled the http/2 support, after the protocol negotiation since I didn't had any hosts supporting http/2 the request should fallback to http/1.1 and application should behave normally.

What did you see instead?

Though requests were falling back to http/1.1 but there was 250MB more memory usage by the application.

@gopherbot gopherbot added this to the Unreleased milestone Nov 15, 2023
@seankhliao
Copy link
Member

please provide a reproducer demonstrating the issue.
a tls connection includes a list of supported application protocols, if h2 wasn't advertised, it should never be used.

@seankhliao seankhliao changed the title x/net/http: Custom TLS Client config and ForceAttemptHTTP2 enabled is leading to memory spike x/net/http2: increased memory use with ForceAttemptHTTP2 Nov 15, 2023
@seankhliao seankhliao added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Nov 15, 2023
@mauri870
Copy link
Member

I think this comment explains why:

	// TODO: let getReadBuf be configurable, and use a less memory-pinning
	// allocator in server.go to minimize memory pinned for many idle conns.
	// Will probably also need to make frame invalidation have a hook too.
	getReadBuf func(size uint32) []byte

https://github.com/golang/net/blob/547e7edf3873d6f3a9c093d3785f9e2289e00746/http2/frame.go#L282-L285

@mauri870
Copy link
Member

Maybe we can come up with something similar to CL 539915 to optimize the memory usage and allocations.

@gopherbot
Copy link
Contributor

Timed out in state WaitingForInfo. Closing.

(I am just a bot, though. Please speak up if this is a mistake or you have the requested information.)

@gopherbot gopherbot closed this as not planned Won't fix, can't repro, duplicate, stale Dec 15, 2023
@golang golang locked and limited conversation to collaborators Dec 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Development

No branches or pull requests

4 participants