Commit 3031bf4
net/http/internal/http2: enable SetReuseFrames in server and Transport
Call Framer.SetReuseFrames once on the per-connection Framer in both
serverConn (serveConn) and Transport (newClientConn) so the parsed
*DataFrame, *WindowUpdateFrame, *HeadersFrame, and *MetaHeadersFrame
structs returned by ReadFrame are reused across calls instead of
being heap-allocated each time. SetReuseFrames has shipped as an
opt-in Framer method since CL 34812 (golang/net, 2017) but no in-tree
caller previously opted in, so the cache was effectively dead code
from the standard library's perspective; this CL turns it on.
A new GODEBUG setting, http2reuseframes, is registered as a compat
escape hatch in line with the existing http2client / http2server /
http2debug settings:
- default (or http2reuseframes=1): reuse on, this CL's behavior.
- GODEBUG=http2reuseframes=0: reuse disabled, pre-CL behavior.
If a deployment encounters an issue the static audit and -race
coverage didn't catch, operators can flip the setting without
recompiling.
Per-frame allocation savings, microbench numbers (linux/amd64, from
the BenchmarkParse* and BenchmarkReadMetaFrame benchmarks added
earlier in this stack):
bench Default Reused
ParseDataFrame 48 B, 1 alloc 0 B, 0 alloc
ParseWindowUpdateFrame 16 B, 1 alloc 0 B, 0 alloc
ParseHeadersFrame 48 B, 1 alloc 0 B, 0 alloc
ReadMetaFrame 472 B, 9 allocs 376 B, 7 allocs
End-to-end allocation reductions on the package's existing read-path
benchmarks (-count=10, benchstat master vs branch):
bench allocs/op delta p
ClientGzip -59.20% 0.000
DownloadFrameSize/16k -46.73% 0.000
DownloadFrameSize/64k -49.00% 0.000
DownloadFrameSize/128k -49.55% 0.000
DownloadFrameSize/256k -49.87% 0.000
DownloadFrameSize/512k -49.98% 0.000
ClientRequestHeaders/0 -8.00% 0.000
ClientResponseHeaders/0 -8.00% 0.000
ClientRequestHeaders/10 -5.41% 0.000
ClientResponseHeaders/10 -3.48% 0.000
geomean (allocs/op) -19.46%
Write-side benchmarks (WriteScheduler*, WriteQueue) are unchanged, as
expected; those paths do not exercise ReadFrame. ClientGzip latency
also improves by -2.74% (p=0.023). The remaining allocations in
ReadMetaFrame come from HPACK Fields-slice growth and the
SetEmitFunc closure, which this CL does not address.
Reuse safety, by frame type:
DataFrame
serverConn.processData and clientConnReadLoop.processData read
Length, StreamID, StreamEnded, and the Data() slice synchronously
before returning. The bytes from Data() flow through
{server,client}Conn.body / cs.bufPipe -> dataBuffer.Write, which
copies into a pool-allocated chunk; no slice retained past
ReadFrame.
WindowUpdateFrame
serverConn.processWindowUpdate and clientConnReadLoop.processWindowUpdate
read only the scalar StreamID and Increment fields. The struct
type has no slice fields, so there is nothing to alias the read
buffer.
HeadersFrame
Both server and Transport set Framer.ReadMetaHeaders during init,
so a bare *HeadersFrame is never delivered to consumer code; the
Framer always returns *MetaHeadersFrame on a HEADERS frame.
readMetaFrame clears MetaHeadersFrame.HeadersFrame.headerFragBuf
and calls invalidate() on the embedded *HeadersFrame before
returning. The aliased frag buf is therefore not exposed past
readMetaFrame.
MetaHeadersFrame
The Fields slice is freshly allocated per parse: readMetaFrame
does *mh = MetaHeadersFrame{HeadersFrame: hf} (Fields zeroed to
nil), then the HPACK emit callback grows it via append, so each
returned MetaHeadersFrame has its own backing array. HPACK
Name/Value strings are independently allocated by the decoder
(hpack.decodeString returns string(u.b) / buf.String(), both of
which copy), so no string aliases the read buffer.
serverConn.processHeaders / processTrailerHeaders and
clientConnReadLoop.processHeaders / handleResponse / processTrailers
iterate Fields synchronously on the read-loop / serve goroutine
and copy strings into a fresh http.Header before returning.
No code stores *MetaHeadersFrame past the dispatch.
Server-side gating: readFrames -> readFrameCh -> serve calls
processFrameFromReader synchronously and only then invokes
readMore(), which unblocks readFrames for the next ReadFrame. So
even though the server consumes frames on a different goroutine
from the one calling ReadFrame, every frame is fully consumed
before the cache is overwritten.
Transport-side gating: clientConnReadLoop.run consumes each frame
synchronously on the read-loop goroutine before the next
ReadFrame, and what escapes to the RoundTrip / response-body
goroutines is either copied (DataFrame -> bufPipe) or composed of
immutable, independently-allocated Go strings (Header maps).
While here, defensively clear GoAwayFrame.debugData after copying
it to cc.goAwayDebug in processGoAway. GoAwayFrame is not in
frameCache today so the retained cc.goAway pointer is safe, but
this Transport already stores a *GoAwayFrame across ReadFrame
calls; clearing the only field that aliases the read buffer
prevents a future addition of GoAwayFrame to the reuse cache from
silently turning cc.goAway.DebugData() into a use-after-overwrite.
Verification:
net/http/internal/http2: go test -race -count=30 PASS (8m13s)
net/http/internal/http2: go test -race -count=10 -cpu=1,2,4,8
PASS (6m17s)
net/http: go test -race -count=5 PASS (2m15s)
TestFrameReuseRaceCorrect -race -count=200 PASS (5s)
TestFrameReuseRaceAdversarial under
H2_REUSE_RACE_NEGATIVE=1 -race -count=10 RACE (10/10)
TestFrameReuseEndToEndStress -race PASS
GODEBUG=http2reuseframes=0 -race
(TestFrameReuseEndToEndStress, TestFrameReuseRaceCorrect)
PASS
The adversarial test deliberately violates the reuse contract and
asserts the race detector fires; the other runs validate the
production code paths both with reuse on (the default) and with
reuse disabled via GODEBUG.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Change-Id: I9d6d0c2e761b0901314c25f362c99062260b31a71 parent e1978bc commit 3031bf4
6 files changed
Lines changed: 38 additions & 0 deletions
File tree
- doc
- src
- internal/godebugs
- net/http/internal/http2
- runtime/metrics
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
188 | 188 | | |
189 | 189 | | |
190 | 190 | | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
191 | 199 | | |
192 | 200 | | |
193 | 201 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
42 | 42 | | |
43 | 43 | | |
44 | 44 | | |
| 45 | + | |
45 | 46 | | |
46 | 47 | | |
47 | 48 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
9 | 9 | | |
10 | 10 | | |
11 | 11 | | |
| 12 | + | |
12 | 13 | | |
13 | 14 | | |
14 | 15 | | |
| |||
22 | 23 | | |
23 | 24 | | |
24 | 25 | | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
25 | 32 | | |
26 | 33 | | |
27 | 34 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
322 | 322 | | |
323 | 323 | | |
324 | 324 | | |
| 325 | + | |
| 326 | + | |
| 327 | + | |
| 328 | + | |
| 329 | + | |
325 | 330 | | |
326 | 331 | | |
327 | 332 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
662 | 662 | | |
663 | 663 | | |
664 | 664 | | |
| 665 | + | |
| 666 | + | |
| 667 | + | |
| 668 | + | |
| 669 | + | |
665 | 670 | | |
666 | 671 | | |
667 | 672 | | |
| |||
2626 | 2631 | | |
2627 | 2632 | | |
2628 | 2633 | | |
| 2634 | + | |
| 2635 | + | |
| 2636 | + | |
| 2637 | + | |
| 2638 | + | |
| 2639 | + | |
| 2640 | + | |
2629 | 2641 | | |
2630 | 2642 | | |
2631 | 2643 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
311 | 311 | | |
312 | 312 | | |
313 | 313 | | |
| 314 | + | |
| 315 | + | |
| 316 | + | |
| 317 | + | |
| 318 | + | |
314 | 319 | | |
315 | 320 | | |
316 | 321 | | |
| |||
0 commit comments