Stream copy default buffer size causes allocation to go to LOH.

[ManickaP]

The constant was picked up to prevent allocation in LOH:

runtime/src/libraries/System.Private.CoreLib/src/System/IO/Stream.cs

Lines 30 to 33 in f7d251f

	// We pick a value that is the largest multiple of 4096 that is still smaller than the large object heap threshold (85K).
	// The CopyTo/CopyToAsync buffer is short-lived and is likely to be collected at Gen0, and it offers a significant
	// improvement in Copy performance.
	private const int DefaultCopyBufferSize = 81920;

Then we refactored Stream.CopyTo* to use ArrayPool (commit from Oct 2016) which rents buffers in nearest, big enough, 2^n size:

runtime/src/libraries/System.Private.CoreLib/src/System/Buffers/Utilities.cs

Lines 17 to 22 in 50ac454

	// Buffers are bucketed so that a request between 2^(n-1) + 1 and 2^n is given a buffer of 2^n
	// Bucket index is log2(bufferSize - 1) with the exception that buffers between 1 and 16 bytes
	// are combined, and the index is slid down by 3 to compensate.
	// Zero is a valid bufferSize, and it is assigned the highest bucket index so that zero-length
	// buffers are not retained by the pool. The pool will return the Array.Empty singleton for these.
	return BitOperations.Log2((uint)bufferSize - 1 | 15) - 3;

This will lead to renting a buffer with the size of 131 072, falling over the LOH threshold.

I believe this side-effect was unintentional. The question is whether it's worth fixing since the buffer gets reused?

Discovered while investigating: dotnet/yarp#151

cc: @stephentoub

[Dotnet-GitSync-Bot]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[stephentoub]

Thanks, yeah, this was discussed here:
#37693 (comment)
After that I experimented with changing it, but decreasing the buffer size makes a measurable impact on throughout in some cases, and the buffer ends up getting pooled a lot of the time, so it wasn't clear that decreasing it was actually the right call.

[ManickaP]

I see, thanks for linking it.
We should at least get rid of the misleading comment above the DefaultCopyBufferSize.

I'm not sure if it's worth updating the constant to 128kB to reflect the actual size it gets from the ArrayPool. I guess the ArrayPool might change in the future and we could end up with even bigger unintentional allocations.

[dotnet-policy-service]

Due to lack of recent activity, this issue has been marked as a candidate for backlog cleanup. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will undo this process.

This process is part of our issue cleanup automation.

[dotnet-policy-service]

This issue will now be closed since it had been marked no-recent-activity but received no further activity in the past 14 days. It is still possible to reopen or comment on the issue, but please note that the issue will be locked if it remains inactive for another 30 days.