16 Commits

Author SHA1 Message Date
Damien Neil
ce413af0b3 internal/impl: faster oneof marshaling
Change size, marshal, and isinit operations on oneofs to look up the
currently-set oneof type in a map rather than testing for each possible
oneof field in turn.

Significantly improves oneof encoding speed for oneofs with a
substantial number of fields:

  go test ./proto -bench=./oneof.*string.*test.TestAll -benchmem -count=8 -cpu=1

  name                                        old time/op    new time/op    delta
  Encode/oneof_(string)_(*test.TestAllTypes)     911ns ± 1%     397ns ± 3%  -56.45%  (p=0.000 n=8+7)
  Decode/oneof_(string)_(*test.TestAllTypes)     899ns ± 1%     922ns ± 1%   +2.49%  (p=0.001 n=7+7)

Fixes golang/protobuf#950

Change-Id: I9393a87975ce09011d885a8af4a63a639ea8452f
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/210281
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-12-09 22:00:58 +00:00
Damien Neil
79571e90e2 internal/impl: improve extension fast path performance
Stash fast-path information for extensions on the ExtensionInfo. In
the usual case where an ExtensionType's underlying implementation is
an *ExtensionInfo, fetching the fast-path information becomes a type
assertion rather than a mutex-guarded map access.

Maintain a global sync.Map for the case where an ExtensionType isn't an
*ExtensionInfo.

Substantially improves performance for fast-path operations on
extensions:

Encode/MessageSet_type_id_before_message_content-12      267ns ± 1%   185ns ± 1%  -30.44%  (p=0.001 n=7+7)
Encode/basic_scalar_types_(*test.TestAllExtensions)-12  1.94µs ± 1%  0.40µs ± 1%  -79.32%  (p=0.000 n=8+7)

Change-Id: If048b521deb3665a090ea3d0a178c61691d4201e
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/210540
Reviewed-by: Joe Tsai <joetsai@google.com>
2019-12-09 18:59:42 +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
ce3384cd34 proto, internal/impl: store unknown MessageSet items in non-mset format
In the v1 implementation, unknown MessageSet items are stored in a
message's unknown fields section in non-MessageSet format. For example,
consider a MessageSet containing an item with type_id T and value V.
If the type_id is not resolvable, the item will be placed in the unknown
fields as a bytes-valued field with number T and contents V. This
conversion is then reversed when marshaling a MessageSet containing
unknown fields.

Preserve this behavior in v2.

One consequence of this change is that actual unknown fields in a
MessageSet (any field other than 1) are now discarded. This matches
the previous behavior.

Change-Id: I3d913613f84e0ae82481078dbc91cb25628651cc
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/205697
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-11-11 19:40:27 +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
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
0484b1a125 internal/impl: refactor MessageInfo
Changes made:
* Move MessageInfo.{methods,extensionFieldInfosMu,extensionFieldInfos}
to the coderMessageInfo type since they are fields all related to the
fast-path implementation, which is what the coderMessageInfo is for.
* Rename message_field.go -> message_reflect_field.go to make it obvious
from the file name that this only deals with the reflection implementation.
* Rename message_test.go -> message_reflect_test.go.
* Move reflection-specific implementation functions from message.go
to message_reflect.go. The intention is to make it such that message.go
is the entry point to message implementations and is agnostic towards
whether we are implementing reflection or the table-driven fast path.

Overall, there is no semantic changes. Just code movement.

Change-Id: I7743c39ba84dc63cd2a02934c3319285e16d6b1c
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/190100
Reviewed-by: Herbie Ong <herbie@google.com>
2019-08-15 22:26:40 +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
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
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
d421150c3b reflect/protoreflect: add Enum.Type and Message.Type
CL/174938 removed these methods in favor of a method that returned
only the descriptors. This CL adds back in the Type methods alongside
the Descriptor methods.

In a vast majority of protobuf usages, only the descriptor information
is needed. However, there is a small percentage that legitimately needs
the Go type information. We should provide both, but document that the
descriptor-only information is preferred.

Change-Id: Ia0a098997fb1bd009994940ae8ea5257ccd87cae
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/184578
Reviewed-by: Damien Neil <dneil@google.com>
2019-07-12 23:43:21 +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
Damien Neil
e91877de26 internal/impl: add fast-path unmarshal
Benchmarks run with:
  go test ./benchmarks/ -bench=Wire  -benchtime=500ms -benchmem -count=8

Fast-path vs. parent commit:

  name                                      old time/op    new time/op    delta
  Wire/Unmarshal/google_message1_proto2-12    1.35µs ± 2%    0.45µs ± 4%  -67.01%  (p=0.000 n=8+8)
  Wire/Unmarshal/google_message1_proto3-12    1.07µs ± 1%    0.31µs ± 1%  -71.04%  (p=0.000 n=8+8)
  Wire/Unmarshal/google_message2-12            691µs ± 2%     188µs ± 2%  -72.78%  (p=0.000 n=7+8)

  name                                      old allocs/op  new allocs/op  delta
  Wire/Unmarshal/google_message1_proto2-12      60.0 ± 0%      25.0 ± 0%  -58.33%  (p=0.000 n=8+8)
  Wire/Unmarshal/google_message1_proto3-12      42.0 ± 0%       7.0 ± 0%  -83.33%  (p=0.000 n=8+8)
  Wire/Unmarshal/google_message2-12            28.6k ± 0%      8.5k ± 0%  -70.34%  (p=0.000 n=8+8)

Fast-path vs. -v1:

  name                                      old time/op    new time/op    delta
  Wire/Unmarshal/google_message1_proto2-12     702ns ± 1%     445ns ± 4%   -36.58%  (p=0.000 n=8+8)
  Wire/Unmarshal/google_message1_proto3-12     604ns ± 1%     311ns ± 1%   -48.54%  (p=0.000 n=8+8)
  Wire/Unmarshal/google_message2-12            179µs ± 3%     188µs ± 2%    +5.30%  (p=0.000 n=7+8)

  name                                      old allocs/op  new allocs/op  delta
  Wire/Unmarshal/google_message1_proto2-12      26.0 ± 0%      25.0 ± 0%    -3.85%  (p=0.000 n=8+8)
  Wire/Unmarshal/google_message1_proto3-12      8.00 ± 0%      7.00 ± 0%   -12.50%  (p=0.000 n=8+8)
  Wire/Unmarshal/google_message2-12            8.49k ± 0%     8.49k ± 0%    -0.01%  (p=0.000 n=8+8)

Change-Id: I6247ac3fd66a63d9acb902cbd192094ee3d151c3
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/185147
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-07-09 19:56:42 +00:00
Joe Tsai
c0e4bb2054 cmd/protoc-gen-go: unexport implementation-specific XXX fields
We modify protoc-gen-go to stop generating exported XXX fields.
The unsafe implementation is unaffected by this change since unsafe
can access fields regardless of visibility. However, for the purego
implementation, we need to respect Go visibility rules as enforced
by the reflect package.

We work around this by generating a exporter function that given
a reference to the message and the field to export, returns a reference
to the unexported field value. This exporter function is protected by
a constant such that it is not linked into the final binary in non-purego
build environment.

Updates golang/protobuf#276

Change-Id: Idf5c1f158973fa1c61187ff41440acb21c5dac94
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/185141
Reviewed-by: Damien Neil <dneil@google.com>
2019-07-08 20:45:09 +00:00
Joe Tsai
93fd968b71 internal/impl: centralize the logic for finding XXX fields
Change-Id: I177fbeeb474eeb86e4a47229ac5c48cb7191e95b
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/185140
Reviewed-by: Damien Neil <dneil@google.com>
2019-07-08 17:14:37 +00:00
Damien Neil
4ae30bbb21 internal/impl: refactor fast-path
Move data used by the fast-path implementations into a substructure of
MessageInfo and initialize it separately.

Change-Id: Ib855ee8ea5cb0379528b52ba0e191319aa5e2dff
Reviewed-on: https://go-review.googlesource.com/c/protobuf/+/184077
Reviewed-by: Joe Tsai <thebrokentoaster@gmail.com>
2019-06-27 20:08:19 +00:00