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 incorrectly configured recyclable memory stream pool sizes #225

Merged
merged 1 commit into from
Oct 18, 2022

Conversation

adam-mccoy
Copy link
Contributor

@adam-mccoy adam-mccoy commented Oct 18, 2022

Background

The NewtonsoftDocumentSerializer makes use of a RecycableMemoryStreamManager to efficiently use and re-use memory streams. However, the manager is configured in such a way as to be quite inefficient with memory.

On construction, the RecyclableMemoryStreamManager takes three arguments:

  • Block Size
  • Large Buffer Multiple
  • Maximum Large Buffer Size

The last two are of interest. For pooling large buffers of memory, a series of pools is created with varying sizes. These sizes are (1 * Large Buffer Multiple), (2 * Large Buffer Multiple), etc. up to the Maximum Large Buffer Size. The idea being to have a fairly small variety of sizes to choose from, with the smaller pools being more densely populated than the larger pools.

Currently, the NewtonsoftDocumentSerializer uses 1MB for the Maximum Large Buffer Size and 4 bytes for the Large Buffer Multiple. This means that there are 262,144 pools created, each one 4 bytes larger than the previous. Having this amount of variance in pool sizes is not helpful, and is in fact quite costly. Around 12MB of memory is used just to track the pools.

Results

Change the Large Buffer Multiple to 256kB and increase the Maximum Large Buffer Size to 2MB. This will result in the following 8 large buffer pools:

  • 256kB
  • 512kB
  • 768kB
  • 1,024kB (1MB)
  • 1,280kB (~1.25MB)
  • 1,536kB (~1.5MB)
  • 1,792kB (~1.75MB)
  • 2,048kB (2MB)

@adam-mccoy adam-mccoy marked this pull request as ready for review October 18, 2022 01:06
@adam-mccoy adam-mccoy requested a review from a team October 18, 2022 01:06
@shortcut-integration
Copy link

Copy link
Contributor

@skermajo skermajo left a comment

Choose a reason for hiding this comment

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

Thanks, all makes sense 👍 Was 256 chosen for a particular reason?

@adam-mccoy
Copy link
Contributor Author

Was 256 chosen for a particular reason?

There's not a great deal of science behind it. I wanted to land at 2MB for the upper limit, which I believe to be generous but not excessive. From there, having 8 different pool sizes seemed like plenty, therefore increments of 256kB.

@adam-mccoy adam-mccoy merged commit e2f91ea into master Oct 18, 2022
@adam-mccoy adam-mccoy deleted the bug/buffer-sizes branch October 18, 2022 03:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants