proposal: x/net/http2: decrease http2 uplink buffer size

[Fangliding]

Proposal Details

When (*clientStream).writeRequestBody sends a request body, it sizes its read/write scratch buffer via frameScratchBufferLen

The problem is requests whose body length is streaming (ContentLength == -1, like gRPC or other duplex http requests). For those, the buffer becomes min(MaxFrameSize, 512 KiB).

Peers very commonly advertise a large SETTINGS_MAX_FRAME_SIZE, like golang, cause the transport keeps 512 KiB buffer for each stream and may consume tens of MiB RAM.

As a supplement, although the buffer is from the pool, considering a long lived connection, IO is almost always blocked at read (wait for next data), so the stream will hold this big buffer until closed

Although we can change Transport.MaxReadFrameSize, but this will affect the entire http2 connection and needs to set at server

For comparison, io.Copy uses a 32 KiB buffer, it's a more appropriate limit,or consider gradually increasing the buffer based on the each read size

[seankhliao]

It's unclear if this is proposing to change a default, or expose some form of control?

[Fangliding]

It's unclear if this is proposing to change a default, or expose some form of control?

This is a request to decrease the hard coded maximum buffer size (https://github.com/golang/net/blob/2707ee21efea70599446dd35979b1d1aedf44ada/http2/transport.go#L1452 change to 32<< 10)

I think providing options is unnecessary

[darkk]

I agree that changing the default value is the core of this proposal.
I believe, MaxWriteFrameSize knob might be a separate and/or alternative proposal if(!) 32 << 10 turns out to be a bad default value for a max constant within the frameScratchBufferLen.

32 << 10 seems to be a reasonable default as it follows io.Copy() using 32 KiB for quite similar purpose.

512 << 10 seems to be there for quite a long time.
It was already there at golang/net@04defd4
It was pushing RAM limits some time ago: #38049 (comment).
It appeared at golang/net@09d2a41 as maxAllocFrameSize, "(kinda arbitrarily)", if I do archaeology correctly.

However, #47840 (comment) suggests, that io.Copy() buffer might be to small if we're past Gigabit links and start talking about 10Gi and above.

[Fangliding]

@seankhliao Hi, is there any progress or not planned？

[seankhliao]

cc @neild @nicholashusin

[neild]

To summarize (as I understand it):

The HTTP/2 transport allocates a temporary buffer to use when writing a request body. Since the request body is provided as an io.Reader, the transport needs to read data into a copy buffer before writing it out to the network with appropriate HTTP/2 framing added.

The temporary buffer is currently max(512KiB, peer's maximum frame size) large.

The request is to reduce 512KiB to something smaller.

So far as I know, 512KiB is a completely arbitrary value that was chosen at some point in the past and persists to this day. So on one hand, there isn't any reason to consider it set in stone. On the other hand, every time we change something we tend to break some users who depended on the current value for whatever reason.

Almost all HTTP/2 connections are encrypted, which means all our writes are going through crypto/tls. I believe crypto/tls has a maximum frame size of 16KiB (TLS maximum, although I think there are extensions that let you negotiate higher?), so writing larger chunks than that doesn't seem particularly helpful.

What is relevant is the HTTP/2 frame size, though. The copy buffer size sets a cap on the size of the HTTP/2 frames we can write. If the server benefits from larger frames, then using a smaller buffer could be a problem.

A possible option is to take advantage of a request body that implements io.WriterTo. Then we can skip the copy buffer entirely. This seems like a straight-up benefit without any downsides I can see. Unfortunately, this doesn't help anyone using io.Pipe to stream the request body (which is the simplest way to stream the body that I know of), since io.Pipe doesn't implement WriterTo. Out of curiosity, @Fangliding, do your request bodies implement WriterTo?

[Fangliding]

Almost all HTTP/2 connections are encrypted, which means all our writes are going through crypto/tls. I believe crypto/tls has a maximum frame size of 16KiB (TLS maximum, although I think there are extensions that let you negotiate higher?), so writing larger chunks than that doesn't seem particularly helpful.

The tls record size limit is only a part, readers that can use buffers up to 512kb are not common

What is relevant is the HTTP/2 frame size, though. The copy buffer size sets a cap on the size of the HTTP/2 frames we can write. If the server benefits from larger frames, then using a smaller buffer could be a problem.

So I also propose that maybe we can gradually increase the size of the buffer based on the n returned by read() (double buffer size if read uses all buffer)

A possible option is to take advantage of a request body that implements io.WriterTo. Then we can skip the copy buffer entirely. This seems like a straight-up benefit without any downsides I can see. Unfortunately, this doesn't help anyone using io.Pipe to stream the request body (which is the simplest way to stream the body that I know of), since io.Pipe doesn't implement WriterTo. Out of curiosity, @Fangliding, do your request bodies implement WriterTo?

It's not difficult to implement a WriterTo, but others will also encounter this problem in the future

[darkk]

crypto/tls has a maximum frame size of 16KiB (TLS maximum, although I think there are extensions that let you negotiate higher?)

To the best of my understanding, both RFC 6066 (max_fragment_length) and RFC 8449 (record_size_limit) allow to negotiate a smaller frame size, but don't allow to go beyond 16 KiB. I was unable to find an extension allowing to negotiate higher.

However, it's just a side note. You're absolutely correct about HTTP/2 frame size.

[wwqgtxx]

I don't entirely agree with the view that "providing options is unnecessary."

Default values ​​have existed for many years, and whether to modify them, and how small to modify them appropriately, will always be a topic of discussion and debate.

Even though HTTP/2 typically operates under crypto/tls, there are still use cases for H2C.

Some data centers might be willing to use larger values ​​for increased throughput.

Meanwhile, some memory-critical scenarios (such as iOS) might prefer smaller values ​​to meet system limitations.

It's difficult to satisfy everyone with a single default value, so why not provide an option that users can choose for themselves?

[darkk]

whether to modify them, and how small to modify them appropriately, will always be a topic of discussion and debate.

Right. Moreover, if we talk about user-facing h2/TLS/TCP, there might be a desire to make it dynamic to align the end of h2 frame, the end of TLS fragment and the end of TCP congestion window (end of specific TCP segment) to avoid postponing decryption and decapsulation for one RTT. I mean something like ssl_dyn_rec_* for nginx.

On the other hand, default value has a nice property: the set of users using the same value (or employing the same behavior) is closer to "all users". The default value that suits broader audience leaves less opportunities for traffic fingerprinting. That's might be a desirable property if we think about it from the angle of RFC 7258 (Pervasive Monitoring Is an Attack).

Making this specific value configurable is to make it part of API. The value we discuss in this proposal is a part of implementation, like io.Copy internal buffer size. The value we discuss has no explicit meaning at protocol level. That's why I mentioned MaxWriteFrameSize as an alternative, but substantially different proposal.