perf(storage): gRPC zerocopy codec#10888
Conversation
3392f73 to
5a1e3a1
Compare
Replace the current custom codec for ReadObjectResponse with a CodecV2 that can handle data split across multiple buffers. Unit tests pass but need to finish up some stuff to get the end-to-end working.
5a1e3a1 to
099a5e9
Compare
| // Consume a bytes field from the input. Returns offsets for the data in the buffer slices | ||
| // and an error. | ||
| func (d *readResponseDecoder) consumeBytes() (*bufferSliceOffsets, error) { | ||
| b := (*d.databufs)[d.currBuf].ReadOnlyData() |
There was a problem hiding this comment.
nit, this function only seems to require (*d.databufs)[d.currBuf].Len().
BrennaEpp
left a comment
There was a problem hiding this comment.
Couple initial comments, looks like there is some commented out code as well.
0723415 to
20e4a84
Compare
| // Consume a varint that represents the length of a bytes field. Return the length of | ||
| // the data, and advance the offsets by the length of the varint. | ||
| func (d *readResponseDecoder) consumeVarint() (uint64, error) { | ||
| b := (*d.databufs)[d.currBuf].ReadOnlyData() |
There was a problem hiding this comment.
I wonder if it's be worth stashing the current buffer's []byte in readResponseDecoder, to avoid the need for repeated calls to ReadOnlyData.
Or perhaps even stash a [][]byte and materialize all the buffers once at the start of decoding. You could avoid an allocation in the common case by keeping a [N][]byte in the decoder:
type readResponseDecoder struct {
bufs [][]byte
staticBufs [4][]byte
}
func (d *readResponseDecoder) init() {
d.bufs = d.staticBufs[:0]
for _, b := range d.databufs {
d.bufs = append(d.bufs, b.ReadOnlyData())
}
}
There was a problem hiding this comment.
I don't fully understand this, why would staticBufs be of len 4?
I think I'll probably just leave this for now if that's okay, I don't see overhead from these calls.
94062c4 to
66bc174
Compare
| currOff := d.currOff | ||
| var buf []byte | ||
| for remaining > 0 { | ||
| b := d.databufs[currBuf].ReadOnlyData() |
There was a problem hiding this comment.
Could simplify the code a tiny bit by making b start at the current offset:
b := b.databufs[currBuf].ReadOnlyData()[currOff:]
if len(b) < remaining {
buf = append(buf, b...)
remaining -= len(b)
currBuf++
currOff = 0
} else {
buf = append(buf, b[:remaining]...)
remaining = 0
}
| d.currOff += remaining | ||
| remaining = 0 | ||
| } | ||
| } |
There was a problem hiding this comment.
Might be a bit simpler as:
d.currOff += n
for d.currBuf < len(d.databufs) && d.currOff >= d.databufs[d.currBuf].Len() {
d.currOff -= d.databufs[d.currBuf].Len()
d.currBuf++
}
Replace the current custom codec for ReadObjectResponse with a CodecV2 that can handle data split across multiple buffers.
Unit tests pass but need to finish up some stuff to get the end-to-end working.