From 61781dd92f1fcd07cf9205a5f6f6ab2d6cd642a4 Mon Sep 17 00:00:00 2001 From: Damien Neil Date: Tue, 21 Jan 2020 13:29:51 -0800 Subject: [PATCH] all: abstract fast-path marshal and unmarshal inputs and outputs We may want to make changes to the inputs and outputs of the fast-path functions in the future. For example, we likely want to add the ability for the fast-path unmarshal to report back whether the unmarshaled message is known to be initialized. Change the signatures of these functions to take in and return struct types which can be extended with whatever fields we want in the future. Change-Id: Idead360785df730283a4630ea405265b72482e62 Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/215719 Reviewed-by: Joe Tsai --- internal/impl/codec_message.go | 4 ++-- internal/impl/decode.go | 6 +++--- internal/impl/encode.go | 7 +++--- internal/impl/legacy_message.go | 24 +++++++++++---------- proto/decode.go | 6 +++++- proto/encode.go | 8 +++++-- proto/size.go | 6 +++--- reflect/protoreflect/methods.go | 24 +++++++++++++++++---- runtime/protoiface/methods.go | 38 +++++++++++++++++++++++++++++---- 9 files changed, 90 insertions(+), 33 deletions(-) diff --git a/internal/impl/codec_message.go b/internal/impl/codec_message.go index e487a26e..24a87a04 100644 --- a/internal/impl/codec_message.go +++ b/internal/impl/codec_message.go @@ -134,9 +134,9 @@ func (mi *MessageInfo) makeCoderMethods(t reflect.Type, si structInfo) { } mi.needsInitCheck = needsInitCheck(mi.Desc) - if mi.methods.MarshalAppend == nil && mi.methods.Size == nil { + if mi.methods.Marshal == nil && mi.methods.Size == nil { mi.methods.Flags |= piface.SupportMarshalDeterministic - mi.methods.MarshalAppend = mi.marshalAppend + mi.methods.Marshal = mi.marshal mi.methods.Size = mi.size } if mi.methods.Unmarshal == nil { diff --git a/internal/impl/decode.go b/internal/impl/decode.go index 50b0cf1c..4d3718f7 100644 --- a/internal/impl/decode.go +++ b/internal/impl/decode.go @@ -58,15 +58,15 @@ func (o unmarshalOptions) DiscardUnknown() bool { return o.flags func (o unmarshalOptions) Resolver() preg.ExtensionTypeResolver { return o.resolver } // unmarshal is protoreflect.Methods.Unmarshal. -func (mi *MessageInfo) unmarshal(b []byte, m pref.Message, opts piface.UnmarshalOptions) error { +func (mi *MessageInfo) unmarshal(m pref.Message, in piface.UnmarshalInput) (piface.UnmarshalOutput, error) { var p pointer if ms, ok := m.(*messageState); ok { p = ms.pointer() } else { p = m.(*messageReflectWrapper).pointer() } - _, err := mi.unmarshalPointer(b, p, 0, newUnmarshalOptions(opts)) - return err + _, err := mi.unmarshalPointer(in.Buf, p, 0, newUnmarshalOptions(in.Options)) + return piface.UnmarshalOutput{}, err } // errUnknown is returned during unmarshaling to indicate a parse error that diff --git a/internal/impl/encode.go b/internal/impl/encode.go index c793021f..306677c1 100644 --- a/internal/impl/encode.go +++ b/internal/impl/encode.go @@ -101,15 +101,16 @@ func (mi *MessageInfo) sizePointerSlow(p pointer, opts marshalOptions) (size int return size } -// marshalAppend is protoreflect.Methods.MarshalAppend. -func (mi *MessageInfo) marshalAppend(b []byte, m pref.Message, opts piface.MarshalOptions) ([]byte, error) { +// marshal is protoreflect.Methods.Marshal. +func (mi *MessageInfo) marshal(m pref.Message, in piface.MarshalInput) (piface.MarshalOutput, error) { var p pointer if ms, ok := m.(*messageState); ok { p = ms.pointer() } else { p = m.(*messageReflectWrapper).pointer() } - return mi.marshalAppendPointer(b, p, newMarshalOptions(opts)) + b, err := mi.marshalAppendPointer(in.Buf, p, newMarshalOptions(in.Options)) + return piface.MarshalOutput{Buf: b}, err } func (mi *MessageInfo) marshalAppendPointer(b []byte, p pointer, opts marshalOptions) ([]byte, error) { diff --git a/internal/impl/legacy_message.go b/internal/impl/legacy_message.go index 80220a2e..6f08912d 100644 --- a/internal/impl/legacy_message.go +++ b/internal/impl/legacy_message.go @@ -50,7 +50,7 @@ func legacyLoadMessageInfo(t reflect.Type, name pref.FullName) *MessageInfo { v := reflect.Zero(t).Interface() if _, ok := v.(legacyMarshaler); ok { - mi.methods.MarshalAppend = legacyMarshalAppend + mi.methods.Marshal = legacyMarshal // We have no way to tell whether the type's Marshal method // supports deterministic serialization or not, but this @@ -363,8 +363,8 @@ type legacyUnmarshaler interface { } var legacyProtoMethods = &piface.Methods{ - MarshalAppend: legacyMarshalAppend, - Unmarshal: legacyUnmarshal, + Marshal: legacyMarshal, + Unmarshal: legacyUnmarshal, // We have no way to tell whether the type's Marshal method // supports deterministic serialization or not, but this @@ -373,26 +373,28 @@ var legacyProtoMethods = &piface.Methods{ Flags: piface.SupportMarshalDeterministic, } -func legacyMarshalAppend(b []byte, m protoreflect.Message, opts piface.MarshalOptions) ([]byte, error) { +func legacyMarshal(m protoreflect.Message, in piface.MarshalInput) (piface.MarshalOutput, error) { v := m.(unwrapper).protoUnwrap() marshaler, ok := v.(legacyMarshaler) if !ok { - return nil, errors.New("%T does not implement Marshal", v) + return piface.MarshalOutput{}, errors.New("%T does not implement Marshal", v) } out, err := marshaler.Marshal() - if b != nil { - out = append(b, out...) + if in.Buf != nil { + out = append(in.Buf, out...) } - return out, err + return piface.MarshalOutput{ + Buf: out, + }, err } -func legacyUnmarshal(b []byte, m protoreflect.Message, opts piface.UnmarshalOptions) error { +func legacyUnmarshal(m protoreflect.Message, in piface.UnmarshalInput) (piface.UnmarshalOutput, error) { v := m.(unwrapper).protoUnwrap() unmarshaler, ok := v.(legacyUnmarshaler) if !ok { - return errors.New("%T does not implement Marshal", v) + return piface.UnmarshalOutput{}, errors.New("%T does not implement Marshal", v) } - return unmarshaler.Unmarshal(b) + return piface.UnmarshalOutput{}, unmarshaler.Unmarshal(in.Buf) } // aberrantMessageType implements MessageType for all types other than pointer-to-struct. diff --git a/proto/decode.go b/proto/decode.go index cea434d3..f5b58088 100644 --- a/proto/decode.go +++ b/proto/decode.go @@ -72,7 +72,11 @@ func (o UnmarshalOptions) Unmarshal(b []byte, m Message) error { func (o UnmarshalOptions) unmarshalMessage(b []byte, m protoreflect.Message) error { if methods := protoMethods(m); methods != nil && methods.Unmarshal != nil && !(o.DiscardUnknown && methods.Flags&protoiface.SupportUnmarshalDiscardUnknown == 0) { - return methods.Unmarshal(b, m, protoiface.UnmarshalOptions(o)) + _, err := methods.Unmarshal(m, protoiface.UnmarshalInput{ + Buf: b, + Options: protoiface.UnmarshalOptions(o), + }) + return err } return o.unmarshalMessageSlow(b, m) } diff --git a/proto/encode.go b/proto/encode.go index 0801ce48..18ffe6ef 100644 --- a/proto/encode.go +++ b/proto/encode.go @@ -98,7 +98,7 @@ func (o MarshalOptions) MarshalAppend(b []byte, m Message) ([]byte, error) { } func (o MarshalOptions) marshalMessage(b []byte, m protoreflect.Message) ([]byte, error) { - if methods := protoMethods(m); methods != nil && methods.MarshalAppend != nil && + if methods := protoMethods(m); methods != nil && methods.Marshal != nil && !(o.Deterministic && methods.Flags&protoiface.SupportMarshalDeterministic == 0) { if methods.Size != nil { sz := methods.Size(m, protoiface.MarshalOptions(o)) @@ -109,7 +109,11 @@ func (o MarshalOptions) marshalMessage(b []byte, m protoreflect.Message) ([]byte } o.UseCachedSize = true } - return methods.MarshalAppend(b, m, protoiface.MarshalOptions(o)) + out, err := methods.Marshal(m, protoiface.MarshalInput{ + Buf: b, + Options: protoiface.MarshalOptions(o), + }) + return out.Buf, err } return o.marshalMessageSlow(b, m) } diff --git a/proto/size.go b/proto/size.go index beaa9251..619104cf 100644 --- a/proto/size.go +++ b/proto/size.go @@ -26,11 +26,11 @@ func sizeMessage(m protoreflect.Message) (size int) { if methods != nil && methods.Size != nil { return methods.Size(m, protoiface.MarshalOptions{}) } - if methods != nil && methods.MarshalAppend != nil { + if methods != nil && methods.Marshal != 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) + out, _ := methods.Marshal(m, protoiface.MarshalInput{}) + return len(out.Buf) } return sizeMessageSlow(m) } diff --git a/reflect/protoreflect/methods.go b/reflect/protoreflect/methods.go index 171ed6f9..89207c02 100644 --- a/reflect/protoreflect/methods.go +++ b/reflect/protoreflect/methods.go @@ -19,18 +19,34 @@ type ( pragma.NoUnkeyedLiterals Flags supportFlags Size func(m Message, opts marshalOptions) int - MarshalAppend func(b []byte, m Message, opts marshalOptions) ([]byte, error) - Unmarshal func(b []byte, m Message, opts unmarshalOptions) error + Marshal func(m Message, in marshalInput) (marshalOutput, error) + Unmarshal func(m Message, in unmarshalInput) (unmarshalOutput, error) IsInitialized func(m Message) error } - supportFlags = uint64 + supportFlags = uint64 + marshalInput = struct { + pragma.NoUnkeyedLiterals + Buf []byte + Options marshalOptions + } + marshalOutput = struct { + pragma.NoUnkeyedLiterals + Buf []byte + } marshalOptions = struct { pragma.NoUnkeyedLiterals AllowPartial bool Deterministic bool UseCachedSize bool } - + unmarshalInput = struct { + pragma.NoUnkeyedLiterals + Buf []byte + Options unmarshalOptions + } + unmarshalOutput = struct { + pragma.NoUnkeyedLiterals + } unmarshalOptions = struct { pragma.NoUnkeyedLiterals Merge bool diff --git a/runtime/protoiface/methods.go b/runtime/protoiface/methods.go index 4b9bb92e..ed8f1537 100644 --- a/runtime/protoiface/methods.go +++ b/runtime/protoiface/methods.go @@ -25,14 +25,14 @@ type Methods = struct { // MarshalAppend must be provided if a custom Size is provided. Size func(m protoreflect.Message, opts MarshalOptions) int - // MarshalAppend appends the wire-format encoding of m to b, returning the result. + // Marshal writes the wire-format encoding of m to the provided buffer. // 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) + Marshal func(m protoreflect.Message, in MarshalInput) (MarshalOutput, error) - // Unmarshal parses the wire-format message in b and merges the result in m. + // Unmarshal parses the wire-format encoding of a message and merges the result to m. // It must not reset m or perform required field checks. - Unmarshal func(b []byte, m protoreflect.Message, opts UnmarshalOptions) error + Unmarshal func(m protoreflect.Message, in UnmarshalInput) (UnmarshalOutput, error) // IsInitialized returns an error if any required fields in m are not set. IsInitialized func(m protoreflect.Message) error @@ -48,6 +48,21 @@ const ( SupportUnmarshalDiscardUnknown ) +// MarshalInput is input to the marshaler. +type MarshalInput = struct { + pragma.NoUnkeyedLiterals + + Buf []byte // output is appended to this buffer + Options MarshalOptions +} + +// MarshalOutput is output from the marshaler. +type MarshalOutput = struct { + pragma.NoUnkeyedLiterals + + Buf []byte // contains marshaled message +} + // MarshalOptions configure the marshaler. // // This type is identical to the one in package proto. @@ -59,6 +74,21 @@ type MarshalOptions = struct { UseCachedSize bool } +// UnmarshalInput is input to the unmarshaler. +type UnmarshalInput = struct { + pragma.NoUnkeyedLiterals + + Buf []byte // input buffer + Options UnmarshalOptions +} + +// UnmarshalOutput is output from the unmarshaler. +type UnmarshalOutput = struct { + pragma.NoUnkeyedLiterals + + // Contents available for future expansion. +} + // UnmarshalOptions configures the unmarshaler. // // This type is identical to the one in package proto.