In Distribtor, pre-allocate buffer for reading protobufs #1719
Conversation
ReadAll() will start at 512 bytes then successively double the buffer, copying the contents each time. If the caller supplies a size we can be much more efficient. Signed-off-by: Bryan Boreham <[email protected]>
bboreham
changed the title
+1 Do we have a benchmark that shows an improvement? |
|
Nope, no benchmark. In my prod distributors this |
|
This is the compressed data - if we could use a pool of buffers for the uncompressed bytes, that would be much bigger impact, but I'm not sure whether |
also +1 on this. |
Signed-off-by: Bryan Boreham <[email protected]>
And extend the test to check it fires. Signed-off-by: Bryan Boreham <[email protected]>
bboreham
force-pushed
the
dist-content-length
branch
from
3703686 to
99fd4d3
Compare
|
I added a size limit. |
| @@ -125,6 +125,7 @@ type Config struct { | |||
|
|
|||
| HATrackerConfig HATrackerConfig `yaml:"ha_tracker,omitempty"` | |||
|
|
|||
| MaxRecvMsgSize int `yaml:"max_send_msg_size"` | |||
There was a problem hiding this comment.
The yaml here is max_send_msg_size, while the CLI flag is max-recv-msg-size. Isn't contradictory?
There was a problem hiding this comment.
Good catch, could you open a PR/issue so that we don't lose track of this?
ReadAll() will start at 512 bytes then successively double the buffer, copying the contents each time. If the caller supplies a size we can be much more efficient. It's not a massive win, but every little helps.
(However, doing this creates a new DoS vector - supplying an fake size that is way too big. I'm thinking we should have a max input size, similar to gRPC)