all: funnel similar functionality through a single function
Some companies (e.g., Google) run a profiling service where they may
choose to special-case certain symbols in a binary to classify
commonly used libraries like protobufs.
This CL funnels similar functionality through a single function
so that they can be more easily identified. This is by no means a
firm statement that these identifiers will never change names,
but at least the code documents warnings to avoid changing the
name of certain identifiers.
This CL provides the following semi-stable symbol names:
"google.golang.org/protobuf/proto".MarshalOptions.size
"google.golang.org/protobuf/proto".MarshalOptions.marshal
"google.golang.org/protobuf/proto".UnmarshalOptions.unmarshal
"google.golang.org/protobuf/encoding/prototext".MarshalOptions.marshal
"google.golang.org/protobuf/encoding/prototext".UnmarshalOptions.unmarshal
"google.golang.org/protobuf/encoding/protojson".MarshalOptions.marshal
"google.golang.org/protobuf/encoding/protojson".UnmarshalOptions.unmarshal
Merge and Clone are not part of the above set since there is a
possibility that MergeOptions will be added in the future.
We use an unexported method so that we have the freedom to change the
method however we want since profilers do not care about that.
Change-Id: Ia79af260d00125f48139420e1e18a86482bd1829
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/234079
Reviewed-by: Damien Neil <dneil@google.com>
diff --git a/encoding/protojson/decode.go b/encoding/protojson/decode.go
index 71c755c..2a60775 100644
--- a/encoding/protojson/decode.go
+++ b/encoding/protojson/decode.go
@@ -52,6 +52,13 @@
// setting the fields. If it returns an error, the given message may be
// partially set.
func (o UnmarshalOptions) Unmarshal(b []byte, m proto.Message) error {
+ return o.unmarshal(b, m)
+}
+
+// unmarshal is a centralized function that all unmarshal operations go through.
+// For profiling purposes, avoid changing the name of this function or
+// introducing other code paths for unmarshal that do not go through this.
+func (o UnmarshalOptions) unmarshal(b []byte, m proto.Message) error {
proto.Reset(m)
if o.Resolver == nil {
diff --git a/encoding/protojson/encode.go b/encoding/protojson/encode.go
index e545feb..73b750a 100644
--- a/encoding/protojson/encode.go
+++ b/encoding/protojson/encode.go
@@ -104,6 +104,13 @@
// MarshalOptions. Do not depend on the output being stable. It may change over
// time across different versions of the program.
func (o MarshalOptions) Marshal(m proto.Message) ([]byte, error) {
+ return o.marshal(m)
+}
+
+// marshal is a centralized function that all marshal operations go through.
+// For profiling purposes, avoid changing the name of this function or
+// introducing other code paths for marshal that do not go through this.
+func (o MarshalOptions) marshal(m proto.Message) ([]byte, error) {
if o.Multiline && o.Indent == "" {
o.Indent = defaultIndent
}
diff --git a/encoding/prototext/decode.go b/encoding/prototext/decode.go
index 77dbe1b..c2f8f28 100644
--- a/encoding/prototext/decode.go
+++ b/encoding/prototext/decode.go
@@ -54,6 +54,13 @@
// Unmarshal reads the given []byte and populates the given proto.Message using options in
// UnmarshalOptions object.
func (o UnmarshalOptions) Unmarshal(b []byte, m proto.Message) error {
+ return o.unmarshal(b, m)
+}
+
+// unmarshal is a centralized function that all unmarshal operations go through.
+// For profiling purposes, avoid changing the name of this function or
+// introducing other code paths for unmarshal that do not go through this.
+func (o UnmarshalOptions) unmarshal(b []byte, m proto.Message) error {
proto.Reset(m)
if o.Resolver == nil {
diff --git a/encoding/prototext/encode.go b/encoding/prototext/encode.go
index dece229..41e5c77 100644
--- a/encoding/prototext/encode.go
+++ b/encoding/prototext/encode.go
@@ -102,6 +102,13 @@
// MarshalOptions object. Do not depend on the output being stable. It may
// change over time across different versions of the program.
func (o MarshalOptions) Marshal(m proto.Message) ([]byte, error) {
+ return o.marshal(m)
+}
+
+// marshal is a centralized function that all marshal operations go through.
+// For profiling purposes, avoid changing the name of this function or
+// introducing other code paths for marshal that do not go through this.
+func (o MarshalOptions) marshal(m proto.Message) ([]byte, error) {
var delims = [2]byte{'{', '}'}
if o.Multiline && o.Indent == "" {
diff --git a/internal/cmd/generate-types/proto.go b/internal/cmd/generate-types/proto.go
index 8ea491b..ded7720 100644
--- a/internal/cmd/generate-types/proto.go
+++ b/internal/cmd/generate-types/proto.go
@@ -413,18 +413,18 @@
}
var protoSizeTemplate = template.Must(template.New("").Parse(`
-func sizeSingular(num protowire.Number, kind protoreflect.Kind, v protoreflect.Value) int {
+func (o MarshalOptions) sizeSingular(num protowire.Number, kind protoreflect.Kind, v protoreflect.Value) int {
switch kind {
{{- range .}}
case {{.Expr}}:
{{if (eq .Name "Message") -}}
- return protowire.SizeBytes(sizeMessage(v.Message()))
+ return protowire.SizeBytes(o.size(v.Message()))
{{- else if or (eq .WireType "Fixed32") (eq .WireType "Fixed64") -}}
return protowire.Size{{.WireType}}()
{{- else if (eq .WireType "Bytes") -}}
return protowire.Size{{.WireType}}(len({{.FromValue}}))
{{- else if (eq .WireType "Group") -}}
- return protowire.Size{{.WireType}}(num, sizeMessage(v.Message()))
+ return protowire.Size{{.WireType}}(num, o.size(v.Message()))
{{- else -}}
return protowire.Size{{.WireType}}({{.FromValue}})
{{- end}}
diff --git a/proto/decode.go b/proto/decode.go
index 1282147..4974b16 100644
--- a/proto/decode.go
+++ b/proto/decode.go
@@ -63,12 +63,15 @@
return o.unmarshal(in.Buf, in.Message)
}
+// unmarshal is a centralized function that all unmarshal operations go through.
+// For profiling purposes, avoid changing the name of this function or
+// introducing other code paths for unmarshal that do not go through this.
func (o UnmarshalOptions) unmarshal(b []byte, m protoreflect.Message) (out protoiface.UnmarshalOutput, err error) {
if o.Resolver == nil {
o.Resolver = protoregistry.GlobalTypes
}
if !o.Merge {
- Reset(m.Interface()) // TODO
+ Reset(m.Interface())
}
allowPartial := o.AllowPartial
o.Merge = true
@@ -105,7 +108,7 @@
func (o UnmarshalOptions) unmarshalMessageSlow(b []byte, m protoreflect.Message) error {
md := m.Descriptor()
if messageset.IsMessageSet(md) {
- return unmarshalMessageSet(b, m, o)
+ return o.unmarshalMessageSet(b, m)
}
fields := md.Fields()
for len(b) > 0 {
diff --git a/proto/encode.go b/proto/encode.go
index 456bfda..7b47a11 100644
--- a/proto/encode.go
+++ b/proto/encode.go
@@ -134,6 +134,9 @@
return o.marshal(in.Buf, in.Message)
}
+// marshal is a centralized function that all marshal operations go through.
+// For profiling purposes, avoid changing the name of this function or
+// introducing other code paths for marshal that do not go through this.
func (o MarshalOptions) marshal(b []byte, m protoreflect.Message) (out protoiface.MarshalOutput, err error) {
allowPartial := o.AllowPartial
o.AllowPartial = true
@@ -206,7 +209,7 @@
func (o MarshalOptions) marshalMessageSlow(b []byte, m protoreflect.Message) ([]byte, error) {
if messageset.IsMessageSet(m.Descriptor()) {
- return marshalMessageSet(b, m, o)
+ return o.marshalMessageSet(b, m)
}
// There are many choices for what order we visit fields in. The default one here
// is chosen for reasonable efficiency and simplicity given the protoreflect API.
diff --git a/proto/messageset.go b/proto/messageset.go
index b6b3de5..1d692c3 100644
--- a/proto/messageset.go
+++ b/proto/messageset.go
@@ -13,24 +13,24 @@
"google.golang.org/protobuf/reflect/protoregistry"
)
-func sizeMessageSet(m protoreflect.Message) (size int) {
+func (o MarshalOptions) sizeMessageSet(m protoreflect.Message) (size int) {
m.Range(func(fd protoreflect.FieldDescriptor, v protoreflect.Value) bool {
size += messageset.SizeField(fd.Number())
size += protowire.SizeTag(messageset.FieldMessage)
- size += protowire.SizeBytes(sizeMessage(v.Message()))
+ size += protowire.SizeBytes(o.size(v.Message()))
return true
})
size += messageset.SizeUnknown(m.GetUnknown())
return size
}
-func marshalMessageSet(b []byte, m protoreflect.Message, o MarshalOptions) ([]byte, error) {
+func (o MarshalOptions) marshalMessageSet(b []byte, m protoreflect.Message) ([]byte, error) {
if !flags.ProtoLegacy {
return b, errors.New("no support for message_set_wire_format")
}
var err error
o.rangeFields(m, func(fd protoreflect.FieldDescriptor, v protoreflect.Value) bool {
- b, err = marshalMessageSetField(b, fd, v, o)
+ b, err = o.marshalMessageSetField(b, fd, v)
return err == nil
})
if err != nil {
@@ -39,7 +39,7 @@
return messageset.AppendUnknown(b, m.GetUnknown())
}
-func marshalMessageSetField(b []byte, fd protoreflect.FieldDescriptor, value protoreflect.Value, o MarshalOptions) ([]byte, error) {
+func (o MarshalOptions) marshalMessageSetField(b []byte, fd protoreflect.FieldDescriptor, value protoreflect.Value) ([]byte, error) {
b = messageset.AppendFieldStart(b, fd.Number())
b = protowire.AppendTag(b, messageset.FieldMessage, protowire.BytesType)
b = protowire.AppendVarint(b, uint64(o.Size(value.Message().Interface())))
@@ -51,12 +51,12 @@
return b, nil
}
-func unmarshalMessageSet(b []byte, m protoreflect.Message, o UnmarshalOptions) error {
+func (o UnmarshalOptions) unmarshalMessageSet(b []byte, m protoreflect.Message) error {
if !flags.ProtoLegacy {
return errors.New("no support for message_set_wire_format")
}
return messageset.Unmarshal(b, false, func(num protowire.Number, v []byte) error {
- err := unmarshalMessageSetField(m, num, v, o)
+ err := o.unmarshalMessageSetField(m, num, v)
if err == errUnknown {
unknown := m.GetUnknown()
unknown = protowire.AppendTag(unknown, num, protowire.BytesType)
@@ -68,7 +68,7 @@
})
}
-func unmarshalMessageSetField(m protoreflect.Message, num protowire.Number, v []byte, o UnmarshalOptions) error {
+func (o UnmarshalOptions) unmarshalMessageSetField(m protoreflect.Message, num protowire.Number, v []byte) error {
md := m.Descriptor()
if !md.ExtensionRanges().Has(num) {
return errUnknown
diff --git a/proto/size.go b/proto/size.go
index 11ba841..554b9c6 100644
--- a/proto/size.go
+++ b/proto/size.go
@@ -23,10 +23,13 @@
return 0
}
- return sizeMessage(m.ProtoReflect())
+ return o.size(m.ProtoReflect())
}
-func sizeMessage(m protoreflect.Message) (size int) {
+// size is a centralized function that all size operations go through.
+// For profiling purposes, avoid changing the name of this function or
+// introducing other code paths for size that do not go through this.
+func (o MarshalOptions) size(m protoreflect.Message) (size int) {
methods := protoMethods(m)
if methods != nil && methods.Size != nil {
out := methods.Size(protoiface.SizeInput{
@@ -42,52 +45,52 @@
})
return len(out.Buf)
}
- return sizeMessageSlow(m)
+ return o.sizeMessageSlow(m)
}
-func sizeMessageSlow(m protoreflect.Message) (size int) {
+func (o MarshalOptions) sizeMessageSlow(m protoreflect.Message) (size int) {
if messageset.IsMessageSet(m.Descriptor()) {
- return sizeMessageSet(m)
+ return o.sizeMessageSet(m)
}
m.Range(func(fd protoreflect.FieldDescriptor, v protoreflect.Value) bool {
- size += sizeField(fd, v)
+ size += o.sizeField(fd, v)
return true
})
size += len(m.GetUnknown())
return size
}
-func sizeField(fd protoreflect.FieldDescriptor, value protoreflect.Value) (size int) {
+func (o MarshalOptions) sizeField(fd protoreflect.FieldDescriptor, value protoreflect.Value) (size int) {
num := fd.Number()
switch {
case fd.IsList():
- return sizeList(num, fd, value.List())
+ return o.sizeList(num, fd, value.List())
case fd.IsMap():
- return sizeMap(num, fd, value.Map())
+ return o.sizeMap(num, fd, value.Map())
default:
- return protowire.SizeTag(num) + sizeSingular(num, fd.Kind(), value)
+ return protowire.SizeTag(num) + o.sizeSingular(num, fd.Kind(), value)
}
}
-func sizeList(num protowire.Number, fd protoreflect.FieldDescriptor, list protoreflect.List) (size int) {
+func (o MarshalOptions) sizeList(num protowire.Number, fd protoreflect.FieldDescriptor, list protoreflect.List) (size int) {
if fd.IsPacked() && list.Len() > 0 {
content := 0
for i, llen := 0, list.Len(); i < llen; i++ {
- content += sizeSingular(num, fd.Kind(), list.Get(i))
+ content += o.sizeSingular(num, fd.Kind(), list.Get(i))
}
return protowire.SizeTag(num) + protowire.SizeBytes(content)
}
for i, llen := 0, list.Len(); i < llen; i++ {
- size += protowire.SizeTag(num) + sizeSingular(num, fd.Kind(), list.Get(i))
+ size += protowire.SizeTag(num) + o.sizeSingular(num, fd.Kind(), list.Get(i))
}
return size
}
-func sizeMap(num protowire.Number, fd protoreflect.FieldDescriptor, mapv protoreflect.Map) (size int) {
+func (o MarshalOptions) sizeMap(num protowire.Number, fd protoreflect.FieldDescriptor, mapv protoreflect.Map) (size int) {
mapv.Range(func(key protoreflect.MapKey, value protoreflect.Value) bool {
size += protowire.SizeTag(num)
- size += protowire.SizeBytes(sizeField(fd.MapKey(), key.Value()) + sizeField(fd.MapValue(), value))
+ size += protowire.SizeBytes(o.sizeField(fd.MapKey(), key.Value()) + o.sizeField(fd.MapValue(), value))
return true
})
return size
diff --git a/proto/size_gen.go b/proto/size_gen.go
index 1118460..3cf61a8 100644
--- a/proto/size_gen.go
+++ b/proto/size_gen.go
@@ -11,7 +11,7 @@
"google.golang.org/protobuf/reflect/protoreflect"
)
-func sizeSingular(num protowire.Number, kind protoreflect.Kind, v protoreflect.Value) int {
+func (o MarshalOptions) sizeSingular(num protowire.Number, kind protoreflect.Kind, v protoreflect.Value) int {
switch kind {
case protoreflect.BoolKind:
return protowire.SizeVarint(protowire.EncodeBool(v.Bool()))
@@ -46,9 +46,9 @@
case protoreflect.BytesKind:
return protowire.SizeBytes(len(v.Bytes()))
case protoreflect.MessageKind:
- return protowire.SizeBytes(sizeMessage(v.Message()))
+ return protowire.SizeBytes(o.size(v.Message()))
case protoreflect.GroupKind:
- return protowire.SizeGroup(num, sizeMessage(v.Message()))
+ return protowire.SizeGroup(num, o.size(v.Message()))
default:
return 0
}
