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

Provide clarification on reusing serialization buffers #20

Open
GiedriusS opened this issue Aug 30, 2021 · 4 comments
Open

Provide clarification on reusing serialization buffers #20

GiedriusS opened this issue Aug 30, 2021 · 4 comments

Comments

@GiedriusS
Copy link

Documentation says:

MarshalToVT() ... This function is useful e.g. when using memory pooling to re-use serialization buffers.

Could you please provide clarification on how to use it? I'm asking because protobufs are typically used with gRPC, and grpc-go's SendMsg() returns before the buffer gets put on the wire. Hence, you cannot reuse it. Here is a relevant issue: grpc/grpc-go#2159. Someone has even attempted this http://www.golangdevops.com/2019/12/31/autopool/ but it's not a solution and the finalizers have poor performance. You could find even more details here thanos-io/thanos#4609.

Are there any examples of the usage of this function? I couldn't find anything.

If I am correct then the recommendation in the README seems dangerous.

@GiedriusS
Copy link
Author

Related: grpc/grpc-go#4715.

@nearmeng
Copy link

Same question, do you have the solution now?

@GiedriusS
Copy link
Author

GiedriusS commented Jan 17, 2024

No, I think you should not use gRPC or the gRPC Go implementation needs to be fixed if you care about pooling on the serialization side. There's some movement here: grpc/grpc-go#6619.

@vmg
Copy link
Member

vmg commented Jan 29, 2024

Hey, sorry I've never gotten to this issue before. I think @GiedriusS has already figured this out on his own, but for those reading: the recommendation in the README is unsafe if you're using pooling for the marshal side of protobuf (and I'm going to update it to clarify this). This is a known issue in the way GRPC is implemented in Go, because it keeps ownership of the buffers after they're passed to the RPC call. The tracking issue in grpc/grpc-go#6619 seems to be stalled again, which is slightly disappointing.

I can't think of any good ways to work around this issue until we get upstream changes in GRPC Go.

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

No branches or pull requests

3 participants