Make proto.Marshal use MarshalVT under the hood

[amaury1093]

This fast-marshaling is cool, but I would like to avoid having the VT suffix in my codebase, and would like to simply continue using proto.Marshal(...).

I read that protoreflect's Message supports an optional ProtoMethods method, and quoting the docs:

ProtoMethods returns optional fast-path implementions of various operations.

Is there a way vtprotobuf's fast-marshaling could be added to those methods, instead of new (Un)MarshalVT methods?

[steve-gray]

The article outlines the rationale for most of the reason this isn't a thing - specifically about sticking to a supported, exposed API. Overwriting/replacing existing methods was part of the reason that gogo ended up with such pain converting to the new API and is where it is.

https://vitess.io/blog/2021-06-03-a-new-protobuf-generator-for-go/

[amaury1093]

I see. protobuf-go's README states that it contains two pieces of code: the protobuf runtime library, and the protoc-gen-go generator.

I'm not aware of what gogo forked exactly, but looking at its repo I'm assuming it forked both the v1 runtime library and the v1 codegen. My line of thinking was that vtprotobuf could instead:

• use the official protobuf V2 runtime library (please don't fork that),
• but fork the protoc-gen-go generator to use custom ProtoMethods() with fast-marshaling.

The protoc-gen-go logic is not that complex, it's just string manipulation at the end of the day. I believe the trade-off of forking it (low maintenance) VS value brought to the community (high) is justified here. Thoughts?

[vmg]

@amaurym: the ProtoMethods API in the ProtoBuf message can only be used if, as you point out, we were to fork all of protoc-gen-go and override the metadata generation for the messages to use our fast APIs. For the initial version of vtprotobuf, we do not plan to fork the upstream generator. It's significantly more code than what I feel comfortable maintaining.

The ideal solution for this would be having a way to extend the generated code upstream so we could replace the callbacks in ProtoMethods. This is something I want to raise with the ProtoBuf maintainers, but until that's an option (or we find a serious performance optimization that can only be implemented by forking protoc-gen-go), the generated code will be opt-in. Cheers!

[amaury1093]

Makes sense, thanks.

The ideal solution for this would be having a way to extend the generated code upstream so we could replace the callbacks in ProtoMethods. This is something I want to raise with the ProtoBuf maintainers,

Yes, this is ideal. Would love a ping here once such an issue is created on the protobuf repo!