Eliminate redundant receive-side copy in gRPC protobuf decode path by kshitijsuri90 · Pull Request #2441 · yarpc/yarpc-go

kshitijsuri90 · 2026-04-03T15:42:43Z

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.

In depth document (Uber access required): link

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

📊 Benchmark 1: `protobuf v1`

(encoding/protobuf/codec_bench_test.go)

Library: github.com/gogo/protobuf
Parameters: -benchmem -count=10 (Payload sizes: 350B / 10KB / 1MB)
Environment: go1.26.1 linux/amd64, AMD EPYC 9B45
Comparison: HEAD~1 (before) vs HEAD (after)

Execution Time (`sec/op`)

Benchmark	Old	New	Delta (vs base)
`Small_350B-96`	49.92u ± 2%	47.81u ± 2%	-4.23% (p=0.000 n=10)
`Medium_10KB-96`	91.33u ± 165%	83.40u ± 2%	-8.68% (p=0.000 n=10)
`Large_1MB-96`	3.218m ± 3%	2.109m ± 2%	-34.44% (p=0.000 n=10)
geomean	244.8u	203.4u	-16.92%

Memory Allocated (`B/op`)

Benchmark	Old	New	Delta (vs base)
`Small_350B-96`	22.75Ki ± 0%	22.67Ki ± 0%	-0.36% (p=0.000 n=10)
`Medium_10KB-96`	105.9Ki ± 0%	101.1Ki ± 0%	-4.56% (p=0.000 n=10)
`Large_1MB-96`	14.537Mi ± 1%	8.183Mi ± 0%	-43.71% (p=0.000 n=10)
geomean	329.8Ki	267.8Ki	-18.81%

Allocations (`allocs/op`)

Benchmark	Old	New	Delta (vs base)
`Small_350B-96`	316.0 ± 0%	317.0 ± 0%	+0.32% (p=0.000 n=10)
`Medium_10KB-96`	323.0 ± 0%	323.0 ± 0%	~ (p=1.000 n=10)
`Large_1MB-96`	552.5 ± 2%	529.0 ± 2%	-4.25% (p=0.000 n=10)
geomean	383.5	378.4	-1.33%

📊 Benchmark 2: `protobuf v2`

(encoding/protobuf/v2/codec_bench_test.go)

Library: google.golang.org/protobuf
Parameters: -benchmem -count=10 (Payload sizes: 350B / 10KB / 1MB)
Environment: go1.26.1 linux/amd64, AMD EPYC 9B45
Comparison: HEAD~1 (before) vs HEAD (after)

Execution Time (`sec/op`)

Benchmark	Old	New	Delta (vs base)
`Small_350B-96`	50.21u ± 3%	48.40u ± 2%	-3.61% (p=0.000 n=10)
`Medium_10KB-96`	93.74u ± 95%	87.47u ± 3%	-6.68% (p=0.000 n=10)
`Large_1MB-96`	3.320m ± 4%	2.178m ± 4%	-34.40% (p=0.000 n=10)
geomean	250.0u	209.7u	-16.12%

Memory Allocated (`B/op`)

Benchmark	Old	New	Delta (vs base)
`Small_350B-96`	22.89Ki ± 0%	22.80Ki ± 0%	-0.37% (p=0.000 n=10)
`Medium_10KB-96`	106.0Ki ± 1%	101.1Ki ± 0%	-4.61% (p=0.000 n=10)
`Large_1MB-96`	14.758Mi ± 2%	8.182Mi ± 0%	-44.56% (p=0.000 n=10)
geomean	332.2Ki	268.3Ki	-19.23%

Allocations (`allocs/op`)

Benchmark	Old	New	Delta (vs base)
`Small_350B-96`	317.0 ± 0%	318.0 ± 0%	+0.32% (p=0.000 n=10)
`Medium_10KB-96`	324.0 ± 0%	324.0 ± 0%	~ (p=0.474 n=10)
`Large_1MB-96`	554.5 ± 1%	529.0 ± 1%	-4.60% (p=0.000 n=10)
geomean	384.7	379.1	-1.45%
RELEASE NOTES: N/A (internal optimization, no API changes)

Test plan

TestBytesBody — Read, Bytes (pointer identity / zero-copy), Close, empty data, large payload
TestServerStreamReceiveMessage — body exposes Bytes(), empty message
TestClientStreamReceiveMessage — body exposes Bytes(), empty message
TestUnmarshalFastPath — Bytes() called / Read() not called, empty body, invalid encoding, malformed protobuf
TestUnmarshalSlowPath — deserializes correctly without Bytes(), empty body
BenchmarkUnmarshalBytesReader — 100B/1KB/10KB/100KB with GC metrics (gogo + v2)
Full test suite passes: go test -count=1 ./transport/grpc/... ./encoding/protobuf/...

RELEASE NOTES: N/A (internal optimization, no API changes)

Update (2026-07-27): rebased onto current main (clean, no conflicts) and fixed the failing go-1-dot-23-lint job:

gofmt: struct field alignment in encoding/protobuf/marshal_test.go
staticcheck U1000: removed the now-unused readCloser type from transport/grpc/stream.go (its only usage was replaced by bytesBody)

make lint verified green in the CI docker image (yarpc-go-1.23).

CLAassistant · 2026-04-03T15:42:52Z

All committers have signed the CLA.

rabbbit · 2026-05-15T03:40:49Z

+
+}
+
+func TestClientStreamReceiveMessage(t *testing.T) {


why are we adding all these? did we not have coverage before?

I would say each new test should have a comment why it needs to exist, as opposed to re-using pre-existing tests.

I added these tests to verify explicitly, the specific behaviour being introduced in the patch. I can add these as subtests to original if that is required?

Strongtests are strongly preferred if they're extensible enough to re-use.

(and see above on comments)

rabbbit · 2026-05-15T03:41:49Z

+	})
+}
+
+func TestUnmarshalSlowPath(t *testing.T) {


wouldn't tests for this exist already?

There exists a coverage test for unmarshal. I added this to explicitly check the slow path. I can remove it if that's the preference?

In what cases would there be a slow path in grpc?

I think we should be adding comments (in code) to every test. Context is precious here.

rabbbit · 2026-05-15T03:42:37Z

 	_, _, err := marshal(transport.Encoding("foo"), nil, newCodec(nil))
 	assert.Equal(t, yarpcerrors.CodeInternal, yarpcerrors.FromError(err).Code())
 }
+


what's up with the v2 proto - which one is used, which one is not?

Both are used in the field at Uber: v1, v2

kshitijsuri90 · 2026-05-17T08:31:52Z

+// Run with:
+//
+//	go test -bench=BenchmarkGRPCCodec_RoundTrip -benchmem ./encoding/protobuf/v2/
+func BenchmarkGRPCCodec_RoundTrip(b *testing.B) {


@rabbbit Added this because there was no v2 equivalent of a round trip test for grpc. There exists one for v1 at

yarpc-go/encoding/protobuf/codec_bench_test.go

Line 42 in 0052f07

func BenchmarkGRPCCodec_RoundTrip(b *testing.B) {

Can we add this as a separate PR? Much easier to stamp/approve, let's make this diff smaller.

Also I think we upgraded to new go version now, perhaps you can even use B.Loop?

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 yarpc#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. Lint fixes (2026-07-27): gofmt field alignment in encoding/protobuf/marshal_test.go; removed the now-unused readCloser type from transport/grpc/stream.go (staticcheck U1000) — its only usage was replaced by bytesBody. RELEASE NOTES: N/A (internal optimization, no API changes) Made-with: Cursor Co-authored-by: Cursor <cursoragent@cursor.com>

rabbbit · 2026-07-29T03:04:34Z

 		return nil, toYARPCStreamError(cs.closeWithErr(err))
 	}
-	return &transport.StreamMessage{Body: ioutil.NopCloser(bytes.NewReader(msg))}, nil
+	return &transport.StreamMessage{Body: newBytesBody(msg)}, nil


I wondered for a second which case we care about - unary or streaming, but this seems harmless.

kshitijsuri90 force-pushed the kshitij/eliminate-receive-side-copy branch 3 times, most recently from 109764f to ce4a313 Compare April 8, 2026 14:14

rabbbit reviewed May 15, 2026

View reviewed changes

kshitijsuri90 commented May 17, 2026

View reviewed changes

kshitijsuri90 force-pushed the kshitij/eliminate-receive-side-copy branch 4 times, most recently from 69bbfb6 to a547889 Compare July 5, 2026 09:55

kshitijsuri90 force-pushed the kshitij/eliminate-receive-side-copy branch from a547889 to c62c34e Compare July 27, 2026 08:32

rabbbit approved these changes Jul 29, 2026

View reviewed changes

bananacocodrilo added this to the v1.89.8 milestone Aug 14, 2026

Merge branch 'main' into kshitij/eliminate-receive-side-copy

5506cc7

Navigation Menu

Uh oh!

Eliminate redundant receive-side copy in gRPC protobuf decode path - #2441

Eliminate redundant receive-side copy in gRPC protobuf decode path#2441
kshitijsuri90 wants to merge 2 commits into
yarpc:mainfrom
kshitijsuri90:kshitij/eliminate-receive-side-copy

kshitijsuri90 commented Apr 3, 2026 •

edited

Loading

Uh oh!

CLAassistant commented Apr 3, 2026 •

edited

Loading

Uh oh!

rabbbit May 15, 2026

Uh oh!

kshitijsuri90 May 17, 2026

Uh oh!

rabbbit Jun 19, 2026

Uh oh!

rabbbit May 15, 2026

Uh oh!

kshitijsuri90 May 17, 2026

Uh oh!

rabbbit Jun 19, 2026

Uh oh!

rabbbit May 15, 2026

Uh oh!

kshitijsuri90 May 17, 2026

Uh oh!

kshitijsuri90 May 17, 2026

Uh oh!

rabbbit Jun 19, 2026

Uh oh!

rabbbit Jul 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

kshitijsuri90 commented Apr 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Safety

Benchmark

📊 Benchmark 1: protobuf v1

Execution Time (sec/op)

Memory Allocated (B/op)

Allocations (allocs/op)

📊 Benchmark 2: protobuf v2

Execution Time (sec/op)

Memory Allocated (B/op)

Allocations (allocs/op)

Test plan

Uh oh!

CLAassistant commented Apr 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

kshitijsuri90 commented Apr 3, 2026 •

edited

Loading

📊 Benchmark 1: `protobuf v1`

Execution Time (`sec/op`)

Memory Allocated (`B/op`)

Allocations (`allocs/op`)

📊 Benchmark 2: `protobuf v2`

Execution Time (`sec/op`)

Memory Allocated (`B/op`)

Allocations (`allocs/op`)

CLAassistant commented Apr 3, 2026 •

edited

Loading