Skip to content

Commit a547889

Browse files
Eliminate redundant receive-side copy in gRPC protobuf decode path
Every protobuf message received over gRPC was copied twice: once by the codec (Materialize, necessary) and again by the encoding layer (bufferpool.ReadFrom, redundant). The second copy exists because the transport wraps the already-materialized []byte in bytes.NewReader, and the encoding layer has no way to extract the raw bytes. Add a bytesBody type to the transport layer that implements io.ReadCloser and exposes the underlying []byte via a Bytes() method. The encoding layer's unmarshal function type-asserts for this method and skips the bufferpool copy when available. Non-gRPC transports (HTTP, TChannel) fall through to the existing ReadFrom path. Changes: - transport/grpc: Add bytesBody with Bytes() method for zero-copy access - transport/grpc: Update all four receive paths (server stream, client stream, unary inbound, unary outbound) to use bytesBody - encoding/protobuf: Add Bytes() fast-path in unmarshal (gogo and v2) Safety: - bytesBody is unexported and implements io.ReadCloser, the interface expected by transport.Request.Body and transport.StreamMessage.Body. It is a drop-in replacement for bytes.NewReader / ioutil.NopCloser. - The Bytes() fast-path is opt-in via interface type assertion. Any reader that does not implement Bytes() (HTTP, TChannel) falls through to the existing bufferpool.ReadFrom slow path with zero behavior change. - The fast-path calls unmarshalBytes, the same function the slow path calls after draining the reader — deserialization logic is identical. - No public API changes; bytesBody is internal to transport/grpc. Benchmark added in #2515 Benchmark (BenchmarkUnmarshalBytesReader, n=10, AMD EPYC 9B45, Go 1.26.1): Library CPU Δ Heap Δ GC Cycles Δ GC Pause Δ gogo -16.1% +1.1% -3.6% -9.3% v2 -15.1% +0.9% -2.8% -7.6% average -15.6% +1.0% -3.2% -8.5% Heap increase at small payloads (<1KB) is due to bytesBody struct being marginally larger than bytes.Reader; at >=10KB the eliminated bufferpool copy dominates and heap decreases. RELEASE NOTES: N/A (internal optimization, no API changes) Made-with: Cursor Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 338f277 commit a547889

8 files changed

Lines changed: 253 additions & 6 deletions

File tree

encoding/protobuf/marshal.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,19 @@ func newCodec(anyResolver jsonpb.AnyResolver) *codec {
5454
}
5555

5656
func unmarshal(encoding transport.Encoding, reader io.Reader, message proto.Message, c *codec) error {
57+
// Fast path: if the reader directly exposes raw bytes, skip the redundant
58+
// copy through bufferpool. This is the case for gRPC transports where the
59+
// bytes have already been materialized from the wire.
60+
if br, ok := reader.(interface{ Bytes() []byte }); ok {
61+
body := br.Bytes()
62+
if len(body) == 0 {
63+
return nil
64+
}
65+
return unmarshalBytes(encoding, body, message, c)
66+
}
67+
68+
// Slow path: drain the reader into a pooled buffer. Used by non-gRPC
69+
// transports (HTTP, TChannel) that provide a plain io.Reader.
5770
buf := bufferpool.Get()
5871
defer bufferpool.Put(buf)
5972
if _, err := buf.ReadFrom(reader); err != nil {

encoding/protobuf/marshal_test.go

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,14 @@
2121
package protobuf
2222

2323
import (
24+
"bytes"
2425
"strings"
2526
"testing"
2627

28+
"github.com/gogo/protobuf/proto"
29+
"github.com/gogo/protobuf/types"
2730
"github.com/stretchr/testify/assert"
31+
"github.com/stretchr/testify/require"
2832
"go.uber.org/yarpc/api/transport"
2933
"go.uber.org/yarpc/yarpcerrors"
3034
)
@@ -38,3 +42,71 @@ func TestUnhandledEncoding(t *testing.T) {
3842
assert.NotNil(t, cleanup, "cleanup function should never be nil")
3943
assert.NotPanics(t, func() { cleanup() }, "cleanup should be safe to call even on error")
4044
}
45+
46+
// testBytesReader simulates a body from gRPC transport that exposes raw bytes
47+
// via a Bytes() method, allowing the unmarshal fast path to skip bufferpool copies.
48+
type testBytesReader struct {
49+
data []byte
50+
reader *bytes.Reader
51+
bytesCalled bool
52+
readCalled bool
53+
}
54+
55+
func newTestBytesReader(data []byte) *testBytesReader {
56+
return &testBytesReader{data: data, reader: bytes.NewReader(data)}
57+
}
58+
59+
func (r *testBytesReader) Read(p []byte) (int, error) {
60+
r.readCalled = true
61+
return r.reader.Read(p)
62+
}
63+
64+
func (r *testBytesReader) Bytes() []byte {
65+
r.bytesCalled = true
66+
return r.data
67+
}
68+
69+
// TestUnmarshalFastPath verifies that when the reader exposes a Bytes() method,
70+
// unmarshal uses the zero-copy path and never calls Read().
71+
func TestUnmarshalFastPath(t *testing.T) {
72+
c := newCodec(nil)
73+
74+
t.Run("Bytes called and Read not called", func(t *testing.T) {
75+
original := &types.StringValue{Value: "hello"}
76+
data, err := proto.Marshal(original)
77+
require.NoError(t, err)
78+
79+
reader := newTestBytesReader(data)
80+
got := &types.StringValue{}
81+
82+
err = unmarshal(Encoding, reader, got, c)
83+
assert.NoError(t, err)
84+
assert.Equal(t, original.Value, got.Value, "Message should be deserialized correctly via fast path")
85+
assert.True(t, reader.bytesCalled, "Bytes() should be called on fast path")
86+
assert.False(t, reader.readCalled, "Read() should not be called on fast path")
87+
})
88+
89+
t.Run("empty body returns nil", func(t *testing.T) {
90+
reader := newTestBytesReader([]byte{})
91+
92+
err := unmarshal(Encoding, reader, nil, c)
93+
assert.NoError(t, err, "Empty body on fast path should return nil")
94+
assert.True(t, reader.bytesCalled, "Bytes() should still be called for empty body")
95+
assert.False(t, reader.readCalled, "Read() should not be called for empty body")
96+
})
97+
98+
t.Run("invalid encoding returns error", func(t *testing.T) {
99+
reader := newTestBytesReader([]byte("data"))
100+
101+
err := unmarshal(transport.Encoding("unknown"), reader, nil, c)
102+
assert.Equal(t, yarpcerrors.CodeInternal, yarpcerrors.FromError(err).Code(),
103+
"Fast path should still return encoding error for unrecognized encoding")
104+
})
105+
106+
t.Run("malformed protobuf returns unmarshal error", func(t *testing.T) {
107+
reader := newTestBytesReader([]byte{0xff, 0xff, 0xff})
108+
109+
err := unmarshal(Encoding, reader, nil, c)
110+
assert.Error(t, err, "Malformed protobuf should return an error on fast path")
111+
})
112+
}

encoding/protobuf/v2/marshal.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,19 @@ func newCodec(anyResolver AnyResolver) *codec {
7070
}
7171

7272
func unmarshal(encoding transport.Encoding, reader io.Reader, message proto.Message, codec *codec) error {
73+
// Fast path: if the reader directly exposes raw bytes, skip the redundant
74+
// copy through bufferpool. This is the case for gRPC transports where the
75+
// bytes have already been materialized from the wire.
76+
if br, ok := reader.(interface{ Bytes() []byte }); ok {
77+
body := br.Bytes()
78+
if len(body) == 0 {
79+
return nil
80+
}
81+
return unmarshalBytes(encoding, body, message, codec)
82+
}
83+
84+
// Slow path: drain the reader into a pooled buffer. Used by non-gRPC
85+
// transports (HTTP, TChannel) that provide a plain io.Reader.
7386
buf := bufferpool.Get()
7487
defer bufferpool.Put(buf)
7588
if _, err := buf.ReadFrom(reader); err != nil {

encoding/protobuf/v2/marshal_test.go

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,16 @@
2121
package v2
2222

2323
import (
24+
"bytes"
2425
"strings"
2526
"testing"
2627

2728
"github.com/stretchr/testify/assert"
29+
"github.com/stretchr/testify/require"
2830
"go.uber.org/yarpc/api/transport"
2931
"go.uber.org/yarpc/yarpcerrors"
32+
"google.golang.org/protobuf/proto"
33+
"google.golang.org/protobuf/types/known/wrapperspb"
3034
)
3135

3236
func TestUnhandledEncoding(t *testing.T) {
@@ -35,3 +39,71 @@ func TestUnhandledEncoding(t *testing.T) {
3539
_, _, err := marshal(transport.Encoding("foo"), nil, newCodec(nil))
3640
assert.Equal(t, yarpcerrors.CodeInternal, yarpcerrors.FromError(err).Code())
3741
}
42+
43+
// testBytesReader simulates a body from gRPC transport that exposes raw bytes
44+
// via a Bytes() method, allowing the unmarshal fast path to skip bufferpool copies.
45+
type testBytesReader struct {
46+
data []byte
47+
reader *bytes.Reader
48+
bytesCalled bool
49+
readCalled bool
50+
}
51+
52+
func newTestBytesReader(data []byte) *testBytesReader {
53+
return &testBytesReader{data: data, reader: bytes.NewReader(data)}
54+
}
55+
56+
func (r *testBytesReader) Read(p []byte) (int, error) {
57+
r.readCalled = true
58+
return r.reader.Read(p)
59+
}
60+
61+
func (r *testBytesReader) Bytes() []byte {
62+
r.bytesCalled = true
63+
return r.data
64+
}
65+
66+
// TestUnmarshalFastPath verifies that when the reader exposes a Bytes() method,
67+
// unmarshal uses the zero-copy path and never calls Read().
68+
func TestUnmarshalFastPath(t *testing.T) {
69+
c := newCodec(nil)
70+
71+
t.Run("Bytes called and Read not called", func(t *testing.T) {
72+
original := &wrapperspb.StringValue{Value: "hello"}
73+
data, err := proto.Marshal(original)
74+
require.NoError(t, err)
75+
76+
reader := newTestBytesReader(data)
77+
got := &wrapperspb.StringValue{}
78+
79+
err = unmarshal(Encoding, reader, got, c)
80+
assert.NoError(t, err)
81+
assert.Equal(t, original.Value, got.Value, "Message should be deserialized correctly via fast path")
82+
assert.True(t, reader.bytesCalled, "Bytes() should be called on fast path")
83+
assert.False(t, reader.readCalled, "Read() should not be called on fast path")
84+
})
85+
86+
t.Run("empty body returns nil", func(t *testing.T) {
87+
reader := newTestBytesReader([]byte{})
88+
89+
err := unmarshal(Encoding, reader, nil, c)
90+
assert.NoError(t, err, "Empty body on fast path should return nil")
91+
assert.True(t, reader.bytesCalled, "Bytes() should still be called for empty body")
92+
assert.False(t, reader.readCalled, "Read() should not be called for empty body")
93+
})
94+
95+
t.Run("invalid encoding returns error", func(t *testing.T) {
96+
reader := newTestBytesReader([]byte("data"))
97+
98+
err := unmarshal(transport.Encoding("unknown"), reader, nil, c)
99+
assert.Equal(t, yarpcerrors.CodeInternal, yarpcerrors.FromError(err).Code(),
100+
"Fast path should still return encoding error for unrecognized encoding")
101+
})
102+
103+
t.Run("malformed protobuf returns unmarshal error", func(t *testing.T) {
104+
reader := newTestBytesReader([]byte{0xff, 0xff, 0xff})
105+
106+
err := unmarshal(Encoding, reader, &wrapperspb.StringValue{}, c)
107+
assert.Error(t, err, "Malformed protobuf should return an error on fast path")
108+
})
109+
}

transport/grpc/handler.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
package grpc
2222

2323
import (
24-
"bytes"
2524
"strings"
2625
"time"
2726

@@ -193,7 +192,7 @@ func (h *handler) handleUnary(
193192
return err
194193
}
195194

196-
transportRequest.Body = bytes.NewReader(requestData)
195+
transportRequest.Body = newBytesBody(requestData)
197196
transportRequest.BodySize = len(requestData)
198197

199198
responseWriter := newResponseWriter()

transport/grpc/outbound.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@
2121
package grpc
2222

2323
import (
24-
"bytes"
2524
"context"
2625
"io/ioutil"
2726
"strings"
@@ -139,7 +138,7 @@ func (o *Outbound) DirectCall(ctx context.Context, request *transport.Request) (
139138
return nil, err
140139
}
141140
return &transport.Response{
142-
Body: ioutil.NopCloser(bytes.NewReader(responseBody)),
141+
Body: newBytesBody(responseBody),
143142
BodySize: len(responseBody),
144143
Headers: responseHeaders,
145144
ApplicationError: metadataToIsApplicationError(responseMD),

transport/grpc/stream.go

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ func (ss *serverStream) ReceiveMessage(_ context.Context) (*transport.StreamMess
9393
return nil, toYARPCStreamError(err)
9494
}
9595
return &transport.StreamMessage{
96-
Body: readCloser{bytes.NewReader(msg)},
96+
Body: newBytesBody(msg),
9797
BodySize: len(msg),
9898
}, nil
9999
}
@@ -106,6 +106,21 @@ func (r readCloser) Close() error {
106106
return nil
107107
}
108108

109+
// bytesBody wraps a []byte and exposes direct access to
110+
// the underlying bytes via the Bytes method.
111+
type bytesBody struct {
112+
data []byte
113+
reader *bytes.Reader
114+
}
115+
116+
func newBytesBody(data []byte) *bytesBody {
117+
return &bytesBody{data: data, reader: bytes.NewReader(data)}
118+
}
119+
120+
func (b *bytesBody) Read(p []byte) (int, error) { return b.reader.Read(p) }
121+
func (b *bytesBody) Bytes() []byte { return b.data }
122+
func (b *bytesBody) Close() error { return nil }
123+
109124
func (ss *serverStream) SendHeaders(headers transport.Headers) error {
110125
md := make(metadata.MD, headers.Len())
111126
for k, v := range headers.Items() {
@@ -173,7 +188,7 @@ func (cs *clientStream) ReceiveMessage(context.Context) (*transport.StreamMessag
173188
if err := cs.stream.RecvMsg(&msg); err != nil {
174189
return nil, toYARPCStreamError(cs.closeWithErr(err))
175190
}
176-
return &transport.StreamMessage{Body: ioutil.NopCloser(bytes.NewReader(msg))}, nil
191+
return &transport.StreamMessage{Body: newBytesBody(msg)}, nil
177192
}
178193

179194
func (cs *clientStream) Close(context.Context) error {

transport/grpc/stream_test.go

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,62 @@ func TestServerStreamSendMessage(t *testing.T) {
144144
})
145145
}
146146

147+
// TestStreamReceiveMessage verifies bytesBody wiring: both server and client
148+
// receive paths wrap payloads in bytesBody (exposes Bytes() for zero-copy).
149+
// The actual fast-path dispatch is tested in TestUnmarshalFastPath.
150+
func TestStreamReceiveMessage(t *testing.T) {
151+
t.Run("server stream wraps body in bytesBody", func(t *testing.T) {
152+
payload := []byte("server receive test")
153+
154+
mockStream := &mockGRPCServerStream{
155+
recvMsgFunc: func(m any) error {
156+
ptr := m.(*[]byte)
157+
*ptr = payload
158+
return nil
159+
},
160+
}
161+
162+
ss := &serverStream{
163+
ctx: context.Background(),
164+
stream: mockStream,
165+
req: &transport.StreamRequest{Meta: &transport.RequestMeta{}},
166+
}
167+
168+
msg, err := ss.ReceiveMessage(context.Background())
169+
assert.NoError(t, err)
170+
171+
br, ok := msg.Body.(interface{ Bytes() []byte })
172+
assert.True(t, ok, "Body should implement Bytes() []byte")
173+
assert.Equal(t, payload, br.Bytes())
174+
assert.Equal(t, len(payload), msg.BodySize)
175+
})
176+
177+
t.Run("client stream wraps body in bytesBody", func(t *testing.T) {
178+
payload := []byte("client receive test")
179+
180+
mockStream := &mockGRPCClientStream{
181+
recvMsgFunc: func(m any) error {
182+
ptr := m.(*[]byte)
183+
*ptr = payload
184+
return nil
185+
},
186+
}
187+
188+
cs := &clientStream{
189+
ctx: context.Background(),
190+
stream: mockStream,
191+
req: &transport.StreamRequest{Meta: &transport.RequestMeta{}},
192+
}
193+
194+
msg, err := cs.ReceiveMessage(context.Background())
195+
assert.NoError(t, err)
196+
197+
br, ok := msg.Body.(interface{ Bytes() []byte })
198+
assert.True(t, ok, "Body should implement Bytes() []byte")
199+
assert.Equal(t, payload, br.Bytes())
200+
})
201+
}
202+
147203
func TestClientStreamSendMessage(t *testing.T) {
148204
t.Run("passes mem.Buffer to gRPC without conversion", func(t *testing.T) {
149205
data := []byte("client test")
@@ -212,6 +268,7 @@ func TestClientStreamSendMessage(t *testing.T) {
212268
type mockGRPCServerStream struct {
213269
grpc.ServerStream
214270
sendMsgFunc func(m any) error
271+
recvMsgFunc func(m any) error
215272
}
216273

217274
func (m *mockGRPCServerStream) SendMsg(msg any) error {
@@ -221,6 +278,13 @@ func (m *mockGRPCServerStream) SendMsg(msg any) error {
221278
return nil
222279
}
223280

281+
func (m *mockGRPCServerStream) RecvMsg(msg any) error {
282+
if m.recvMsgFunc != nil {
283+
return m.recvMsgFunc(msg)
284+
}
285+
return nil
286+
}
287+
224288
type mockGRPCClientStream struct {
225289
grpc.ClientStream
226290
sendMsgFunc func(m any) error

0 commit comments

Comments
 (0)