Commit Graph

23 Commits

Author SHA1 Message Date
Damien Neil
b0d217f664 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>
2020-01-06 22:47:37 +00:00
Damien Neil
01c0e8d680 proto, internal/impl: make wire output more consistent with v1
The v1 wire marshaler sorts fields as follows:
  - All extensions, sorted by field number.
  - All non-oneof fields, sorted by field number.
  - All oneof fields, in indeterminate order.

We already make some steps toward supporting this ordering: The
fast path encoder places extensions in sorted order at the start
of the message.

This commit moves oneof fields to the end of the message, makes the
reflection-based encoder use this ordering when deterministic marshaling
is enabled, and adds a test to catch unintentional changes to the
ordering.

Users SHOULD NOT depend on stability of the marshal output. It is
subject to change over time. Without deterministic marshaling enabled,
it is subject to change over calls to Marshal.

Change-Id: I6cfd89090d790a3bb50785f32b94d2781d7d08db
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/206800
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-11-12 20:59:03 +00:00
Damien Neil
1e5516a4c2 proto: improve slice growth in MarshalAppend
When allocating more space for the destination message in MarshalAppend,
use the same slice growth algorithm as the Go runtime's append rather
than allocating precisely the desired space.

Change-Id: If6033f6f7abdca473bc5188c4d3938ce57d3bdd2
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/197758
Reviewed-by: Joe Tsai <joetsai@google.com>
2019-09-28 01:00:09 +00:00
Damien Neil
37ef691e6b internal/impl: call Marshal/Unmarshal methods on legacy types
Call the Marshal or Unmarshal method on legacy messages implementing
protoV1.Marshaler or protoV2.Unmarshaler.

We do this in the impl package by creating an appropriate function in
the protoiface.Methods struct for legacy messages.

In proto.MarshalAppend, return the bytes provided by the fast-path
marshal function even when the returned error is non-nil.

Fixes golang/protobuf#955

Change-Id: I36924af9ff959a946c43f2295ef3202216e81b32
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/197357
Reviewed-by: Joe Tsai <joetsai@google.com>
2019-09-26 20:54:39 +00:00
Damien Neil
302cb325fb proto: support message_set_wire_format
MessageSets are a deprecated proto1 feature, long since superseded by
extensions. Add disabled-by-default support behind flags.Proto1Legacy.

Change-Id: I7d3ace07f3b0efd59673034f3dc633b908345a88
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/185538
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-07-15 19:32:30 +00:00
Joe Tsai
f8b855d768 runtime/protoiface: API adjustments
The following adjustments were made:
* The pragma.NoUnkeyedLiterals is moved to be the first field.
This is done to keep the options struct smaller. Even if the last
field is zero-length, Go GC implementation details forces the struct
to be padded at the end.
* Methods are documented as always treating AllowPartial as true.
* Added a support flag for UnmarshalOptions.DiscardUnknown.

Change-Id: I1f75d226542ab2bb0123d9cea143c7060df226d8
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/185998
Reviewed-by: Damien Neil <dneil@google.com>
2019-07-12 23:12:26 +00:00
Joe Tsai
0f81b38d61 runtime/protoiface: move and rename XXX_Methods
This CL moves and renames the protoreflect.ProtoMessage.XXX_Methods
to protoreflect.Message.ProtoMethods.

Since one needs to obtain a protoreflect.Message now to get at
the fast-path methods, we modify the method signatures to take
in a protoreflect.Message instead of protoreflect.ProtoMessage.
Doing so also avoids the wrapper hack that was formerly done on
impl.messageReflectWrapper.

After this change the new protoc-gen-go no longer generates
any XXX fields or methods. All internal fields and methods are truly
hidden from the end-user.

name                                     old time/op    new time/op    delta
Wire/Unmarshal/google_message1_proto2-4    1.50µs ±10%    1.50µs ± 2%    ~     (p=0.483 n=10+9)
Wire/Unmarshal/google_message1_proto3-4    1.06µs ± 6%    1.06µs ± 4%    ~     (p=0.814 n=9+9)
Wire/Unmarshal/google_message2-4            734µs ±22%     689µs ±13%    ~     (p=0.133 n=10+9)
Wire/Marshal/google_message1_proto2-4       790ns ±46%     652ns ± 8%    ~     (p=0.590 n=10+9)
Wire/Marshal/google_message1_proto3-4       872ns ± 4%     857ns ± 3%    ~     (p=0.168 n=9+9)
Wire/Marshal/google_message2-4              232µs ±16%     221µs ± 3%  -4.75%  (p=0.014 n=9+9)
Wire/Size/google_message1_proto2-4          164ns ± 2%     167ns ± 4%  +1.87%  (p=0.046 n=9+10)
Wire/Size/google_message1_proto3-4          240ns ± 9%     229ns ± 1%  -4.81%  (p=0.000 n=9+8)
Wire/Size/google_message2-4                58.9µs ± 9%    59.6µs ± 2%  +1.23%  (p=0.040 n=9+9)

name                                     old alloc/op   new alloc/op   delta
Wire/Unmarshal/google_message1_proto2-4      912B ± 0%      912B ± 0%    ~     (all equal)
Wire/Unmarshal/google_message1_proto3-4      688B ± 0%      688B ± 0%    ~     (all equal)
Wire/Unmarshal/google_message2-4            470kB ± 0%     470kB ± 0%    ~     (p=0.215 n=10+10)
Wire/Marshal/google_message1_proto2-4        240B ± 0%      240B ± 0%    ~     (all equal)
Wire/Marshal/google_message1_proto3-4        224B ± 0%      224B ± 0%    ~     (all equal)
Wire/Marshal/google_message2-4             90.1kB ± 0%    90.1kB ± 0%    ~     (all equal)
Wire/Size/google_message1_proto2-4          0.00B          0.00B         ~     (all equal)
Wire/Size/google_message1_proto3-4          0.00B          0.00B         ~     (all equal)
Wire/Size/google_message2-4                 0.00B          0.00B         ~     (all equal)

name                                     old allocs/op  new allocs/op  delta
Wire/Unmarshal/google_message1_proto2-4      24.0 ± 0%      24.0 ± 0%    ~     (all equal)
Wire/Unmarshal/google_message1_proto3-4      6.00 ± 0%      6.00 ± 0%    ~     (all equal)
Wire/Unmarshal/google_message2-4            8.49k ± 0%     8.49k ± 0%    ~     (all equal)
Wire/Marshal/google_message1_proto2-4        1.00 ± 0%      1.00 ± 0%    ~     (all equal)
Wire/Marshal/google_message1_proto3-4        1.00 ± 0%      1.00 ± 0%    ~     (all equal)
Wire/Marshal/google_message2-4               1.00 ± 0%      1.00 ± 0%    ~     (all equal)
Wire/Size/google_message1_proto2-4           0.00           0.00         ~     (all equal)
Wire/Size/google_message1_proto3-4           0.00           0.00         ~     (all equal)
Wire/Size/google_message2-4                  0.00           0.00         ~     (all equal)

Change-Id: Ibf3263ad0f293326695c22020a92a6b938ef4f65
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/185697
Reviewed-by: Damien Neil <dneil@google.com>
2019-07-12 19:31:58 +00:00
Damien Neil
a9940822d4 all: remove protoreflect.Message.Len
Len looks like it should be O(1), but the need to check for
non-zero-length repeated fields makes it at minimum O(n) where n is
the number of repeated fields. In practice, it's O(n) where n is the
number of fields altogether.

The Len function is not especially useful, easily duplicated with Range
and a counter, and can be surprisingly inefficient. Drop it.

Change-Id: I24b27433217e131e842bd18dd58475bcdf62ef97
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/183678
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-06-25 21:59:46 +00:00
Damien Neil
8c86fc5e7d all: remove non-fatal UTF-8 validation errors (and non-fatal in general)
Immediately abort (un)marshal operations when encountering invalid UTF-8
data in proto3 strings. No other proto implementation supports non-UTF-8
data in proto3 strings (and many reject it in proto2 strings as well).
Producing invalid output is an interoperability threat (other
implementations won't be able to read it).

The case where existing string data is found to contain non-UTF8 data is
better handled by changing the field to the `bytes` type, which (aside
from UTF-8 validation) is wire-compatible with `string`.

Remove the errors.NonFatal type, since there are no remaining cases
where it is needed. "Non-fatal" errors which produce results and a
non-nil error are problematic because they compose poorly; the better
approach is to take an option like AllowPartial indicating which
conditions to check for.

Change-Id: I9d189ec6ffda7b5d96d094aa1b290af2e3f23736
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/183098
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-06-20 20:55:13 +00:00
Joe Tsai
378c1329de reflect/protoreflect: add alternative message reflection API
Added API:
	Message.Len
	Message.Range
	Message.Has
	Message.Clear
	Message.Get
	Message.Set
	Message.Mutable
	Message.NewMessage
	Message.WhichOneof
	Message.GetUnknown
	Message.SetUnknown

Deprecated API (to be removed in subsequent CL):
	Message.KnownFields
	Message.UnknownFields

The primary difference with the new API is that the top-level
Message methods are keyed by FieldDescriptor rather than FieldNumber
with the following semantics:
* For known fields, the FieldDescriptor must exactly match the
field descriptor known by the message.
* For extension fields, the FieldDescriptor must implement ExtensionType,
where ContainingMessage.FullName matches the message name, and
the field number is within the message's extension range.
When setting an extension field, it automatically stores
the extension type information.
* Extension fields are always considered nullable,
implying that repeated extension fields are nullable.
That is, you can distinguish between a unpopulated list and an empty list.
* Message.Get always returns a valid Value even if unpopulated.
The behavior is already well-defined for scalars, but for unpopulated
composite types, it now returns an empty read-only version of it.

Change-Id: Ia120630b4db221aeaaf743d0f64160e1a61a0f61
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/175458
Reviewed-by: Damien Neil <dneil@google.com>
2019-06-17 17:33:24 +00:00
Damien Neil
c37adefdac internal/impl: add fast-path marshal implementation
This is a port of the v1 table marshaler, with some substantial
cleanup and refactoring.

Benchstat results from the protobuf reference benchmark data comparing the
v1 package with v2, with AllowPartial:true set for the new package. This
is not an apples-to-apples comparison, since v1 doesn't have a way to
disable required field checks.  Required field checks in v2 package
currently go through reflection, which performs terribly; my initial
experimentation indicates that fast-path required field checks will
not add a large amount of cost; these results are incomplete but not
wholly inaccurate.

name                                           old time/op  new time/op  delta
/dataset.google_message3_1.pb/Marshal-12        219ms ± 1%   232ms ± 1%   +5.85%  (p=0.004 n=6+5)
/dataset.google_message2.pb/Marshal-12          261µs ± 3%   248µs ± 1%   -5.14%  (p=0.002 n=6+6)
/dataset.google_message1_proto2.pb/Marshal-12   681ns ± 2%   637ns ± 3%   -6.53%  (p=0.002 n=6+6)
/dataset.google_message1_proto3.pb/Marshal-12  1.10µs ± 8%  0.99µs ± 3%   -9.63%  (p=0.002 n=6+6)
/dataset.google_message3_3.pb/Marshal-12       44.2ms ± 3%  35.2ms ± 1%  -20.28%  (p=0.004 n=6+5)
/dataset.google_message4.pb/Marshal-12         91.4ms ± 2%  94.9ms ± 2%   +3.78%  (p=0.002 n=6+6)
/dataset.google_message3_2.pb/Marshal-12       78.7ms ± 6%  80.8ms ± 4%     ~     (p=0.310 n=6+6)
/dataset.google_message3_4.pb/Marshal-12       10.6ms ± 3%  10.6ms ± 8%     ~     (p=0.662 n=5+6)
/dataset.google_message3_5.pb/Marshal-12        675ms ± 4%   510ms ± 2%  -24.40%  (p=0.002 n=6+6)
/dataset.google_message3_1.pb/Marshal           219ms ± 1%   236ms ± 7%   +8.06%  (p=0.004 n=5+6)
/dataset.google_message2.pb/Marshal             257µs ± 1%   250µs ± 3%     ~     (p=0.052 n=5+6)
/dataset.google_message1_proto2.pb/Marshal      685ns ± 1%   628ns ± 1%   -8.41%  (p=0.008 n=5+5)
/dataset.google_message1_proto3.pb/Marshal     1.08µs ± 1%  0.98µs ± 2%   -9.31%  (p=0.004 n=5+6)
/dataset.google_message3_3.pb/Marshal          43.7ms ± 1%  35.1ms ± 1%  -19.76%  (p=0.002 n=6+6)
/dataset.google_message4.pb/Marshal            93.4ms ± 4%  94.9ms ± 2%     ~     (p=0.180 n=6+6)
/dataset.google_message3_2.pb/Marshal           105ms ± 2%    98ms ± 7%   -6.81%  (p=0.009 n=5+6)
/dataset.google_message3_4.pb/Marshal          16.3ms ± 6%  15.7ms ± 3%   -3.44%  (p=0.041 n=6+6)
/dataset.google_message3_5.pb/Marshal           676ms ± 4%   504ms ± 2%  -25.50%  (p=0.004 n=6+5)

Change-Id: I72cc4597117f4cf5d236ef505777d49dd4a5f75d
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/171020
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-05-16 22:13:43 +00:00
Damien Neil
e89e6244e0 all: change module to google.golang.org/protobuf
Temporarily remove go.mod, since we can't generate an accurate one until
the corresponding v1 change is submitted.

Change-Id: I1e1ad97f2b455e33f61ffaeb8676289795e47e72
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/177000
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-05-14 17:28:29 +00:00
Joe Tsai
ac31a352ca reflect/protoreflect: add helper methods to FieldDescriptor
Added API:
	FieldDescriptor.IsExtension
	FieldDescriptor.IsList
	FieldDescriptor.MapKey
	FieldDescriptor.MapValue
	FieldDescriptor.ContainingOneof
	FieldDescriptor.ContainingMessage

Deprecated API (to be removed in subsequent CL):
	FieldDescriptor.Oneof
	FieldDescriptor.Extendee

These methods help cleanup several common usage patterns.

Change-Id: I9a3ffabc2edb2173c536509b22f330f98bba7cf3
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/176977
Reviewed-by: Damien Neil <dneil@google.com>
2019-05-14 17:14:05 +00:00
Joe Tsai
0fc49f8225 reflect/protoreflect: add Descriptor specific methods
Added methods:
	Enum.Descriptor
	Message.Descriptor
	EnumType.Descriptor
	MessageType.Descriptor
	ExtensionType.Descriptor
	Message.New

All functionality is switched over to use those methods instead of
implicitly relying on the fact that {Enum,Message}Type implicitly
implement the associated descriptor interface.

This CL does not yet remove {Enum,Message}.Type or prevent
{Enum,Message,Extension}Type from implementating a descriptor.
That is a subsequent CL.

The Message.New method is also added to replace functionality
that will be lost when the Type methods are removed.

Change-Id: I7fefde1673bbd40bfdac489aca05cec9a6c98eb1
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/174918
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Herbie Ong <herbie@google.com>
2019-05-13 19:34:41 +00:00
Joe Tsai
d24bc72368 reflect/protoreflect: rename methods with Type suffix
The protobuf type system uses the word "descriptor" instead of "type".
We should avoid the "type" verbage when we aren't talking about Go types.
The old names are temporarily kept around for compatibility reasons.

Change-Id: Icc99c913528ead011f7a74aa8399d9c5ec6dc56e
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/172238
Reviewed-by: Herbie Ong <herbie@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
2019-04-20 06:35:24 +00:00
Damien Neil
bc310b58c6 proto: validate UTF-8 in proto3 strings
Change-Id: I6a495730c3f438e7b2c4ca86edade7d6f25aa47d
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/171700
Reviewed-by: Herbie Ong <herbie@google.com>
2019-04-12 17:43:10 +00:00
Damien Neil
03e748680c proto: replace CachedSize fast-path method with UseCachedSize option
Using an option instead of a separate method has several useful properties:

It makes it explicit whether the fast-path AppendMarshal is expected to use
cached sizes or not.

It properly plumbs the decision to use cached sizes through the call stack.
Consider the case where message A includes B includes C: If A and C support
cached sizes but B does not, we would like to use the size cache in all
messages which support it. Placing this decision in the options allows this
to work properly with no additional effort.

Placing this option in MarshalOptions permits users to request use of
existing cached sizes. This is a two-edged sword: There are places where
this ability can permit substantial efficiencies, but this is also an
exceedingly sharp-edged API. I believe that on balance the benefits
outweigh the risks, especially since the prerequisites for using
cached sizes are intuitively obvious. (You must have called Size, and
you must not have changed the message.)

This CL adds a Size method to MarshalOptions, rather than adding a SizeOptions
type. Future additions to MarshalOptions may affect the size of the encoded
output (e.g., an option to skip encoding unknown fields) and using the same
options for both Marshal and Size makes it easier to use a consistent
configuration for each.

Change-Id: I6adbb55b717dd03d39f067e1d0b7381945000976
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/171119
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-04-08 05:14:36 +00:00
Damien Neil
3016b73382 proto: fix MarshalAppend fast path
Was overwriting the output buffer rather than appending to it.

Change-Id: I6ffb72a440f464f4259cfebc42c1dc75b73fb5ae
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/171117
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-04-08 04:31:27 +00:00
Damien Neil
cc2b078f98 proto: enable/disable fast path with build tags
Remove the Reflection field from MarshalOptions and UnmarshalOptions.
Disable the fast path and use the reflection-based implementation when
the 'protoreflect' build tag is set.

Change-Id: Ic674e3af67501de27fb03ec2712fbed40eae7fef
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/170896
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-04-07 17:00:00 +00:00
Damien Neil
4686e239b6 proto: add IsInitialized
Move all checks for required fields into a proto.IsInitialized function.

Initial testing makes me confident that we can provide a fast-path
implementation of IsInitialized which will perform more than
acceptably.  (In the degenerate-but-common case where a message
transitively contains no required fields, this check can be nearly
zero cost.)

Unifying checks into a single function provides consistent behavior
between the wire, text, and json codecs.

Performing the check after decoding eliminates the wire decoder bug
where a split message is incorrectly seen as missing required fields.

Performing the check after decoding also provides consistent and
arguably more correct behavior when the target message was partially
prepopulated.

Change-Id: I9478b7bebb263af00c0d9f66a1f26e31ff553522
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/170787
Reviewed-by: Herbie Ong <herbie@google.com>
2019-04-05 22:21:46 +00:00
Damien Neil
96c229ab14 proto: check for required fields in encoding/decoding
Change-Id: I0555a92e0399782f075b1dcd248e880dd48c7d6d
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/170579
Reviewed-by: Herbie Ong <herbie@google.com>
2019-04-04 17:56:20 +00:00
Damien Neil
0d3e8cc096 proto, runtime/protoiface: add support for fast-path marshaling
Allow message implementations to provide optimized versions of standard
operations. Generated messages now include a ProtoReflectMethods method,
returning a protoiface.Methods struct containing pointers to assorted
optional functions.

The Methods struct also includes a Flags field indicating support for
optional features such as deterministic marshaling.

Implementation of the fast paths (and tests) will come in later CLs.

Change-Id: Idd1beed0ecf43ec5e5e7b8da2ee1e08d3ce32213
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/170340
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-04-02 21:23:04 +00:00
Damien Neil
99f24c33b0 proto: wire encoding support
Add proto.Marshal.

Change-Id: If7254bb4c4cbbee782a2a163762f9fcf98e7ab08
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/167388
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-03-14 19:12:27 +00:00