Revert Series()/sync.Pool changes for now #4609
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix #4595 by reverting these changes for now. grpc-go does double-buffer i.e. SendMsg() can return before the whole message is on the wire hence it is impossible to share the buffers for now. http://www.golangdevops.com/2019/12/31/autopool/ has shown some promise but with finalizers the CPU usage is even higher and RAM usage is barely lower:
Relevant discussion: https://cloud-native.slack.com/archives/CL25937SP/p1629797444067700
What's interesting is that vtprotobuf's documentation says:
But I'm not sure how it could be safe to do that with when SendMsg() returns before putting the message on the wire: grpc/grpc-go#2159. So, another goroutine might write over a slice that is about to be sent :)
Thus, all of this needs quite a bit more research so revert these changes for now. But all of this shows that there are huge performance gains to be made here