Protobuf: APIv1: proto: direct fast-path for old messages

Created on 18 Feb 2020  路  3Comments  路  Source: golang/protobuf

As things currently stand, the v1.4.0 release of github.com/golang/protobuf/proto is going to use google.golang.org/protobuf/proto under the hood for unmarshaling and marshaling. To accomplish this, it uses the proto.MessageV2 function to convert any generated message to one that satisfies the v2 message interface.

If the message is recently generated, then it implements the interface and things are great. However, if it does not, then the MessageV2 wraps the message in some internal wrapper type so that it implements the v2 message interface. The process of wrapping involves allocating a 16B struct, and would occur for every top-level message being passed to unmarshal or marshal. This is a performance regression as it is an allocation that formerly did not exist.

To avoid this regression, we could hack v2 to expose low-level details of internal/impl, so that the v1 proto package can directly call the fast-path without allocating the stub structure. I didn't look too deeply into this, but I suspect that this is a non-trivial amount of work and addition of technical debt.

How much attention should we give to the performance of the new proto runtime working with old generated messages?

  • Long term, all messages should eventually be re-generated so that this won't matter. It's technical debt added to just support a temporary use-case.
  • Working on this will delay v2 more than it already is.
  • It's possible that the performance hit is offset by the v2 implementation actually being faster (I did not benchmark this).
  • My fear is that users will blindly benchmark v2 without regenerating their messages and claim that v2 is slower than v1, and write blog posts that no one should use it.

Most helpful comment

My vote is to do nothing to address this, but:

  • Highlight in the release notes that the v2 runtime should be use with messages generated by the v2 generator for best performance results.
  • If this is a sufficiently noticeable problem, we could do the performance hack I suggested in a hypothetical v1.4.1.

I prefer to not spend time on technical work that doesn't matter in the near future.

All 3 comments

My vote is to do nothing to address this, but:

  • Highlight in the release notes that the v2 runtime should be use with messages generated by the v2 generator for best performance results.
  • If this is a sufficiently noticeable problem, we could do the performance hack I suggested in a hypothetical v1.4.1.

I prefer to not spend time on technical work that doesn't matter in the near future.

I also think documenting and highlighting that users should migrate to the new v2 runtime and v2 generated code, and that the v1 wrapping has known (but limited) allocation regressions in order to retain compatibility.

My draft release notes documents this regression and explains that users should regenerate their proto files. Closing as "resolved". Can re-open if there's demonstrated need to take more action after release.

Was this page helpful?
0 / 5 - 0 ratings