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

[chunkwriting] Return original buffer to the pool #284

Merged
merged 2 commits into from
Apr 27, 2022

Conversation

kfarnung
Copy link
Contributor

@kfarnung kfarnung commented Jul 9, 2021

When the buffer isn't filled to capacity a new slice is created that's
smaller than the original. In that case the smaller slice is returned to
the pool which prevents the rest of the capacity from being used again.

The solution is to pass the original slice through and attach the
length. This allows the original slice to be returned to the pool once
the operation is complete.

This change also simplifies the sendChunk method, ensuring that the
buffer is returned to the TransferManager even when no bytes were read
from the reader.

When the buffer isn't filled to capacity a new slice is created that's
smaller than the original. In that case the smaller slice is returned to
the pool which prevents the rest of the capacity from being used again.

The solution is to pass the original slice through and attach the
length. This allows the original slice to be returned to the pool once
the operation is complete.

This change also simplifies the `sendChunk` method, ensuring that the
buffer is returned to the TransferManager even when no bytes were read
from the reader.
@serprex
Copy link
Contributor

serprex commented Nov 3, 2021

Copy link
Contributor

@mohsha-msft mohsha-msft left a comment

Choose a reason for hiding this comment

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

Ohh I get what you're trying to do. Thanks for the contribution!

@zezha-msft zezha-msft merged commit 69bf9eb into Azure:dev Apr 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants