From d8669c7c846dbb43b14a497ab6cda4449514f461 Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Tue, 13 Jul 2021 23:53:51 -0400 Subject: [PATCH] Raise the max size poolable by ArrayPool We previously set an arbitrary cut-off of 2MB max array size. That was before we would trim arrays in response to memory pressure. This now raises the limit as high as we can while maintaining the current power-of-two-based scheme. --- .../tests/ArrayPool/UnitTests.cs | 47 +++++++++++++++---- .../Compression/WebSocketDeflater.cs | 6 +-- .../Compression/WebSocketInflater.cs | 5 +- .../TlsOverPerCoreLockedStacksArrayPool.cs | 3 +- .../System/Globalization/CompareInfo.Icu.cs | 4 +- 5 files changed, 45 insertions(+), 20 deletions(-) diff --git a/src/libraries/System.Buffers/tests/ArrayPool/UnitTests.cs b/src/libraries/System.Buffers/tests/ArrayPool/UnitTests.cs index 7456e1b46fe68e..5673d22d363935 100644 --- a/src/libraries/System.Buffers/tests/ArrayPool/UnitTests.cs +++ b/src/libraries/System.Buffers/tests/ArrayPool/UnitTests.cs @@ -8,6 +8,7 @@ using System.Numerics; using System.Threading.Tasks; using Microsoft.DotNet.RemoteExecutor; +using Microsoft.DotNet.XUnitExtensions; using Xunit; namespace System.Buffers.ArrayPool.Tests @@ -240,15 +241,11 @@ public static void NewDefaultArrayPoolWithSmallBufferSizeRoundsToOurSmallestSupp } [Fact] - public static void ReturningABufferGreaterThanMaxSizeDoesNotThrow() + public static void ReturningToCreatePoolABufferGreaterThanMaxSizeDoesNotThrow() { ArrayPool pool = ArrayPool.Create(maxArrayLength: 16, maxArraysPerBucket: 1); byte[] rented = pool.Rent(32); pool.Return(rented); - - ArrayPool.Shared.Return(new byte[3 * 1024 * 1024]); - ArrayPool.Shared.Return(new char[3 * 1024 * 1024]); - ArrayPool.Shared.Return(new string[3 * 1024 * 1024]); } [Fact] @@ -292,11 +289,11 @@ public static void CanRentManySizedBuffers(ArrayPool pool) [InlineData(1024, 1024)] [InlineData(4096, 4096)] [InlineData(1024 * 1024, 1024 * 1024)] - [InlineData(1024 * 1024 + 1, 1024 * 1024 + 1)] + [InlineData(1024 * 1024 + 1, 1024 * 1024 * 2)] [InlineData(1024 * 1024 * 2, 1024 * 1024 * 2)] public static void RentingSpecificLengthsYieldsExpectedLengths(int requestedMinimum, int expectedLength) { - foreach (ArrayPool pool in new[] { ArrayPool.Create(), ArrayPool.Shared }) + foreach (ArrayPool pool in new[] { ArrayPool.Create((int)BitOperations.RoundUpToPowerOf2((uint)requestedMinimum), 1), ArrayPool.Shared }) { byte[] buffer1 = pool.Rent(requestedMinimum); byte[] buffer2 = pool.Rent(requestedMinimum); @@ -313,7 +310,7 @@ public static void RentingSpecificLengthsYieldsExpectedLengths(int requestedMini pool.Return(buffer1); } - foreach (ArrayPool pool in new[] { ArrayPool.Create(), ArrayPool.Shared }) + foreach (ArrayPool pool in new[] { ArrayPool.Create((int)BitOperations.RoundUpToPowerOf2((uint)requestedMinimum), 1), ArrayPool.Shared }) { char[] buffer1 = pool.Rent(requestedMinimum); char[] buffer2 = pool.Rent(requestedMinimum); @@ -330,7 +327,7 @@ public static void RentingSpecificLengthsYieldsExpectedLengths(int requestedMini pool.Return(buffer1); } - foreach (ArrayPool pool in new[] { ArrayPool.Create(), ArrayPool.Shared }) + foreach (ArrayPool pool in new[] { ArrayPool.Create((int)BitOperations.RoundUpToPowerOf2((uint)requestedMinimum), 1), ArrayPool.Shared }) { string[] buffer1 = pool.Rent(requestedMinimum); string[] buffer2 = pool.Rent(requestedMinimum); @@ -348,6 +345,38 @@ public static void RentingSpecificLengthsYieldsExpectedLengths(int requestedMini } } + [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.Is64BitProcess))] + [InlineData(1024 * 1024 * 1024 - 1, true)] + [InlineData(1024 * 1024 * 1024, true)] + [InlineData(1024 * 1024 * 1024 + 1, false)] + [InlineData(0X7FFFFFC7 /* Array.MaxLength */, false)] + [OuterLoop] + public static void RentingGiganticArraySucceeds(int length, bool expectPooled) + { + var options = new RemoteInvokeOptions(); + options.StartInfo.UseShellExecute = false; + options.StartInfo.EnvironmentVariables.Add(TrimSwitchName, "false"); + + RemoteExecutor.Invoke((lengthStr, expectPooledStr) => + { + int length = int.Parse(lengthStr); + byte[] array; + try + { + array = ArrayPool.Shared.Rent(length); + } + catch (OutOfMemoryException) + { + return; + } + + Assert.InRange(array.Length, length, int.MaxValue); + ArrayPool.Shared.Return(array); + + Assert.Equal(bool.Parse(expectPooledStr), ReferenceEquals(array, ArrayPool.Shared.Rent(length))); + }, length.ToString(), expectPooled.ToString(), options).Dispose(); + } + [Fact] public static void RentingAfterPoolExhaustionReturnsSizeForCorrespondingBucket_SmallerThanLimit() { diff --git a/src/libraries/System.Net.WebSockets/src/System/Net/WebSockets/Compression/WebSocketDeflater.cs b/src/libraries/System.Net.WebSockets/src/System/Net/WebSockets/Compression/WebSocketDeflater.cs index e7f18072842433..f3ecf278886d34 100644 --- a/src/libraries/System.Net.WebSockets/src/System/Net/WebSockets/Compression/WebSocketDeflater.cs +++ b/src/libraries/System.Net.WebSockets/src/System/Net/WebSockets/Compression/WebSocketDeflater.cs @@ -46,15 +46,11 @@ public ReadOnlySpan Deflate(ReadOnlySpan payload, bool endOfMessage) { Debug.Assert(_buffer is null, "Invalid state, ReleaseBuffer not called."); - // Do not try to rent more than 1MB initially, because it will actually allocate - // instead of renting. Be optimistic that what we're sending is actually going to fit. - const int MaxInitialBufferLength = 1024 * 1024; - // For small payloads there might actually be overhead in the compression and the resulting // output might be larger than the payload. This is why we rent at least 4KB initially. const int MinInitialBufferLength = 4 * 1024; - _buffer = ArrayPool.Shared.Rent(Math.Clamp(payload.Length, MinInitialBufferLength, MaxInitialBufferLength)); + _buffer = ArrayPool.Shared.Rent(Math.Max(payload.Length, MinInitialBufferLength)); int position = 0; while (true) diff --git a/src/libraries/System.Net.WebSockets/src/System/Net/WebSockets/Compression/WebSocketInflater.cs b/src/libraries/System.Net.WebSockets/src/System/Net/WebSockets/Compression/WebSocketInflater.cs index 6ade12d539a440..d47e646fa60ea6 100644 --- a/src/libraries/System.Net.WebSockets/src/System/Net/WebSockets/Compression/WebSocketInflater.cs +++ b/src/libraries/System.Net.WebSockets/src/System/Net/WebSockets/Compression/WebSocketInflater.cs @@ -82,10 +82,9 @@ public void Prepare(long payloadLength, int userBufferLength) } else { - // Rent a buffer as close to the size of the user buffer as possible, - // but not try to rent anything above 1MB because the array pool will allocate. + // Rent a buffer as close to the size of the user buffer as possible. // If the payload is smaller than the user buffer, rent only as much as we need. - _buffer = ArrayPool.Shared.Rent(Math.Min(userBufferLength, (int)Math.Min(payloadLength, 1024 * 1024))); + _buffer = ArrayPool.Shared.Rent((int)Math.Min(userBufferLength, payloadLength)); } } diff --git a/src/libraries/System.Private.CoreLib/src/System/Buffers/TlsOverPerCoreLockedStacksArrayPool.cs b/src/libraries/System.Private.CoreLib/src/System/Buffers/TlsOverPerCoreLockedStacksArrayPool.cs index 59cb6556ad7adc..438ca8e9cb2046 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Buffers/TlsOverPerCoreLockedStacksArrayPool.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Buffers/TlsOverPerCoreLockedStacksArrayPool.cs @@ -24,12 +24,11 @@ internal sealed partial class TlsOverPerCoreLockedStacksArrayPool : ArrayPool // TODO https://github.com/dotnet/coreclr/pull/7747: "Investigate optimizing ArrayPool heuristics" // - Explore caching in TLS more than one array per size per thread, and moving stale buffers to the global queue. // - Explore changing the size of each per-core bucket, potentially dynamically or based on other factors like array size. - // - Explore changing number of buckets and what sizes of arrays are cached. // - Investigate whether false sharing is causing any issues, in particular on LockedStack's count and the contents of its array. // ... /// The number of buckets (array sizes) in the pool, one for each array length, starting from length 16. - private const int NumBuckets = 17; // Utilities.SelectBucketIndex(2*1024*1024) + private const int NumBuckets = 27; // Utilities.SelectBucketIndex(1024 * 1024 * 1024 + 1) /// Maximum number of per-core stacks to use per array size. private const int MaxPerCorePerArraySizeStacks = 64; // selected to avoid needing to worry about processor groups /// The maximum number of buffers to store in a bucket's global queue. diff --git a/src/libraries/System.Private.CoreLib/src/System/Globalization/CompareInfo.Icu.cs b/src/libraries/System.Private.CoreLib/src/System/Globalization/CompareInfo.Icu.cs index 4d27d9e881e493..a9b9274952ae0a 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Globalization/CompareInfo.Icu.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Globalization/CompareInfo.Icu.cs @@ -722,7 +722,9 @@ private unsafe int IcuGetHashCodeOfString(ReadOnlySpan source, CompareOpti // according to ICU User Guide the performance of ucol_getSortKey is worse when it is called with null output buffer // the solution is to try to fill the sort key in a temporary buffer of size equal 4 x string length - // 1MB is the biggest array that can be rented from ArrayPool.Shared without memory allocation + // (The ArrayPool used to have a limit on the length of buffers it would cache; this code was avoiding + // exceeding that limit to avoid a per-operation allocation, and the performance implications here + // were not re-evaluated when the limit was lifted.) int sortKeyLength = (source.Length > 1024 * 1024 / 4) ? 0 : 4 * source.Length; byte[]? borrowedArray = null;