Skip to content
This repository was archived by the owner on Dec 18, 2018. It is now read-only.

Use pooled memory for Stream CopyToAsync #447

Merged
merged 2 commits into from
Dec 4, 2015

Conversation

benaadams
Copy link
Contributor

Default buffer size Is crazy large (82kB); and as is used for lifetime of connection; per connection - likely ends up in Gen2

(Ref https://github.com/dotnet/coreclr/issues/2223)

@benaadams benaadams changed the title Smaller default buffer Use pooled memory for filtered Stream Dec 3, 2015
@benaadams benaadams changed the title Use pooled memory for filtered Stream Use pooled memory for filtered Stream Copy Dec 3, 2015
@benaadams benaadams changed the title Use pooled memory for filtered Stream Copy Use pooled memory for Stream CopyToAsync Dec 3, 2015
@halter73 halter73 merged commit 1ca6769 into aspnet:dev Dec 4, 2015
{
var returnedBlock = task.Result;
returnedBlock.Pool?.Return(returnedBlock);
Copy link
Member

Choose a reason for hiding this comment

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

Why can the pool be null?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If SocketInput asks for blocks larger than a pooled block; and then returns them to pool - had a related PR #402 to stop it; wasn't sure - too much churn at the time.

Copy link
Member

Choose a reason for hiding this comment

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

In this case, since we aren't providing a minimum size to memory.Lease(), the pool should never be null.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Weird; thought non-pooled blocks could get returned; but looks like that changed way back...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added PR #448 to address this; re-closed the only use pooled blocks PR

@benaadams benaadams deleted the don't-use-default-buffer-size branch December 4, 2015 13:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants