proto, internal/impl: don't create fast path Size for legacy Marshalers
Implementations of the legacy Marshaler type have no way to efficiently
compute the size of the message. Rather than generating an inefficient
fast-path Size method which marshals the message and examines the
length of the result, don't generate a fast-path at all.
Drop the requirement that a fast-path MarshalAppend requires a
corresponding Size.
Avoids O(N^2) behavior when marshaling a legacy Marshaler that
recursively calls proto.Marshal.
Change-Id: I4793cf32275d08f29c8e1a1a44a193d9a5724058
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/213443
Reviewed-by: Joe Tsai <joetsai@google.com>
diff --git a/internal/impl/legacy_message.go b/internal/impl/legacy_message.go
index aaf6d60..ac96e71 100644
--- a/internal/impl/legacy_message.go
+++ b/internal/impl/legacy_message.go
@@ -51,7 +51,6 @@
v := reflect.Zero(t).Interface()
if _, ok := v.(legacyMarshaler); ok {
mi.methods.MarshalAppend = legacyMarshalAppend
- mi.methods.Size = legacySize
// We have no way to tell whether the type's Marshal method
// supports deterministic serialization or not, but this
@@ -364,7 +363,6 @@
}
var legacyProtoMethods = &piface.Methods{
- Size: legacySize,
MarshalAppend: legacyMarshalAppend,
Unmarshal: legacyUnmarshal,
@@ -375,11 +373,6 @@
Flags: piface.SupportMarshalDeterministic,
}
-func legacySize(m protoreflect.Message, opts piface.MarshalOptions) int {
- b, _ := legacyMarshalAppend(nil, m, opts)
- return len(b)
-}
-
func legacyMarshalAppend(b []byte, m protoreflect.Message, opts piface.MarshalOptions) ([]byte, error) {
v := m.(unwrapper).protoUnwrap()
marshaler, ok := v.(legacyMarshaler)
diff --git a/proto/encode.go b/proto/encode.go
index 702190f..0801ce4 100644
--- a/proto/encode.go
+++ b/proto/encode.go
@@ -100,13 +100,15 @@
func (o MarshalOptions) marshalMessage(b []byte, m protoreflect.Message) ([]byte, error) {
if methods := protoMethods(m); methods != nil && methods.MarshalAppend != nil &&
!(o.Deterministic && methods.Flags&protoiface.SupportMarshalDeterministic == 0) {
- sz := methods.Size(m, protoiface.MarshalOptions(o))
- if cap(b) < len(b)+sz {
- x := make([]byte, len(b), growcap(cap(b), len(b)+sz))
- copy(x, b)
- b = x
+ if methods.Size != nil {
+ sz := methods.Size(m, protoiface.MarshalOptions(o))
+ if cap(b) < len(b)+sz {
+ x := make([]byte, len(b), growcap(cap(b), len(b)+sz))
+ copy(x, b)
+ b = x
+ }
+ o.UseCachedSize = true
}
- o.UseCachedSize = true
return methods.MarshalAppend(b, m, protoiface.MarshalOptions(o))
}
return o.marshalMessageSlow(b, m)
diff --git a/proto/size.go b/proto/size.go
index 9947580..beaa925 100644
--- a/proto/size.go
+++ b/proto/size.go
@@ -22,9 +22,16 @@
}
func sizeMessage(m protoreflect.Message) (size int) {
- if methods := protoMethods(m); methods != nil && methods.Size != nil {
+ methods := protoMethods(m)
+ if methods != nil && methods.Size != nil {
return methods.Size(m, protoiface.MarshalOptions{})
}
+ if methods != nil && methods.MarshalAppend != nil {
+ // This is not efficient, but we don't have any choice.
+ // This case is mainly used for legacy types with a Marshal method.
+ b, _ := methods.MarshalAppend(nil, m, protoiface.MarshalOptions{})
+ return len(b)
+ }
return sizeMessageSlow(m)
}
diff --git a/runtime/protoiface/methods.go b/runtime/protoiface/methods.go
index e0adf9d..b7ba129 100644
--- a/runtime/protoiface/methods.go
+++ b/runtime/protoiface/methods.go
@@ -34,7 +34,7 @@
Size func(m protoreflect.Message, opts MarshalOptions) int
// MarshalAppend appends the wire-format encoding of m to b, returning the result.
- // Size must be provided if a custom MarshalAppend is provided.
+ // Size should be provided if a custom MarshalAppend is provided.
// It must not perform required field checks.
MarshalAppend func(b []byte, m protoreflect.Message, opts MarshalOptions) ([]byte, error)