Skip to content

Commit 29665f4

Browse files
authored
transport: Pool read buffers used by the HTTP/2 framer (#9032)
## Problem The HTTP/2 framer in gRPC uses a `bufio.Reader` with a 32KB buffer by default. When there are a large number of transports, these buffers consume significant memory, even when the transport is idle. ## Solution #8964 added a `ReadyReader` interface that allows non-memory-pinning reads. This PR replaces the standard `bufio.Reader` with a custom `io.Reader` implementation that uses pooled buffers and releases the buffer once all data is consumed. To defer the re-allocation of the read buffer, the reader calls `ReadOnReady` on the underlying `io.Reader`. For this to work, the underlying `io.Reader` must implement either the `ReadyReader` interface or `syscall.RawConn`. If neither condition is met, the framer gracefully falls back to using the regular `bufio.Reader`. Additional Changes: * The ALTS connection has been refactored to implement the `ReadyReader` interface. * The write buffer pools used by the framer are updated to use the `mem.BufferPool` interface, allowing the pools to be shared across both read and write operations. * Use `syscall.Read` instead of `unix.Read` to avoid triggering the race detector, see comment for details. * Add environment variable protection for the changes to allow fast rollback. ## Benchmarks In a [real-world benchmark](https://github.com/arjan-bal/custom-go-client-benchmark/tree/retry-dp), where a GCS directpath client downloads a file in a loop, the average "in use" memory falls from 28.3MB to 21.3MB (-24%). Local Benchmarks show no significant difference ``` ❯ go run benchmark/benchresult/main.go streaming-before streaming-after streaming-networkMode_Local-bufConn_false-keepalive_false-benchTime_2m0s-trace_false-latency_0s-kbps_0-MTU_0-maxConcurrentCa lls_120-reqSize_1024B-respSize_1024B-compressor_off-channelz_false-preloader_false-clientReadBufferSize_-1-clientWriteBuffer Size_-1-serverReadBufferSize_-1-serverWriteBufferSize_-1-sleepBetweenRPCs_0s-connections_1-recvBufferPool_simple-sharedWrite Buffer_true Title Before After Percentage TotalOps 29981273 29966908 -0.05% SendOps 0 0 NaN% RecvOps 0 0 NaN% Bytes/op 4971.06 4971.41 0.00% Allocs/op 19.79 19.79 0.00% ReqT/op 2046721570.13 2045740919.47 -0.05% RespT/op 2046721570.13 2045740919.47 -0.05% 50th-Lat 461.523µs 460.906µs -0.13% 90th-Lat 654.435µs 655.327µs 0.14% 99th-Lat 1.225856ms 1.240984ms 1.23% Avg-Lat 478.845µs 479.553µs 0.15% GoVersion go1.25.0 go1.25.0 GrpcVersion 1.81.0-dev 1.81.0-dev ``` RELEASE NOTES: * transport: Pool HTTP/2 framer read buffers to reduce idle memory consumption. Currently limited to Linux for ALTS and non-encrypted transports (TCP, Unix). To disable, set `GRPC_GO_EXPERIMENTAL_HTTP_FRAMER_READ_BUFFER_POOLING=false` and report any issues.
1 parent f6304e9 commit 29665f4

7 files changed

Lines changed: 175 additions & 51 deletions

File tree

credentials/alts/internal/conn/record.go

Lines changed: 42 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,9 @@ import (
2727
"net"
2828

2929
core "google.golang.org/grpc/credentials/alts/internal"
30-
"google.golang.org/grpc/internal/mem"
30+
imem "google.golang.org/grpc/internal/mem"
3131
"google.golang.org/grpc/internal/transport/readyreader"
32+
"google.golang.org/grpc/mem"
3233
)
3334

3435
// ALTSRecordCrypto is the interface for gRPC ALTS record protocol.
@@ -75,16 +76,19 @@ const (
7576

7677
var (
7778
protocols = make(map[string]ALTSRecordFunc)
78-
writeBufPool *mem.BinaryTieredBufferPool
79+
writeBufPool *imem.BinaryTieredBufferPool
7980
// readBufPool pools buffers of at least `altsReadBufferInitialSize` size.
8081
// Since the read buffer size is slightly larger than 32KB, using a regular
8182
// BinaryTieredBufferPool results in allocating buffers of almost double the
8283
// required length.
83-
readBufPool = mem.NewDirtySimplePool()
84+
readBufPool = imem.NewDirtySimplePool()
85+
86+
// Compile-time check to ensure conn implements ReadyReader.
87+
_ readyreader.Reader = &conn{}
8488
)
8589

8690
func init() {
87-
pool, err := mem.NewDirtyBinaryTieredBufferPool(
91+
pool, err := imem.NewDirtyBinaryTieredBufferPool(
8892
8,
8993
12, // Go page size, 4KB
9094
14, // 16KB (max HTTP/2 frame size used by gRPC)
@@ -126,7 +130,8 @@ type conn struct {
126130
// nextFrame stores the next frame (in protected buffer) info.
127131
nextFrame []byte
128132
// overhead is the calculated overhead of each frame.
129-
overhead int
133+
overhead int
134+
constPool constBufferPool // stored as a field to avoid heap allocations.
130135
}
131136

132137
// NewConn creates a new secure channel instance given the other party role and
@@ -163,21 +168,38 @@ func NewConn(c net.Conn, side core.Side, recordProtocol string, key []byte, prot
163168
return altsConn, nil
164169
}
165170

171+
type constBufferPool struct {
172+
buffer []byte
173+
}
174+
175+
func (p *constBufferPool) Get(int) *[]byte {
176+
return &p.buffer
177+
}
178+
179+
func (p *constBufferPool) Put(*[]byte) {}
180+
166181
// Read reads and decrypts a frame from the underlying connection, and copies the
167182
// decrypted payload into b. If the size of the payload is greater than len(b),
168183
// Read retains the remaining bytes in an internal buffer, and subsequent calls
169184
// to Read will read from this buffer until it is exhausted.
170185
func (p *conn) Read(b []byte) (n int, err error) {
186+
p.constPool.buffer = b
187+
_, n, err = p.ReadOnReady(len(b), &p.constPool)
188+
return n, err
189+
}
190+
191+
func (p *conn) ReadOnReady(bufSize int, pool mem.BufferPool) (*[]byte, int, error) {
171192
if len(p.buf) == 0 {
172193
var framedMsg []byte
173194
var protected []byte
174195
if p.protectedHandle != nil {
175196
protected = *p.protectedHandle
176197
protected = protected[:cap(protected)]
177198
}
199+
var err error
178200
framedMsg, p.nextFrame, err = ParseFramedMsg(p.nextFrame, altsRecordLengthLimit)
179201
if err != nil {
180-
return 0, err
202+
return nil, 0, err
181203
}
182204
// Check whether the next frame to be decrypted has been
183205
// completely received yet.
@@ -217,40 +239,42 @@ func (p *conn) Read(b []byte) (n int, err error) {
217239
// Connection was idle, need to re-allocate the read buffer.
218240
newBuf, nRead, err := p.reader.ReadOnReady(altsReadBufferInitialSize, readBufPool)
219241
if err != nil {
220-
return 0, err
242+
return nil, 0, err
221243
}
222244
p.protectedHandle = newBuf
223245
protected = (*newBuf)[:nRead]
224246
} else {
225247
nRead, err := p.Conn.Read(protected[len(protected):cap(protected)])
226248
if err != nil {
227-
return 0, err
249+
return nil, 0, err
228250
}
229251
protected = protected[:len(protected)+nRead]
230252
}
231253
framedMsg, p.nextFrame, err = ParseFramedMsg(protected, altsRecordLengthLimit)
232254
if err != nil {
233-
return 0, err
255+
return nil, 0, err
234256
}
235257
}
236258
// Now we have a complete frame, decrypted it.
237259
msg := framedMsg[MsgLenFieldSize:]
238260
msgType := binary.LittleEndian.Uint32(msg[:msgTypeFieldSize])
239261
if msgType&0xff != altsRecordMsgType {
240-
return 0, fmt.Errorf("received frame with incorrect message type %v, expected lower byte %v",
262+
return nil, 0, fmt.Errorf("received frame with incorrect message type %v, expected lower byte %v",
241263
msgType, altsRecordMsgType)
242264
}
243265
ciphertext := msg[msgTypeFieldSize:]
244266

245267
// Decrypt directly into the buffer, avoiding a copy from p.buf if
246268
// possible.
247-
if len(b) >= len(ciphertext) {
248-
dec, err := p.crypto.Decrypt(b[:0], ciphertext)
269+
if bufSize >= len(ciphertext) {
270+
allocatedBuf := pool.Get(bufSize)
271+
dec, err := p.crypto.Decrypt((*allocatedBuf)[:0], ciphertext)
249272
if err != nil {
250-
return 0, err
273+
pool.Put(allocatedBuf)
274+
return nil, 0, err
251275
}
252276
p.dropProtectedIfEmtpy()
253-
return len(dec), nil
277+
return allocatedBuf, len(dec), nil
254278
}
255279
// Decrypt requires that if the dst and ciphertext alias, they
256280
// must alias exactly. Code here used to use msg[:0], but msg
@@ -261,14 +285,15 @@ func (p *conn) Read(b []byte) (n int, err error) {
261285
// check: https://golang.org/pkg/crypto/cipher/#AEAD.
262286
p.buf, err = p.crypto.Decrypt(ciphertext[:0], ciphertext)
263287
if err != nil {
264-
return 0, err
288+
return nil, 0, err
265289
}
266290
}
267291

268-
n = copy(b, p.buf)
292+
allocatedBuf := pool.Get(bufSize)
293+
n := copy(*allocatedBuf, p.buf)
269294
p.buf = p.buf[n:]
270295
p.dropProtectedIfEmtpy()
271-
return n, nil
296+
return allocatedBuf, n, nil
272297
}
273298

274299
func (p *conn) dropProtectedIfEmtpy() {

internal/envconfig/envconfig.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,16 @@ var (
142142
//
143143
// TODO: In release v1.82.0, env var will be enabled by default.
144144
Enable8KBDefaultHeaderListSize = boolFromEnv("GRPC_GO_EXPERIMENTAL_ENABLE_8KB_DEFAULT_HEADER_LIST_SIZE", false)
145+
146+
// EnableHTTPFramerReadBufferPooling enables the use of the
147+
// readyreader.Reader interface to perform non-memory-pinning reads,
148+
// provided the underlying net.Conn supports it. This reduces memory usage
149+
// when subchannels are idle.
150+
//
151+
// This environment variable serves as an escape hatch to disable the
152+
// feature if unforeseen issues arise, and it will be removed in a future
153+
// release.
154+
EnableHTTPFramerReadBufferPooling = boolFromEnv("GRPC_GO_EXPERIMENTAL_HTTP_FRAMER_READ_BUFFER_POOLING", true)
145155
)
146156

147157
func boolFromEnv(envVar string, def bool) bool {

internal/transport/http_util.go

Lines changed: 31 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,9 @@ import (
3636
"golang.org/x/net/http2"
3737
"golang.org/x/net/http2/hpack"
3838
"google.golang.org/grpc/codes"
39+
"google.golang.org/grpc/internal/envconfig"
40+
imem "google.golang.org/grpc/internal/mem"
41+
"google.golang.org/grpc/internal/transport/readyreader"
3942
"google.golang.org/grpc/mem"
4043
)
4144

@@ -296,15 +299,15 @@ func decodeGrpcMessageUnchecked(msg string) string {
296299
}
297300

298301
type bufWriter struct {
299-
pool *sync.Pool
302+
pool *imem.SimpleBufferPool
300303
buf []byte
301304
offset int
302305
batchSize int
303306
conn io.Writer
304307
err error
305308
}
306309

307-
func newBufWriter(conn io.Writer, batchSize int, pool *sync.Pool) *bufWriter {
310+
func newBufWriter(conn io.Writer, batchSize int, pool *imem.SimpleBufferPool) *bufWriter {
308311
w := &bufWriter{
309312
batchSize: batchSize,
310313
conn: conn,
@@ -326,7 +329,7 @@ func (w *bufWriter) Write(b []byte) (int, error) {
326329
return n, toIOError(err)
327330
}
328331
if w.buf == nil {
329-
b := w.pool.Get().(*[]byte)
332+
b := w.pool.Get(w.batchSize)
330333
w.buf = *b
331334
}
332335
written := 0
@@ -407,22 +410,32 @@ type framer struct {
407410
errDetail error
408411
}
409412

410-
var writeBufferPoolMap = make(map[int]*sync.Pool)
411-
var writeBufferMutex sync.Mutex
413+
var ioBufferPoolMap = make(map[int]*imem.SimpleBufferPool)
414+
var ioBufferMutex sync.Mutex
415+
416+
func bufferedReader(r io.Reader, bufSize int) io.Reader {
417+
if bufSize <= 0 {
418+
return r
419+
}
420+
if envconfig.EnableHTTPFramerReadBufferPooling {
421+
if rr := readyreader.NewNonBlocking(r); rr != nil {
422+
readPool := ioBufferPool(bufSize)
423+
return readyreader.NewBuffered(rr, bufSize, readPool)
424+
}
425+
}
426+
return bufio.NewReaderSize(r, bufSize)
427+
}
412428

413429
func newFramer(conn io.ReadWriter, writeBufferSize, readBufferSize int, sharedWriteBuffer bool, maxHeaderListSize uint32, memPool mem.BufferPool) *framer {
414430
if writeBufferSize < 0 {
415431
writeBufferSize = 0
416432
}
417-
var r io.Reader = conn
418-
if readBufferSize > 0 {
419-
r = bufio.NewReaderSize(r, readBufferSize)
420-
}
421-
var pool *sync.Pool
433+
r := bufferedReader(conn, readBufferSize)
434+
var writePool *imem.SimpleBufferPool
422435
if sharedWriteBuffer {
423-
pool = getWriteBufferPool(writeBufferSize)
436+
writePool = ioBufferPool(writeBufferSize)
424437
}
425-
w := newBufWriter(conn, writeBufferSize, pool)
438+
w := newBufWriter(conn, writeBufferSize, writePool)
426439
f := &framer{
427440
writer: w,
428441
fr: http2.NewFramer(w, r),
@@ -578,20 +591,15 @@ func (df *parsedDataFrame) Header() http2.FrameHeader {
578591
return df.FrameHeader
579592
}
580593

581-
func getWriteBufferPool(size int) *sync.Pool {
582-
writeBufferMutex.Lock()
583-
defer writeBufferMutex.Unlock()
584-
pool, ok := writeBufferPoolMap[size]
594+
func ioBufferPool(size int) *imem.SimpleBufferPool {
595+
ioBufferMutex.Lock()
596+
defer ioBufferMutex.Unlock()
597+
pool, ok := ioBufferPoolMap[size]
585598
if ok {
586599
return pool
587600
}
588-
pool = &sync.Pool{
589-
New: func() any {
590-
b := make([]byte, size)
591-
return &b
592-
},
593-
}
594-
writeBufferPoolMap[size] = pool
601+
pool = imem.NewDirtySimplePool()
602+
ioBufferPoolMap[size] = pool
595603
return pool
596604
}
597605

internal/transport/http_util_test.go

Lines changed: 76 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
package transport
2020

2121
import (
22+
"bufio"
2223
"bytes"
2324
"errors"
2425
"fmt"
@@ -31,6 +32,9 @@ import (
3132
"time"
3233

3334
"golang.org/x/net/http2"
35+
"google.golang.org/grpc/internal/envconfig"
36+
"google.golang.org/grpc/internal/testutils"
37+
"google.golang.org/grpc/internal/transport/readyreader"
3438
"google.golang.org/grpc/mem"
3539
)
3640

@@ -259,7 +263,7 @@ func (s) TestWriteBadConnection(t *testing.T) {
259263
// Configure the bufWriter with a batchsize that results in data being flushed
260264
// to the underlying conn, midway through Write().
261265
writeBufferSize := (len(data) - 1) / 2
262-
writer := newBufWriter(&badNetworkConn{}, writeBufferSize, getWriteBufferPool(writeBufferSize))
266+
writer := newBufWriter(&badNetworkConn{}, writeBufferSize, ioBufferPool(writeBufferSize))
263267

264268
errCh := make(chan error, 1)
265269
go func() {
@@ -413,3 +417,74 @@ func (s) TestFramer_ParseDataFrame(t *testing.T) {
413417
})
414418
}
415419
}
420+
421+
type testReadyReader struct {
422+
readyreader.Reader
423+
}
424+
425+
func (t *testReadyReader) Read([]byte) (int, error) {
426+
return 0, io.EOF
427+
}
428+
429+
func (s) TestBufferedReader(t *testing.T) {
430+
normalReader := bytes.NewReader(nil)
431+
432+
tests := []struct {
433+
name string
434+
reader io.Reader
435+
bufSize int
436+
enablePooling bool
437+
wantTypeOf any
438+
}{
439+
{
440+
name: "bufSize_0",
441+
reader: normalReader,
442+
bufSize: 0,
443+
enablePooling: true,
444+
wantTypeOf: (*bytes.Reader)(nil),
445+
},
446+
{
447+
name: "env_var_disabled_normal_reader",
448+
reader: normalReader,
449+
bufSize: 10,
450+
enablePooling: false,
451+
wantTypeOf: (*bufio.Reader)(nil),
452+
},
453+
{
454+
name: "env_var_disabled_ready_reader",
455+
reader: &testReadyReader{},
456+
bufSize: 10,
457+
enablePooling: false,
458+
wantTypeOf: (*bufio.Reader)(nil),
459+
},
460+
{
461+
name: "env_var_enabled_normal_reader",
462+
reader: normalReader,
463+
bufSize: 10,
464+
enablePooling: true,
465+
wantTypeOf: (*bufio.Reader)(nil),
466+
},
467+
{
468+
name: "env_var_enabled_ready_reader",
469+
reader: &testReadyReader{},
470+
bufSize: 10,
471+
enablePooling: true,
472+
wantTypeOf: readyreader.NewBuffered(nil, 10, mem.DefaultBufferPool()),
473+
},
474+
}
475+
476+
for _, tt := range tests {
477+
t.Run(tt.name, func(t *testing.T) {
478+
testutils.SetEnvConfig(t, &envconfig.EnableHTTPFramerReadBufferPooling, tt.enablePooling)
479+
480+
got := bufferedReader(tt.reader, tt.bufSize)
481+
482+
gotType := reflect.TypeOf(got)
483+
wantType := reflect.TypeOf(tt.wantTypeOf)
484+
485+
if gotType != wantType {
486+
t.Errorf("bufferedReader() type = %v, want %v", gotType, wantType)
487+
}
488+
})
489+
}
490+
}

0 commit comments

Comments
 (0)