From 19a8cef60cf31768661628f5a2d31bc9488df540 Mon Sep 17 00:00:00 2001 From: Roy Salame Date: Tue, 9 Sep 2025 22:16:33 -0400 Subject: [PATCH 1/4] proto: enable use cached size option --- encoding/proto/proto.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/encoding/proto/proto.go b/encoding/proto/proto.go index ceec319dd2fb..f8740ecc8164 100644 --- a/encoding/proto/proto.go +++ b/encoding/proto/proto.go @@ -23,10 +23,11 @@ package proto import ( "fmt" - "google.golang.org/grpc/encoding" - "google.golang.org/grpc/mem" "google.golang.org/protobuf/proto" "google.golang.org/protobuf/protoadapt" + + "google.golang.org/grpc/encoding" + "google.golang.org/grpc/mem" ) // Name is the name registered for the proto compressor. @@ -48,7 +49,7 @@ func (c *codecV2) Marshal(v any) (data mem.BufferSlice, err error) { size := proto.Size(vv) if mem.IsBelowBufferPoolingThreshold(size) { - buf, err := proto.Marshal(vv) + buf, err := proto.MarshalOptions{UseCachedSize: true}.Marshal(vv) if err != nil { return nil, err } @@ -56,7 +57,7 @@ func (c *codecV2) Marshal(v any) (data mem.BufferSlice, err error) { } else { pool := mem.DefaultBufferPool() buf := pool.Get(size) - if _, err := (proto.MarshalOptions{}).MarshalAppend((*buf)[:0], vv); err != nil { + if _, err := (proto.MarshalOptions{UseCachedSize: true}).MarshalAppend((*buf)[:0], vv); err != nil { pool.Put(buf) return nil, err } From 188bece3e83277f4c65bde21061354e70641cd59 Mon Sep 17 00:00:00 2001 From: Roy Salame Date: Thu, 11 Sep 2025 07:49:51 -0400 Subject: [PATCH 2/4] proto: move MarshalOptions closer to size call and add comment clarifying the use of UseCachedSize --- encoding/proto/proto.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/encoding/proto/proto.go b/encoding/proto/proto.go index f8740ecc8164..c5c41a22c481 100644 --- a/encoding/proto/proto.go +++ b/encoding/proto/proto.go @@ -48,8 +48,12 @@ func (c *codecV2) Marshal(v any) (data mem.BufferSlice, err error) { } size := proto.Size(vv) + // proto.Size caches the size, enabling UseCachedSize + // lets us reuse that value instead of recomputing it during marshal. + marshalOptions := proto.MarshalOptions{UseCachedSize: true} + if mem.IsBelowBufferPoolingThreshold(size) { - buf, err := proto.MarshalOptions{UseCachedSize: true}.Marshal(vv) + buf, err := marshalOptions.Marshal(vv) if err != nil { return nil, err } @@ -57,7 +61,7 @@ func (c *codecV2) Marshal(v any) (data mem.BufferSlice, err error) { } else { pool := mem.DefaultBufferPool() buf := pool.Get(size) - if _, err := (proto.MarshalOptions{UseCachedSize: true}).MarshalAppend((*buf)[:0], vv); err != nil { + if _, err := marshalOptions.MarshalAppend((*buf)[:0], vv); err != nil { pool.Put(buf) return nil, err } From 50dfe64fd036a322e70ed96c1ee510aecb844aab Mon Sep 17 00:00:00 2001 From: Roy Salame Date: Thu, 11 Sep 2025 15:04:49 -0400 Subject: [PATCH 3/4] proto: revert import sort --- encoding/proto/proto.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/encoding/proto/proto.go b/encoding/proto/proto.go index c5c41a22c481..d98f2e0352e6 100644 --- a/encoding/proto/proto.go +++ b/encoding/proto/proto.go @@ -23,11 +23,10 @@ package proto import ( "fmt" - "google.golang.org/protobuf/proto" - "google.golang.org/protobuf/protoadapt" - "google.golang.org/grpc/encoding" "google.golang.org/grpc/mem" + "google.golang.org/protobuf/proto" + "google.golang.org/protobuf/protoadapt" ) // Name is the name registered for the proto compressor. From ec0c47eaaa3f81bc78a0be16aaf0689387381a00 Mon Sep 17 00:00:00 2001 From: Roy Salame Date: Thu, 11 Sep 2025 18:45:32 -0400 Subject: [PATCH 4/4] proto: improve comment --- encoding/proto/proto.go | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/encoding/proto/proto.go b/encoding/proto/proto.go index d98f2e0352e6..1ab874c7ad26 100644 --- a/encoding/proto/proto.go +++ b/encoding/proto/proto.go @@ -46,9 +46,21 @@ func (c *codecV2) Marshal(v any) (data mem.BufferSlice, err error) { return nil, fmt.Errorf("proto: failed to marshal, message is %T, want proto.Message", v) } + // Important: if we remove this Size call then we cannot use + // UseCachedSize in MarshalOptions below. size := proto.Size(vv) - // proto.Size caches the size, enabling UseCachedSize - // lets us reuse that value instead of recomputing it during marshal. + + // MarshalOptions with UseCachedSize allows reusing the result from the + // previous Size call. This is safe here because: + // + // 1. We just computed the size. + // 2. We assume the message is not being mutated concurrently. + // + // Important: If the proto.Size call above is removed, using UseCachedSize + // becomes unsafe and may lead to incorrect marshaling. + // + // For more details, see the doc of UseCachedSize: + // https://pkg.go.dev/google.golang.org/protobuf/proto#MarshalOptions marshalOptions := proto.MarshalOptions{UseCachedSize: true} if mem.IsBelowBufferPoolingThreshold(size) {