287 Commits

Author SHA1 Message Date
Damien Neil
3dbd95a558 reflect/protoreflect: add List.AppendMutable and Map.Mutable
Add methods to add a new, mutable message to a list or map, matching the
existing Message.Mutable.

These methods are purely a convenience, as each can be implemented in
terms of the existing interface.

Change-Id: I889c20fe37ea0f2a566555212e99e6378fb9fe1d
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/220117
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-02-19 20:35:39 +00:00
Damien Neil
725bfeaf40 internal/impl: support legacy Merger interface
Change-Id: I796be0bb1bb40605073228446364f3a2f1073ef1
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/219557
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-02-18 17:00:39 +00:00
Damien Neil
e8e8875f94 proto, runtime/protoiface, internal/impl: add fast-path Merge
Comparing -tags=protoreflect to fast-path:

name                              old time/op    new time/op    delta
pkg:google.golang.org/protobuf/internal/benchmarks goos:linux goarch:amd64
/Clone/google_message1_proto2-12    1.70µs ± 1%    0.30µs ± 1%  -82.64%  (p=0.001 n=7+7)
/Clone/google_message1_proto3-12    1.01µs ± 1%    0.19µs ± 1%  -80.77%  (p=0.000 n=7+8)
/Clone/google_message2-12            818µs ± 8%     141µs ± 6%  -82.78%  (p=0.000 n=8+8)
pkg:google.golang.org/protobuf/internal/benchmarks/micro goos:linux goarch:amd64
EmptyMessage/Clone-12               51.1ns ± 1%    39.3ns ± 3%  -23.03%  (p=0.000 n=7+8)
RepeatedInt32/Clone-12              24.5µs ± 1%     1.1µs ± 3%  -95.64%  (p=0.000 n=8+8)
Required/Clone-12                    978ns ± 1%     132ns ± 2%  -86.46%  (p=0.000 n=8+8)

name                              old alloc/op   new alloc/op   delta
pkg:google.golang.org/protobuf/internal/benchmarks goos:linux goarch:amd64
/Clone/google_message1_proto2-12    1.08kB ± 0%    0.74kB ± 0%  -31.85%  (p=0.000 n=8+8)
/Clone/google_message1_proto3-12      872B ± 0%      544B ± 0%  -37.61%  (p=0.000 n=8+8)
/Clone/google_message2-12            602kB ± 0%     411kB ± 0%  -31.65%  (p=0.000 n=8+8)
pkg:google.golang.org/protobuf/internal/benchmarks/micro goos:linux goarch:amd64
EmptyMessage/Clone-12                96.0B ± 0%     64.0B ± 0%  -33.33%  (p=0.000 n=8+8)
RepeatedInt32/Clone-12              25.4kB ± 0%     3.2kB ± 0%  -87.33%  (p=0.000 n=8+8)
Required/Clone-12                     416B ± 0%      256B ± 0%  -38.46%  (p=0.000 n=8+8)

name                              old allocs/op  new allocs/op  delta
pkg:google.golang.org/protobuf/internal/benchmarks goos:linux goarch:amd64
/Clone/google_message1_proto2-12      52.0 ± 0%      21.0 ± 0%  -59.62%  (p=0.000 n=8+8)
/Clone/google_message1_proto3-12      33.0 ± 0%       3.0 ± 0%  -90.91%  (p=0.000 n=8+8)
/Clone/google_message2-12            22.3k ± 0%      7.5k ± 0%  -66.41%  (p=0.000 n=8+8)
pkg:google.golang.org/protobuf/internal/benchmarks/micro goos:linux goarch:amd64
EmptyMessage/Clone-12                 3.00 ± 0%      2.00 ± 0%  -33.33%  (p=0.000 n=8+8)
RepeatedInt32/Clone-12               1.51k ± 0%     0.00k ± 0%  -99.80%  (p=0.000 n=8+8)
Required/Clone-12                     51.0 ± 0%      18.0 ± 0%  -64.71%  (p=0.000 n=8+8)

Change-Id: Ife9018097c34cb025dc9c4fdd9a61b2f947853c6
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/219147
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2020-02-14 21:47:10 +00:00
Damien Neil
316febd1ab internal/impl: pass *coderFieldInfo into fast-path functions
Refactor the fast-path size, marshal, unmarshal, and isinit functions to
take the *coderFieldInfo for the field as input.

This replaces a number of closures capturing field-specific information
with functions taking that information as an explicit parameter.

Change-Id: I8cb39701265edb7b673f6f04a0152d5f4dbb4d5d
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/218937
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2020-02-12 18:34:15 +00:00
Joe Tsai
93bccf763e all: scrub all TODOs
TODOs that we do not intend to address have been deleted.
Those that are blocking v2 release are marked with "blocks".

Change-Id: I7efa9e546d0637b562101d0edc7009893d762722
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/218878
Reviewed-by: Damien Neil <dneil@google.com>
2020-02-10 19:28:48 +00:00
Damien Neil
01b51b4f96 proto, internal/errors: add Error sentinel, errors.Wrap
Add a sentinel proto.Error error which matches all errors returned by
packages in this module.

Document that protoregistry.NotFound is an exact sentinel value for
performance reasons.

Add a Wrap function to the internal/errors package and use it to wrap
errors from outside sources (resolvers). Wrapped errors match
proto.Error.

Fixes golang/protobuf#1021.

Change-Id: I45567df3fd6c8dc9a5caafdb55654827f6fb1941
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/215338
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-02-07 21:09:48 +00:00
Damien Neil
9afe9bb78b internal/impl: validate messagesets
Change-Id: Id90bb386e7481bb9dee5a07889f308f1e1810825
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/218438
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-02-07 20:06:04 +00:00
Damien Neil
f9d4fdf054 internal/impl: fix validation of required group fields
Change-Id: I3c3b5cfbea599dc08096aa5992b7829c2e50f25d
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/218578
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-02-07 20:05:54 +00:00
Damien Neil
6fb29949b8 all: tests, tweaks for lazy extension decoding
Add a test to confirm that extensions are lazily decoded when we expect.

Drop the UnmarshalDefaultResolver flag. I added it thinking for some
reason that internal/impl couldn't depend on protoregistry; since it can
(and does), it's simpler to just test if the resolver is the expected
value.

Use a default set of options when lazily unmarshaling extensions.

Change-Id: Ied7666ffdc3bf90630260a80c9568d9a945048bc
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/218038
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2020-02-06 19:43:25 +00:00
Damien Neil
4eefd77886 internal/impl: init map value MessageInfos in validator
I'm not sure how to write a good test for this one, since it's so
specific to both the code and the ordering of initialization. Just
sticking the fuzzer-provided case into our standard test message set
doesn't do it, because something else has initialized the MessageInfo by
the time the test gets there.

Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=20543
Change-Id: I508222b43e52287f73e2ed32ce9b954a5f81717b
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/218257
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-02-06 18:58:27 +00:00
Damien Neil
40cba14b26 internal/impl: fix for lazy decoding of groups
Bit of a weird case in why this wasn't caught by tests: When validating
extension groups, we were validating an empty buffer rather than the
message content. For groups, this validation always fails due to a lack
of a group end tag. We'd then skip lazy decoding of the extension field
and proceed with eager decoding, which would behave correctly.

Change extension validation to report an error immediately on an invalid
result from the validator, which is both safe (assuming we trust the
validator) and would have caught this problem (by failing to decode the
extension field, rather than silently failing to eager decoding).

Change-Id: Id6c2d21fb687062bc74d9eb93760a1c24a6fe883
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/217767
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-02-05 22:49:38 +00:00
Damien Neil
0f783d864b internal/impl: fix off-by-one in varint validation
Fixes: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=20532
Change-Id: I670698a1ef780f341f336929384132febe2b40a1
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/217766
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-02-05 17:16:50 +00:00
Damien Neil
cadb4ab3b1 internal/impl: refactor validation a bit
Return the size of the field read from the validator, permitting us to
avoid an extra parse when skipping over groups.

Return an UnmarshalOutput from the validator, since it already combines
two of the validator outputs: bytes read and initialization status.

Remove initialization status from the ValidationStatus enum, since it's
covered by the UnmarshalOutput.

Change-Id: I3e684c45d15aa1992d8dc3bde0f608880d34a94b
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/217763
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-02-05 05:32:50 +00:00
Damien Neil
d025c95110 proto, internal/protobuild: add test proto template builder
The proto package tests often test several variations of messages with a
similar shape. For example, most tests are performed with a proto2
message with a regular field, a proto2 message with an extension field,
and a proto3 message.

Add a protobuild package which can initialize all these variations from
a single template. For example, these three messages:

	&testpb.TestAllTypes{OptionalInt32: proto.Int32(1)}

	&test3pb.TestAllTypes{OptionalInt32: 1}

	m := &testpb.TestAllExtensions{}
	proto.SetExtension(m, &testpb.E_OptionalInt32, 1)

can all be constructed from the template:

	protobuild.Message{"optional_int32": 1}

This reduces redundancy in tests and will make it more practical to
test alternative code generators.

Change-Id: I3245a4bf74ee1bce957bc772fed513d427720677
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/217457
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2020-02-03 19:14:55 +00:00
Damien Neil
4d918167a9 internal/impl: catch varint overflow in validator
Fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=20477

Change-Id: I6afe82e3818f8b4e9cf5eded2125317eae8be49d
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/217309
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2020-02-03 18:21:31 +00:00
Joe Tsai
74b1460c5b encoding: add Format helper function and method
The Format function and MarshalOptions.Format method are helper
functions for directly obtaining the formatted string for a message
without having to deal with errors or convert a []byte to string.
It is only intended for human consumption (e.g., debugging or logging).

We also add a MarshalOptions.Multiline option to specify that the output
should use some default indentation in a multiline output.

This assists in the v1 to v2 migration where:
	protoV1.CompactTextString(m) => prototext.MarshalOptions{}.Format(m)
	protoV1.MarshalTextString(m) => prototext.Format(m)

At Google, there are approximately 10x more usages of MarshalTextString than
CompactTextString, so it makes sense that the top-level Format function
does multiline expansion by default.

Fixes #850

Change-Id: I149c9e190a6d99b985d3884df675499a3313e9b3
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/213460
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Herbie Ong <herbie@google.com>
2020-01-30 07:50:58 +00:00
Damien Neil
1887ff702c internal/impl: better fast-path init checks for extensions
Unknown extensions are initialized.

Valid extensions with no isInit func are initialized.

Change-Id: I2975c7ef85d2b777eca467d3b1861d20de8e24fc
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/216960
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-30 05:43:23 +00:00
Damien Neil
6f2977906d internal/impl: fix validator bytes field length decoding
Missing a bounds check on the first byte.

Change-Id: I089fa8dcc1a14d11faca1acba758b6b811b16ac4
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/216957
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-30 00:26:49 +00:00
Damien Neil
c70f5d59d1 internal/impl: avoid redundant lazy extension inits
After taking the lock on a lazy extension's state, check to see if it
was initialized while we were waiting for the lock.

Change-Id: I1cbd52e9d655eec6c9142c97689ae36f219a28f2
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/216898
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2020-01-29 23:04:37 +00:00
Damien Neil
0ae1c9789a internal/impl: lazy extension decoding
Historically, extensions have been placed in the unknown fields section
of the unmarshaled message and decoded lazily on demand. The current
unmarshal implementation decodes extensions eagerly at unmarshal time,
permitting errors to be immediately reported and correctly detecting
unset required fields in extension values.

Add support for validated lazy extension decoding, where the extension
value is fully validated at initial unmarshal time but the fully
unmarshaled message is only created lazily.

Make this behavior conditional on the protolegacy flag for now.

Change-Id: I9d742496a4bd4dafea83fca8619cd6e8d7e65bc3
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/216764
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-29 21:35:31 +00:00
Damien Neil
a522d5fa0c internal/impl: fix tag decoding when field num doesn't fit in int32
Discoverd by OSS-Fuzz.

Change-Id: Ie2feefacee4ae632802fa920ac9694b525690eb2
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/216619
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-29 20:57:21 +00:00
Damien Neil
524c60670a runtime/protoiface: use more efficient options representation
Change the representation of option flags in protoiface from bools to a
bitfield. This brings the representation of options in protoiface in
sync with that in internal/impl.

This change has several benefits:

1. We will probably find that we need to add more option flags over time.
Converting to the more efficient representation of these flags as high
in the call stack as possible minimizes the performance implication of
the struct growing.

2. On a similar note, this avoids the need to convert from the compact
representation to the larger one when passing from internal/impl to
proto, since the {Marshal,Unmarshal}State methods take the compact form.

3. This removes unused options from protoiface. Instead of documenting
that AllowPartial is always set, we can just not include an AllowPartial
flag in the protoiface options.

4. Conversely, this provides a way to add option flags to protoiface
that we don't want to expose in the proto package.

name                             old time/op    new time/op    delta
EmptyMessage/Wire/Marshal-12       11.1ns ± 7%    10.1ns ± 1%   -9.35%  (p=0.000 n=8+8)
EmptyMessage/Wire/Unmarshal-12     7.07ns ± 0%    6.74ns ± 1%   -4.58%  (p=0.000 n=8+8)
EmptyMessage/Wire/Validate-12      4.30ns ± 1%    3.80ns ± 8%  -11.45%  (p=0.000 n=7+8)
RepeatedInt32/Wire/Marshal-12      1.17µs ± 1%    1.21µs ± 7%   +4.09%  (p=0.000 n=8+8)
RepeatedInt32/Wire/Unmarshal-12     938ns ± 0%     942ns ± 3%     ~     (p=0.178 n=7+8)
RepeatedInt32/Wire/Validate-12      521ns ± 4%     543ns ± 7%     ~     (p=0.157 n=7+8)
Required/Wire/Marshal-12           97.2ns ± 1%    95.3ns ± 1%   -1.98%  (p=0.001 n=7+7)
Required/Wire/Unmarshal-12         41.0ns ± 9%    38.6ns ± 3%   -5.73%  (p=0.048 n=8+8)
Required/Wire/Validate-12          25.4ns ±11%    21.4ns ± 3%  -15.62%  (p=0.000 n=8+7)

Change-Id: I3ac1b00ab36cfdf61316ec087a5dd20d9248e4f6
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/216760
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-28 23:33:31 +00:00
Damien Neil
212b05b808 internal/testprotos: make TestAllExtensions recursive
Tweak the test message to allow creating messages with extensions that
contain extensions that contain extensions, etc.

Change-Id: I41844ae699c88ab96bf0d30db3a3fbaf09616161
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/216761
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-28 23:28:36 +00:00
Damien Neil
a60e709ac8 proto: fix DiscardUnknown
UnmarshalOptions.DiscardUnknown was simply not working. Oops. Fix it.
Add a test.

Change-Id: I76888eae1221d99a007f0e9cdb711d292e6856b1
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/216762
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-28 23:27:58 +00:00
Damien Neil
cb0bfd0f40 internal/impl: reduce redundant MessageInfo initializations in validator
name                            old time/op    new time/op    delta
EmptyMessage/Wire/Validate-12     4.58ns ± 0%    4.29ns ± 1%   -6.22%  (p=0.000 n=7+8)
RepeatedInt32/Wire/Validate-12     702ns ± 1%     518ns ± 0%  -26.12%  (p=0.001 n=7+7)
Required/Wire/Validate-12         30.6ns ± 6%    22.1ns ± 0%  -27.81%  (p=0.000 n=8+7)

Change-Id: I0d1db8583aa0bf4468bc385c213eb6adff001297
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/216627
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-28 22:46:45 +00:00
Damien Neil
8fa11b1122 internal/impl: inline most field decoding in the validator
name                            old time/op    new time/op    delta
EmptyMessage/Wire/Validate-12     4.51ns ± 1%    4.57ns ± 0%   +1.19%  (p=0.045 n=8+8)
RepeatedInt32/Wire/Validate-12     910ns ± 0%     726ns ± 3%  -20.13%  (p=0.000 n=8+8)
Required/Wire/Validate-12         34.5ns ± 0%    29.6ns ± 5%  -13.99%  (p=0.000 n=7+8)

Change-Id: I8ac90ed3fc79dfef7f2500f13b33fd2593fc0fc1
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/216625
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-28 22:46:30 +00:00
Damien Neil
5d82883c5a internal/impl: inline small tag decoding in the validator
name                            old time/op    new time/op    delta
EmptyMessage/Wire/Validate-12     4.59ns ± 0%    4.51ns ± 1%   -1.74%  (p=0.001 n=8+8)
RepeatedInt32/Wire/Validate-12    1.28µs ± 0%    0.91µs ± 0%  -28.71%  (p=0.000 n=7+8)
Required/Wire/Validate-12         48.3ns ± 2%    34.5ns ± 0%  -28.69%  (p=0.001 n=7+7)

Change-Id: If7c431ee23d930d44af0fc26b7bd2149d3aded64
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/216624
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-28 22:46:12 +00:00
Damien Neil
adbbc8ec47 internal/impl: inline some small varint decoding
Inline decoding of 1- and 2-byte varints in generated unmarshal
functions.

name                             old time/op  new time/op  delta
EmptyMessage/Wire/Unmarshal      40.2ns ± 2%  40.1ns ± 1%     ~     (p=0.355 n=37+37)
EmptyMessage/Wire/Unmarshal-12   7.12ns ± 1%  6.87ns ± 1%   -3.49%  (p=0.000 n=37+39)
RepeatedInt32/Wire/Unmarshal     6.46µs ± 1%  5.78µs ± 1%  -10.65%  (p=0.000 n=35+33)
RepeatedInt32/Wire/Unmarshal-12  1.05µs ± 2%  0.98µs ± 2%   -6.79%  (p=0.000 n=33+40)
Required/Wire/Unmarshal           251ns ± 1%   216ns ± 1%  -13.69%  (p=0.000 n=38+36)
Required/Wire/Unmarshal-12       42.4ns ± 1%  37.7ns ± 2%  -11.02%  (p=0.000 n=37+39)

Change-Id: Iecfc38fcae00979b89a093368821cca7f2357578
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/216421
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2020-01-27 19:09:55 +00:00
Damien Neil
170b2bfca6 internal/impl: precompute required bit in validator
Required field validation populates a bitmask of observed required
fields. Store a uint64 containing the bit to set in the validationInfo
rather than the index of the bit. Provides a noticeable speed increase
in validation.

name                             old time/op  new time/op  delta
EmptyMessage/Wire/Unmarshal      40.2ns ± 1%  40.2ns ± 2%    ~     (p=0.860 n=35+37)
EmptyMessage/Wire/Unmarshal-12   7.13ns ± 5%  7.12ns ± 1%    ~     (p=0.112 n=37+37)
RepeatedInt32/Wire/Unmarshal     6.57µs ± 1%  6.46µs ± 1%  -1.56%  (p=0.000 n=39+35)
RepeatedInt32/Wire/Unmarshal-12  1.05µs ± 2%  1.05µs ± 2%    ~     (p=0.659 n=37+33)
Required/Wire/Unmarshal           258ns ± 1%   251ns ± 1%  -2.87%  (p=0.000 n=32+38)
Required/Wire/Unmarshal-12       44.3ns ± 2%  42.4ns ± 1%  -4.36%  (p=0.000 n=36+37)

Change-Id: Ib1cb74d3e348355a6a2f66aecf8fdc4b58cd84d4
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/216420
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-26 22:23:18 +00:00
Damien Neil
ce8f7f6353 internal/impl: inline small tag decoding
Inline varint decoding of small (1- and 2-byte) field tags in the
fast-path unmarshaler.

name                             old time/op  new time/op  delta
EmptyMessage/Wire/Unmarshal      40.6ns ± 1%  40.2ns ± 1%   -1.02%  (p=0.000 n=37+35)
EmptyMessage/Wire/Unmarshal-12   6.77ns ± 2%  7.13ns ± 5%   +5.32%  (p=0.000 n=37+37)
RepeatedInt32/Wire/Unmarshal     9.46µs ± 1%  6.57µs ± 1%  -30.56%  (p=0.000 n=38+39)
RepeatedInt32/Wire/Unmarshal-12  1.50µs ± 2%  1.05µs ± 2%  -30.00%  (p=0.000 n=39+37)
Required/Wire/Unmarshal           371ns ± 1%   258ns ± 1%  -30.44%  (p=0.000 n=38+32)
Required/Wire/Unmarshal-12       60.3ns ± 1%  44.3ns ± 2%  -26.45%  (p=0.000 n=38+36)

Change-Id: Ie80415dea8cb6b840eafa52f0572046a1910a9b1
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/216419
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-26 22:23:05 +00:00
Damien Neil
0bf97b7e36 internal/impl: messageset validation and isinit fixes
Recognize messagesets in the validator. Currently, this just gives
up and reports an unknown validity rather than trying to descend
into the messageset.

Plumb fast-path initialization checks through messageset decoding.

Change-Id: Ice55f28e8555764e4ce2720251830e8cf475c133
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/216245
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-24 20:27:57 +00:00
Damien Neil
c600d6c086 all: do best-effort initialization check on fast path unmarshal
Add a fast check for required fields to the fast path unmarshal.
This is best-effort and will fail to detect some initialized
messages: Messages with more than 64 required fields, messages
split across multiple tags, possibly other cases.

In the cases where it works (which is most of them in practice),
this permits us to skip the IsInitialized check.

Change-Id: I6b70953a333033a5e64fb7ca37a59786cb0f75a0
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/215878
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-22 20:57:14 +00:00
Damien Neil
d30e561d9e proto: add MarshalState, UnmarshalState
Add functions to the proto package which plumb through the fast-path state.

As a sample use case: A followup CL adds an Initialized field to
protoiface.UnmarshalOutput, permitting the unmarshaller to report back
when it can confirm that a message is fully initialized. We want to
preserve that information when an unmarshal operation threads through
the proto package (such as when unmarshaling extensions).

To allow these functions to be added as methods of MarshalOptions and
UnmarshalOptions rather than top-level functions, separate the options
from the input structs.

Also update options passed to fast-path methods to set AllowPartial and
Merge to reflect the expected behavior of those methods. (Always allow
partial, never merge.)

Change-Id: I482477b0c9340793be533e75a86d0bb88708716a
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/215877
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-22 20:52:17 +00:00
Damien Neil
f0831e87e2 internal/impl: change unmarshal func return to unmarshalOptions
The fast-path unmarshal funcs return the number of bytes consumed.

Change these functions to return an unmarshalOutput struct instead, to
make it easier to add to the results. This is groundwork for allowing
the fast-path unmarshaler to indicate when the unmarshaled message is
known to be initialized.

Change-Id: Ia8c44731a88f5be969a55cd98ea26282f412c7ae
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/215720
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-22 00:22:58 +00:00
Damien Neil
61781dd92f 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 <joetsai@google.com>
2020-01-22 00:22:46 +00:00
Damien Neil
f12fb45fd6 all: add ProtoMethods method to protoreflect.Message
Promote the fast-path magic ProtoMethods method to first-class citizen
of the protoreflect.Message interface.

To avoid polluting the protoreflect package with the various types
required by this method, make the necessary protoiface types unnamed and
duplicate them in protoreflect.

Updates golang/protobuf#1022.

Change-Id: I9595bae40b3bc7536d727fb6f99b3bce8f73da87
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/215718
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-21 21:05:54 +00:00
Damien Neil
2f643a9741 internal/impl: avoid type conversion on UnmarshalOptions.Resolver
Remove a trivial difference in the definition of the resolver
unmarshaler option to avoid a relatively expensive interface->interface
type conversion.

Change-Id: Iecf9a686af5d17fe3e2d9b80f886c644bf8a25df
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/215657
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-21 19:16:14 +00:00
Damien Neil
6635e7d00a internal/impl: recognized required bytes fields in validation
Add a missed case in validation so we correctly validate bytes fields.
Fixes a case where we would report required bytes fields as potentially
missing.

Change-Id: I3dc4196d6995942d32a795a64214b3679d60ab6c
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/215000
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-15 23:51:51 +00:00
Damien Neil
2ae60936c2 internal/impl: fix unmarshal of group containing their own field number
The fast-path unmarshal was getting confused when parsing a group
containing a field with a number the same as the group's own field
number. Separate the handling of EndGroup tags.

Change-Id: I637702b42c94a26102e693ee29a55e80b37d7f28
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/214737
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-14 19:43:24 +00:00
Joe Tsai
1c8015fff5 all: minor tweaks
Fix a typo in legacy_enum.go.
Rename package in ancient legacy proto so that it doesn't confuse
tooling that assume that the package and directory names match.

Change-Id: I0b896045e74b0a7f998d3e5693b853eb3aa3839c
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/214182
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-12 09:18:34 +00:00
Joe Tsai
55f18259ef internal/testprotos/legacy: rename and regenerate
Avoid dots and dashes in the directory to avoid issues on
build systems that cannot support them well.

Change-Id: I7ea5e6ce0b16c7158c7e53bcf5c3c1a334fe4718
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/214342
Reviewed-by: Damien Neil <dneil@google.com>
2020-01-12 08:13:18 +00:00
Damien Neil
7abc2def69 internal/impl: omit isInit func when not needed
When generating the fast-path functions for a message, group, repeated
message, or repeated group field, check to see if the field message type
requires initialization checks. If not, leave the isInit func unset.

This permits the fast-path isInitialized to skip over these fields
entirely.

Change-Id: Icb5c380077d2216c4215bb0ebc16408e905aaece
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/214179
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-09 23:00:50 +00:00
Damien Neil
54a0a0476a internal/impl: check for required fields in missing map value
If a map value is a message with required fields, the validator should
note that it is uninitialized if a map item contains no value. In this
case, the value is an empty message which obviously does not have the
required field set.

Change-Id: I7698e60765e3c95478f293e121bba3ad7fc88e27
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/213900
Reviewed-by: Joe Tsai <joetsai@google.com>
2020-01-09 05:38:08 +00:00
Damien Neil
b0c26f1868 internal/impl: add message validator
This adds a experimental function to the internal/impl package which
validates a wire-format message against a message type. The validator
reports whether the message can be successfully unmarshaled, and whether
the result is initialized (all required fields are set). In some cases,
the validator returns ambiguous results when full validation would be
expensive.

The validator is unused outside of tests. In the future, it may be used
to permit lazy unmarshaling of some data. It is being added now for
testing; in particular, the wire fuzzer now checks the validator output
for consistency with the unmarshaler.

The validator adds a small amount of unused per-MessageType state. If
this becomes a concern, we could conditionalize it with a build tag.

Change-Id: I4216ef81d6a9ed975302eed189b02d08608858b4
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/212302
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2020-01-07 21:36:47 +00:00
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
Joe Tsai
14ac241a96 internal/impl: return nil for nil enum or message
Ensure that EnumOf, EnumDescriptorOf, EnumTypeOf, ProtoMessageV1Of,
ProtoMessageV2Of, MessageOf, MessageDescriptorOf, and MessageTypeOf
all return nil if passed a nil interface.

This parallels the behavior of reflect.TypeOf or reflect.ValueOf,
which return nil or an invalid value rather than panicking.

Change-Id: I461f15542f16cb0922d627bca6fcad5fc27d87e2
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/213239
Reviewed-by: Damien Neil <dneil@google.com>
2020-01-06 19:10:59 +00:00
Damien Neil
f2427c09d6 proto, internal/impl: reject invalid field numbers in map items
Change-Id: I44a44a36538f6f8b94078b43711d865edb6244f5
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/212257
Reviewed-by: Herbie Ong <herbie@google.com>
2019-12-21 00:16:12 +00:00
Damien Neil
2c0824b512 internal/impl: fix size for zero-length packed extensions
The size calculation for packed repeated extension fields was
considering a zero-length list as encoding to a zero-length
wire.BytesType field, rather than being omitted entirely.

Change-Id: I7d4424a21ca8afd4fa81391caede49cadb4e2505
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/212297
Reviewed-by: Joe Tsai <joetsai@google.com>
2019-12-20 22:08:18 +00:00
Damien Neil
7e690b5b4c internal/impl: fix map decode when value is before key
Fix a bug in handling the case where the encoding for a map item places
the value field (2) before the key field (1).

Change-Id: I2e6ad9af729a199e960e566ed7ef96bba3726990
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/211804
Reviewed-by: Joe Tsai <joetsai@google.com>
2019-12-18 17:42:10 +00:00
Damien Neil
3e42b667d2 internal/impl: faster map fast path
Avoid using protobuf reflection on map values in the fast path. Range
operations in particular are expensive in protoreflect, because the
closure passed to Map.Range escapes.

Iterate maps using a reflect.MapIter when available.

When operating on maps of messages where we have a *MessageInfo for the
message type, directly jump to the fast-path *MessageInfo methods rather
than passing through the proto package.

Benchmarks deltas for a google.protobuf.Struct with JSON represention:
  {"parameters":{"a":{"b":{"c":{"d":{"e":{"f":{"g":{}}}}}}}}}

Compared to previous revision:

  name                      old time/op  new time/op  delta
  NestedStruct/Size         7.22µs ± 2%  4.84µs ± 2%  -32.96%  (p=0.000 n=8+8)
  NestedStruct/Size-8       9.30µs ± 2%  5.89µs ± 2%  -36.60%  (p=0.000 n=8+8)
  NestedStruct/Marshal      77.6µs ±12%   9.8µs ± 4%  -87.33%  (p=0.000 n=8+8)
  NestedStruct/Marshal-8    91.6µs ± 2%  11.9µs ± 2%  -86.99%  (p=0.000 n=8+8)
  NestedStruct/Unmarshal    11.5µs ± 4%   8.7µs ± 2%  -24.76%  (p=0.000 n=8+8)
  NestedStruct/Unmarshal-8  15.4µs ± 4%  11.9µs ± 2%  -22.41%  (p=0.000 n=8+8)

Compared to github.com/golang/protobuf:

  name                      old time/op  new time/op  delta
  NestedStruct/Size         5.42µs ± 1%  4.84µs ± 2%  -10.61%  (p=0.000 n=8+8)
  NestedStruct/Size-8       6.34µs ± 2%  5.89µs ± 2%   -7.10%  (p=0.000 n=8+8)
  NestedStruct/Marshal      12.5µs ± 2%   9.8µs ± 4%  -21.41%  (p=0.000 n=7+8)
  NestedStruct/Marshal-8    14.1µs ± 3%  11.9µs ± 2%  -15.52%  (p=0.000 n=8+8)
  NestedStruct/Unmarshal    9.66µs ± 1%  8.65µs ± 2%  -10.40%  (p=0.000 n=7+8)
  NestedStruct/Unmarshal-8  11.7µs ± 3%  11.9µs ± 2%   +1.95%  (p=0.038 n=8+8)

Change-Id: I0effe6491f30d41f31904777f74eca3ac3694db3
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/211737
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-12-17 22:14:17 +00:00