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

Fix LengthDelimitedCodec buffer over-reservation. #4997

Merged
merged 1 commit into from
Sep 11, 2022
Merged

Fix LengthDelimitedCodec buffer over-reservation. #4997

merged 1 commit into from
Sep 11, 2022

Conversation

yotamofek
Copy link
Contributor

The LengthDelimitedCodec calls BytesMut::reserve on two occasions:

  • after the header is read, to allocate memory for the expected frame
  • after a frame is read, to allocate memory for another header that may follow

In both cases, the implementation currently calls src.reserve(n) which reserves the necessary n bytes for additional data, disregarding the fact that the buffer might already have additional data present that just wasn't decoded yet.

Motivation

This causes excess memory allocation, which might be quite substantial in certain cases.

Solution

Take into account the buffer's current length when calculating the amount of memory that should be reserved.

As a small drive-by cleanup, I also removed a check that n != 0 before calling to src.advance(n), since that is redundant according to the docs:

A call with cnt == 0 should never panic and be a no-op.

Not sure if/how to write tests for this, any feedback would be welcome.

@Darksonn Darksonn added A-tokio-util Area: The tokio-util crate M-codec Module: tokio-util/codec labels Sep 11, 2022
Copy link
Contributor

@Darksonn Darksonn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.

@Darksonn Darksonn merged commit 1c82309 into tokio-rs:master Sep 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio-util Area: The tokio-util crate M-codec Module: tokio-util/codec
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants