pkg/proto: adopt CodecV2 and gRPC buffer pooling #2070
Conversation
|
@jzelinskie We have quite a bit of work left before the Vitess PR is green... I would really like to see how these changes affect spicedb benchmarks. Do you have a plan to benchmark this branch? Cheers! |
jzelinskie
force-pushed
the
grpc-codecv2
branch
3 times, most recently
from
ec25c3f
to
1a0d8be
Compare
pkg/proto/dispatch/v1/01_codec.go
Outdated
| size := m.SizeVT() | ||
| if mem.IsBelowBufferPoolingThreshold(size) { | ||
| buf := make([]byte, size) | ||
| n, err := m.MarshalToSizedBufferVT(buf[:size]) |
There was a problem hiding this comment.
if we're allocating the buffer as size of size, why do we need to slice here and below?
There was a problem hiding this comment.
I cleaned this up a bit to try and make it more clear (and after checking guarantees from the pool implementations).
I'm being defensive for the scenario where the number of bytes written are not actually the exact size of the buffer, which looks possible based on the generated code.
jzelinskie
force-pushed
the
grpc-codecv2
branch
from
1a0d8be
to
9010087
Compare
We're getting out 2 stable releases of SpiceDB and then looking to begin testing this in some different staging environments to see how it goes. |
jzelinskie
force-pushed
the
grpc-codecv2
branch
from
9010087
to
bf4f425
Compare
jzelinskie
force-pushed
the
grpc-codecv2
branch
from
bf4f425
to
44372c1
Compare
This replaces our existing gRPC codec (which enabled us to use vtprotobuf) with the new v2 codec interface that can support buffer pooling.
Huge thanks to engineers from Google and LinkedIn for fleshing out this work and upstreaming the necessary changes. There was a great talk from gRPConf I can share once it's uploaded to YouTube.
I did regenerate our go.mod which combined all of the indirect dependencies into one block. If there were purposely split out, I can fix it.
See the sister PR to this introducing the Codec for Vitess: vitessio/vitess#16790