74 Commits

Author SHA1 Message Date
Joe Tsai
8e9d5f6e8a internal/protolegacy: add stub v1 proto package for testing purposes
The protolegacy package is a minimal version of the v1 proto package.
This allows us to use this stub version as the dependency for
internal/testprotos/legacy packages and avoid a dependency
on the real v1 proto package.

The implementation of most v1 functionality will panic if called.
This way, we know if we the v2 code depends on one of those
unimplemented functions.

Updates golang/protobuf#962

Change-Id: I20b4091706fd456e4b01ae0931cce30a872639b0
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/199297
Reviewed-by: Damien Neil <dneil@google.com>
2019-10-05 23:12:32 +00:00
Damien Neil
6e40b32926 internal/impl: weak field bugfixes
Fix a reversed error check in impl.Export{}.WeakNil.

Check to see if we have a type for the weak field on marshal/size.

Treat a typed nil valued in XXX_Weak as not indicating presence for
the field.

Change-Id: Id667ac7eb4f53236be9e181017082bd8cd21d115
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/198717
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-10-04 17:07:45 +00:00
Damien Neil
47d5893acf internal/impl: support non-struct-pointer legacy message types
Support, to some limited degree, types which implement protoV1.Message
but which are not struct pointers. Our ability to work with these types
is largely limited to calling Marshal or Unmarshal methods, when
present.

Change-Id: Ie1b851d9e753e2b2cb189b17ffeefebe2d8b3a8f
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/198237
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-10-04 16:13: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
Joe Tsai
f2c4ddc7a1 proto/equal: equate nil
Modify Equal to treat nil messages as equal iff both are nil.
Of special note, a typed nil pointer to T is equal to a new(T)
since they are indistinguishable from a protobuf reflection.

Change-Id: Ibf90b43a982e7376e07b4159be198f06230ec194
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/196618
Reviewed-by: Damien Neil <dneil@google.com>
2019-09-23 16:43:47 +00:00
Joe Tsai
641611d984 proto: fix self-merging
While odd, it is possible to merge a message into itself.
In such a situation, the material impact is that repeated
and unknown fields are duplicated. The previous logic would
inifinite loop since the list iteration logic uses the current
length, but since the current length is ever growing, this loop
will never terminate. Instead, record the list length once
and iterate exactly that many times.

Change-Id: Ief98afa1b20bd950a9c2422d4462b170dbe6fa11
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/196857
Reviewed-by: Damien Neil <dneil@google.com>
2019-09-23 16:14:39 +00:00
Joe Tsai
c908144c88 proto: fix race in Merge
Some existing targets (whether correctly or not) rely on it Merge
being safe to call concurrently so long as the set of fields being
merged are disjoint.

Change-Id: I4db9e64efccc7a2d44a5f9b52261b611cce461b0
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/196737
Reviewed-by: Damien Neil <dneil@google.com>
2019-09-20 19:55:42 +00:00
Joe Tsai
6e095998ae proto, internal/impl: implement support for weak fields
Change-Id: I0a3ff79542a3316295fd6c58e1447e597be97ab9
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/189923
Reviewed-by: Damien Neil <dneil@google.com>
2019-09-19 22:41:12 +00:00
Joe Tsai
84177c9bf3 all: use typed variant of protoreflect.ValueOf
Change-Id: I7479632b57e7c8efade12a2eb2b855e9c321adb1
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/196037
Reviewed-by: Damien Neil <dneil@google.com>
2019-09-17 21:33:16 +00:00
Joe Tsai
705acadcc7 proto: reset message by default in Unmarshal
We change Unmarshal to reset a message by default.
* We add a Merge option to UnmarshalOptions for explicit merging.
* We speed up Reset by checking for the Reset method.
* Remove TODOs in prototext and protojson about reset behavior.

Fixes golang/protobuf#890

Change-Id: Ibd8963c741053f564acf061fbdb846699942109c
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/195457
Reviewed-by: Damien Neil <dneil@google.com>
2019-09-17 21:13:42 +00:00
Joe Tsai
cd4a31e202 encoding/prototext: add MarshalOptions.EmitUnknown
This changes text marshaling to avoid unknown fields by default
and instead adds an option so that unknown fields be emitted.
This ensures that the default marshal/unknown can round-trip.

Change-Id: I85c84ba6ab7916d538ec6bfd4e9d399a8fcba14e
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/195778
Reviewed-by: Herbie Ong <herbie@google.com>
2019-09-17 02:56:29 +00:00
Joe Tsai
09217f08d2 all: make error messages unstable
Use internal/detrand in the construction of our error messages.
This alters whether there is one or two spaces following the "proto:" prefix.
While it is easy for users to still work around this mutation,
sit at least forces them to write test infrastructure to more fuzzily
match on error strings.

Change-Id: I4ddca717526ee3fc4dbb1e0b36cfca8c6e0df36d
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/194038
Reviewed-by: Herbie Ong <herbie@google.com>
2019-09-07 00:39:30 +00:00
Damien Neil
d91c422d95 all: remove use of deprecated NewMessage
Replace NewMessage calls with NewField, NewElement, or NewValue.

Change-Id: I6d2bb4f11f0eb2ba7a52308b1addb111137ad4b9
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/193266
Reviewed-by: Joe Tsai <joetsai@google.com>
2019-09-04 17:55:31 +00:00
Damien Neil
293dc761cb internal/impl: change Go representation of extension lists to []T
Change-Id: Iebcefe0330c8f858c7735f9362abfd87043ee39d
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/192458
Reviewed-by: Joe Tsai <joetsai@google.com>
2019-09-03 21:19:03 +00:00
Damien Neil
79bfdbe45b all: rename ExtensionType Descriptor method to TypeDescriptor (1/2)
Descriptor methods generally return a Descriptor with no Go type
information. ExtensionType's Descriptor is an exception, returning an
ExtensionTypeDescriptor containing both the proto descriptor and a
reference back to the ExtensionType. The pure descriptor is accessed
by xt.Descriptor().Descriptor().

Rename ExtensionType's Descriptor method to TypeDescriptor to make it
clear that it behaves a bit differently.

Change 1/2: Add the TypeDescriptor method and deprecate Descriptor.

Change-Id: I1806095044d35a474d60f94d2a28bdf528f12238
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/192139
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-08-28 18:34:29 +00:00
Damien Neil
c5060d2fe6 reflect/protoreflect: add non-allocating Value constructors
Passing a non-pointer type to protoreflect.NewValue causes an
unnecessary allocation in order to store the value in an interface{}.
While this allocation could be avoided by a smarter compiler, no such
compiler exists today.

Add functions for creating new values of a specific type, avoiding the
allocation. (And also adding a small amount of type safety, although
this is unlikely to be important.)

Update the proto and internal/impl packages to use these functions.

Change-Id: Ic733de22ddf19c530189166c853348e1b54b7391
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/191457
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-08-26 17:48:05 +00:00
Tuo Shan
6e25d8c6a6 proto: add tests for unmarshalling invalid field numbers
This change adds tests for unmarshalling fields with various invalid field
numbers. Our current behavior is that proto.Unmarshal will return an error when
it sees zero and larger than max field numbers and return nil for reserved
ones, which matches the C++ behavior. (Note: depending on which parser helper
in the C++ implementation, one may need to call additional method to check the
result, which we don't have in Go)

Change-Id: I8791fd077f25656107556f5606d55d05c1b4a120
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/191459
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-08-23 21:35:40 +00:00
Damien Neil
f1e905b042 all: unify protoV1.ExtensionDesc and proto.ExtensionType
Change protoV1.ExtensionDesc to directly implement ExtensionType
rather than delegating to one.

Unify the previous types protoiface.ExtensionDescV1 and
filetype.Extension in impl.ExtensionInfo. The protoV1.ExtensionDesc
type becomes an alias to ExtensionInfo.

This gives us:

  - Just one implementation of ExtensionType.
  - Generated foopb.E_Ext vars are canonical ExtensionTypes.
  - Generated foopb.E_Ext vars are also v1.ExtensionDescs for backwards
    compatibility.
  - Conversion between legacy and modern representations happens
    transparently when lazily initializing an ExtensionInfo.

Overall, a simplification for users of generated code, since they can
mostly ignore the ExtensionDesc/ExtentionType distinction and use the
same value in either the old or new API.

This is change 3/5 in a series of commits changing protoV1.ExtensionDesc
to directly implement protoreflect.ExtensionType.

1. [v2] Add protoimpl.ExtensionInfo as an alias for
   protoiface.ExtensionDescV1.

2. [v1] Update references to protoimpl.ExtensionInfo to use
   protoiface.ExtensionInfo.

3. [v2] Create protoimpl.ExtensionInfo (an alias to a new type in
   the impl package) and remove protoiface.ExtensionDescV1.

4. [v1] Remove unneeded explicit conversions between ExtensionDesc and
   ExtensionType (since the former now directly implements the latter).

5. [v2] Remove stub conversion functions.

Change-Id: I96ee890541ec11b2412e1a72c9d7b96e4d7f66b4
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/189563
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-08-20 21:32:57 +00:00
Joe Tsai
9b22b9382e internal/impl: treat a nil oneof wrapper as if it were unset
The old implementation had the behavior where a nil wrapper value:
	m := new(foopb.Message)
	m.OneofField = (*foopb.Message_OneofUint32)(nil)
was functionally equivalent to it being directly set to nil:
	m := new(foopb.Message)
	m.OneofField = nil
preserve this semantic in both the table-drive implementation
and the reflection implementation.

Change-Id: Ie44d51e044d4822e61d0e646fbc44aa8d9b90c1f
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/189559
Reviewed-by: Herbie Ong <herbie@google.com>
2019-08-16 00:24:53 +00:00
Damien Neil
16163b4f67 all: drop reflect/prototype package
Remove the remaining uses of the prototype package.

The most significant change is to impl.MessageInfo, which now directly
implements the MessageType interface. This involves two notable changes
to exported fields of MessageInfo:

  - PBType is now Desc.
  - GoType is now GoReflectType. (Name changed to avoid a conflict with
    the GoType method of the MessageType interface.)

Fixes golang/protobuf#911

Change-Id: Ie2aa4766d6887ceaa9cf06b1f109aa6e6e2a208f
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/189340
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-08-09 19:06:30 +00:00
Joe Tsai
1799d1111a all: rename tag and flags for legacy support
Rename build tag "proto1_legacy" -> "protolegacy"
to be consistent with the "protoreflect" tag.

Rename flag constant "Proto1Legacy" -> "ProtoLegacy" since
it covers more than simply proto1 legacy features.
For example, it covers alpha-features of proto3 that
were eventually removed from the final proto3 release.

Change-Id: I0f4fcbadd4b5a61c87645e2e5be11d187e59157c
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/189345
Reviewed-by: Damien Neil <dneil@google.com>
2019-08-08 20:49:00 +00:00
Damien Neil
92f76189a3 all: refactor extensions, add proto.GetExtension etc.
Change protoiface.ExtensionDescV1 to implement protoreflect.ExtensionType.

ExtensionDescV1's Name field conflicts with the Descriptor Name method,
so change the protoreflect.{Message,Enum,Extension}Type types to no
longer implement the corresponding Descriptor interface. This also leads
to a clearer distinction between the two types.

Introduce a protoreflect.ExtensionTypeDescriptor type which bridges
between ExtensionType and ExtensionDescriptor.

Add extension accessor functions to the proto package:
proto.{Has,Clear,Get,Set}Extension. These functions take a
protoreflect.ExtensionType parameter, which allows writing the
same function call using either the old or new API:

  proto.GetExtension(message, somepb.E_ExtensionFoo)

Fixes golang/protobuf#908

Change-Id: Ibc65d12a46666297849114fd3aefbc4a597d9f08
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/189199
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-08-08 18:20:51 +00:00
Joe Tsai
e815d6a43b all: remove dead code
Change-Id: I1344d6afca9d3348db849c2b5f387ac18b80d2ba
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/189021
Reviewed-by: Damien Neil <dneil@google.com>
2019-08-06 21:16:48 +00:00
Damien Neil
8003f08e51 proto, internal/impl: zero-length proto2 bytes fields should be non-nil
Fix decoding of zero-length bytes fields to produce a non-nil []byte.

Change-Id: Ifb7791a47df81091700f7226523371d1386fb1ad
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/188765
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-08-05 21:44:30 +00:00
Joe Tsai
74615a3960 proto: remove dependency on github.com/golang/protobuf
The only remaining dependencies are for benchmarks and
internal/testprotos/legacy.

Change-Id: I0f7f5292000ccad91bc9526e40fa4d0ec3a36e43
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/186157
Reviewed-by: Damien Neil <dneil@google.com>
2019-07-15 22:26:29 +00:00
Joe Tsai
6bd33b6e6d proto: equate floating-point NaNs
Treating NaNs as inequal has benefits in mathmetical operations,
but is almost certainly never desired behavior in tests.
Making them equal allows us to document that Equal reports true
if the encoded bytes are also equal (under deterministic marshaling).

Change-Id: Id11c9c1681d8785bcc52f0f42064339194065649
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/186179
Reviewed-by: Damien Neil <dneil@google.com>
2019-07-15 21:36:38 +00:00
Joe Tsai
c51e2e0293 all: support enforce_utf8 override
In 2014, when proto3 was being developed, there were a number of early
adopters of the new syntax. Before the finalization of proto3 when
it was released in open-source in July 2016, a decision was made to
strictly validate strings in proto3. However, some of the early adopters
were already using invalid UTF-8 with string fields.
The google.protobuf.FieldOptions.enforce_utf8 option only exists to support
those grandfathered users where they can opt-out of the validation logic.
Practical use of that option in open source is impossible even if a user
specifies the proto1_legacy build tag since it requires a hacked
variant of descriptor.proto that is not externally available.

This CL supports enforce_utf8 by modifiyng internal/filedesc to
expose the flag if it detects it in the raw descriptor.
We add an strs.EnforceUTF8 function as a centralized place to determine
whether to perform validation. Validation opt-out is supported
only in builds with legacy support.

We implement support for validating UTF-8 in all proto3 string fields,
even if they are backed by a Go []byte.

Change-Id: I9c0628b84909bc7181125f09db730c80d490e485
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/186002
Reviewed-by: Damien Neil <dneil@google.com>
2019-07-15 19:53:05 +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
a8a2cea3e7 proto: move T->*T wrappers from internal/scalar to proto
Usage of these is pervasive in code which works with proto2, and proto2
will be with us for a long, long time to come. Move them to the proto
package.

Change-Id: I1b2e57429fd5a8f107a848a4492d20c27f304bd7
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/185543
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-07-12 17:35:01 +00:00
Joe Tsai
09cef32bce proto: cleanup invalid extension exception
The invalidExtensions flag no longer seems necessary. Tests pass without it.

Change-Id: Ieb35e26912b047718ccbfcdc926625aec1cd8c87
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/185937
Reviewed-by: Damien Neil <dneil@google.com>
2019-07-12 16:21:09 +00:00
Joe Tsai
6c28674cea proto: fix merge semantics for oneof message
The proper semantics for a message field within a oneof
when unmarshaling is to merge into an existing message,
rather than replacing it.

Change-Id: I7c08f6e4fa958c6ee6241e9083f7311515a97e15
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/185957
Reviewed-by: Damien Neil <dneil@google.com>
2019-07-12 16:20:52 +00:00
Damien Neil
7492a09da9 internal/impl: support packed extensions
Change-Id: I5a9e22f1c98f5db9caae1681775017da5aa67394
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/185541
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-07-11 18:07:16 +00:00
Damien Neil
3d0706ac2e proto, internal/impl: make IsInitialized more consistent
Make the fast-path and slow-path versions of IsInitialized report
exactly the same errors: An errors.RequiredNotSet containing the
full name of one of the unset required fields.

Bugfix: Fast-path IsInitialized on a nil message reports an error only
when the message directly contains required fields.

Bugfix: Include fast-path IsInitialized in legacy messageIfaceWrapper.

Fixes golang/protobuf#887

Change-Id: Ia5e4b386f8c23f6f855d995f4a098b1338acbae3
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/185397
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-07-09 19:49:22 +00:00
Damien Neil
dd380ab64d proto: bench_test fixes
Don't fail on test cases with partial messages.

Create a new message on each decode cycle.

Change-Id: I7c1d91a2853e340fc0bae05a238bb28eb682e6ce
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/185377
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-07-09 19:32:32 +00:00
Joe Tsai
8d30bbeede all: remove dependency on proto v1
This does not remove all dependencies,
but all of the cases where it can now be implemented in terms of v2.

Change-Id: Idc5b0273f0d35c284bf2141eb9cce998692ceb15
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/184878
Reviewed-by: Herbie Ong <herbie@google.com>
2019-07-03 04:59:17 +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
Joe Tsai
2fc306a8e3 proto: implement Merge
Change-Id: Ibb579bf5ad8548359dfd9805fd3022bcd53a6379
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/183679
Reviewed-by: Damien Neil <dneil@google.com>
2019-06-24 22:36:08 +00:00
Damien Neil
42cfff4a76 benchmarks: add general-purpose benchmarks directory
Move the benchmarks using the common protobuf datasets out of proto/ and
into their own directory. Add benchmarks for text and JSON.

Move initialization out of the Benchmark function to avoid including it
in CPU/memory profiles.

We could put benchmarks in each individual package (proto, prototext,
etc.), but the need for common infrastructure around managing the test
data makes it simpler to keep the benchmarks together. Also, it's nice
to have a one-stop overview of performance.

Change-Id: I17c37efb91b2413fc43ab1b4c35bff2e1330bc0a
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/183245
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-06-24 19:20:48 +00:00
Damien Neil
5322bdb290 internal/impl: add fast-path for IsInitialized
This currently returns uninformative errors from the fast path and then
consults the slow, reflection-based path only when an error is detected.
Perhaps it's worth going through the effort of producing better errors
directly on the fast path.

Change-Id: I68536e9438010dbd97dbaff4f47b78430221d94b
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/171462
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-06-24 17:42:23 +00:00
Damien Neil
cedb595154 proto, internal/impl: avoid string->[]byte conversions
We trust the compiler to optimize away the string->[]byte conversion in
code like:

	b = wire.AppendBytes(b, []byte(s))

In testing (go 1.12.5 linux/amd64), this optimization is not happening.
Perhaps newer versions of the compiler will optimize this, but we
shouldn't rely on it; avoid unnecessary conversions.

Benchmark differences vs https://golang.org/cl/171462:

  name                                   old time/op    new time/op    delta
  Wire/Marshal/google_message1_proto2-6     310ns ± 2%     189ns ± 3%  -39.20%  (p=0.000 n=8+8)
  Wire/Marshal/google_message1_proto3-6     389ns ± 8%     261ns ± 2%  -33.03%  (p=0.000 n=8+8)
  Wire/Marshal/google_message2-6            103µs ±11%      59µs ± 4%  -42.17%  (p=0.000 n=8+8)

  name                                   old alloc/op   new alloc/op   delta
  Wire/Marshal/google_message1_proto2-6      592B ± 0%      240B ± 0%  -59.46%  (p=0.000 n=8+8)
  Wire/Marshal/google_message1_proto3-6      576B ± 0%      224B ± 0%  -61.11%  (p=0.000 n=8+8)
  Wire/Marshal/google_message2-6            196kB ± 0%      90kB ± 0%  -54.05%  (p=0.000 n=8+8)

  name                                   old allocs/op  new allocs/op  delta
  Wire/Marshal/google_message1_proto2-6      5.00 ± 0%      1.00 ± 0%  -80.00%  (p=0.000 n=8+8)
  Wire/Marshal/google_message1_proto3-6      5.00 ± 0%      1.00 ± 0%  -80.00%  (p=0.000 n=8+8)
  Wire/Marshal/google_message2-6            1.66k ± 0%     0.00k ± 0%  -99.94%  (p=0.000 n=8+8)

Change-Id: Idab7634b8c86604dffa46895ba2e61be38c9bd9c
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/183380
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-06-24 17:41:45 +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
Damien Neil
a80229e4ed proto: add benchmark using protobuf repo test data
The primary (cross-language) protobuf repository contains benchmark data
sets. Add benchmarks using this data. (A version of this benchmark exists
in the protobuf repository, but it uses the v1 API and isn't trivial to
get working.)

Fetch the small benchmark datasets from the
github.com/protocolbuffers/protobuf repo by default. Add a
download_benchdata.bash script which fetches the larger datasets as
well.

Generate necessary packages under internal/testprotos/benchmarks.

To run:

  go run ./proto -bench=BenchmarkData

Usual caveats about benchmarking apply: While these benchmarks use
realistic data, isolated microbenchmarking of proto operations is not
necessarily representitive of performance in production systems.

Change-Id: I58d107554baf104568c86997b5ad50be8b2a5790
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/183297
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-06-20 20:38:50 +00:00
Joe Tsai
ef75becaa8 proto: fix memory aliasing bug in Reset
Fix a bug where using Message.Range alone does not clear
memory references for empty, but allocated lists and maps.
Thus, we iterate over every known field and explicitly call Clear.

Change-Id: I9c1847d4056cf66f3199947150f3140d0783444a
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/183197
Reviewed-by: Damien Neil <dneil@google.com>
2019-06-20 19:48:37 +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
Joe Tsai
60b368fb35 runtime/protolegacy: remove package
The legacy logic has now been merged into impl.

Change-Id: I999dfa2b7f49cbd98614a54d8ec70e611a3d033a
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/178544
Reviewed-by: Damien Neil <dneil@google.com>
2019-05-23 18:01:25 +00:00
Joe Tsai
1c28304cc5 proto, encoding/protojson, encoding/prototext: use Resolver interface
Instead of accepting a concrete protoregistry.Types type,
accept an interface that provides the necessary functionality
to perform the serialization.

The advantages of this approach:
* There is no need for complex logic to allow a Parent or custom
Resolver on the protoregistry.Types type.
* Users can pass their own custom resolver implementations directly
to the serialization functions.
* This is a more principled approach to plumbing custom resolvers
than the previous approach of overloading behavior on the concrete
Types type.

The disadvantages of this approach:
* A pointer to a concrete type is 8B, while an interface is 16B.
However, the expansion of the {Marshal,Unmarshal}Options structs
should be a concern solved separately from how to plumb custom resolvers.
* The resolver interfaces as defined today may be insufficient to
provide functionality needed in the future if protobuf expands its
feature set. For example, let's suppose the Any message permits
directly representing a enum by name. This would require the ability
to lookup an enum by name. To support that hypothetical need,
we can document that the serializers type-assert the provided Resolver
to a EnumTypeResolver and use that if possible. There is some loss
of type safety with this approach, but provides a clear path forward.

Change-Id: I81ca80e59335d36be6b43d57ec8e17abfdfa3bad
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/177044
Reviewed-by: Damien Neil <dneil@google.com>
2019-05-22 19:38:45 +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