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

Make PooledArrayBufferWriter more versatile, rename #8453

Merged
merged 1 commit into from Jun 1, 2023

Conversation

ReubenBond
Copy link
Member

@ReubenBond ReubenBond commented May 25, 2023

Supports slicing, for example

Microsoft Reviewers: Open in CodeFlow

@ReubenBond ReubenBond mentioned this pull request May 25, 2023
5 tasks
@ReubenBond ReubenBond merged commit bcf87d2 into dotnet:main Jun 1, 2023
18 checks passed
@ReubenBond ReubenBond deleted the perf/pooledbuffer/1 branch June 1, 2023 13:11

public void Deserialize<TInput>(ref Reader<TInput> reader, scoped ref PooledBuffer value)
{
const int MaxSpanLength = 4096;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why such a small buffer size? Wouldn't something like 1MB (or even bigger) be more appropriate? There's no reason to prefer small buffers when they are pooled.

Copy link
Member Author

@ReubenBond ReubenBond Jun 6, 2023

Choose a reason for hiding this comment

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

Each buffer is owned by at most one instance at a time, so if the buffers are individually large and there are many instances, then the memory footprint of the application may become unnecessarily large. Ideally, this should be changed to have adaptive buffer sizes (eg powers of 2 from 256b to 1MB), maybe with an initial size hint (defaulting to 4k, perhaps)

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.

None yet

2 participants