Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Connect: Use custom codec for vtproto #3310

Merged
merged 1 commit into from
May 24, 2024

Conversation

simonswine
Copy link
Contributor

@simonswine simonswine commented May 24, 2024

This implements a custom codec for use with connect-go. It uses the vtproto provided methods UnmarhsalVT rather than the upstream protobuf decoder/encoder.

@simonswine simonswine changed the title Use custom codec for vtproto within connect-go Connect: Use custom codec for vtproto May 24, 2024
@simonswine simonswine force-pushed the 20240524_use-custom-connect-go branch from 3832b72 to 95fc6f9 Compare May 24, 2024 10:12
@simonswine simonswine marked this pull request as ready for review May 24, 2024 13:25
@simonswine simonswine requested review from korniltsev and a team as code owners May 24, 2024 13:25
This implements a custom coded within connect-go.
@simonswine simonswine force-pushed the 20240524_use-custom-connect-go branch from 95fc6f9 to ad5711b Compare May 24, 2024 13:25
@simonswine
Copy link
Contributor Author

simonswine commented May 24, 2024

While I don't think this lead to significant shift in performance, it brings makes us use the benefits of vtprotobuf for the proto that are marshalled/unmarshalled from/for requests.

func (p *YourProto) MarshalVT() ([]byte, error): this function behaves identically to calling proto.Marshal(p), except the actual marshalling has been fully unrolled and does not use reflection or allocate memory. This function simply allocates a properly sized buffer by calling SizeVT on the message and then uses MarshalToSizedBufferVT to marshal to it.
unmarshal: generates a func (p *YourProto) UnmarshalVT(data []byte) that behaves similarly to calling proto.Unmarshal(data, p) on the message, except the unmarshalling is performed by unrolled codegen without using reflection and allocating as little memory as possible. If the receiver p is not fully zeroed-out, the unmarshal call will actually behave like proto.Merge(data, p). This is because the proto.Unmarshal in the ProtoBuf API is implemented by resetting the destination message and then calling proto.Merge on it. To ensure proper Unmarshal semantics, ensure you've called proto.Reset on your message before calling UnmarshalVT, or that your message has been newly allocated.

https://github.com/planetscale/vtprotobuf/blob/615f978279caa2965ae5d57c7d242f88b38e6802/README.md?plain=1#L21-L38

Copy link
Collaborator

@korniltsev korniltsev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

did you run any benchmark locally?

@simonswine
Copy link
Contributor Author

No, I only deployed it to the distributors in ops.

image

@simonswine simonswine merged commit 616e81a into grafana:main May 24, 2024
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants