From d4fcdc1240ec278980b4e63dda9769b76c1c802a Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Thu, 29 Jun 2023 09:56:51 -0400 Subject: [PATCH] Bump SharedArrayPool's max arrays per partition default from 8 to 32 (#87905) The 8 value was picked arbitrarily years ago, with a smaller value being picked because nothing was ever trimmed from the pool. Since then, we've seen a significant increase in use of the pool, putting pressure on its storage, and we also added trimming so that memory pressure causes arrays to be pitched. Longer term, we might want to remove this limit entirely and have more of a dynamic scheme for allowing the buckets to grow and shrink. For now, though, I'm bumping the limit up from 8 arrays per core to 32 arrays per core to provide some more wiggle room. 32 is also somewhat arbitrary, though recent examples on a few real services that were hitting the 8 limit (resulting in increased allocation and contention) were mollified by 32. --- .../tests/ArrayPool/UnitTests.cs | 22 +++++++------------ .../src/System/Buffers/SharedArrayPool.cs | 4 ++-- 2 files changed, 10 insertions(+), 16 deletions(-) diff --git a/src/libraries/System.Buffers/tests/ArrayPool/UnitTests.cs b/src/libraries/System.Buffers/tests/ArrayPool/UnitTests.cs index 87b8005e9b468..cc704c68fac13 100644 --- a/src/libraries/System.Buffers/tests/ArrayPool/UnitTests.cs +++ b/src/libraries/System.Buffers/tests/ArrayPool/UnitTests.cs @@ -18,6 +18,7 @@ namespace System.Buffers.ArrayPool.Tests public partial class ArrayPoolUnitTests : ArrayPoolTest { private const int MaxEventWaitTimeoutInMs = 200; + private const string MaxArraysPerPartitionDefault = "32"; private struct TestStruct { @@ -602,14 +603,14 @@ private static void ConfigurablePool_AllocatedArraysAreCleared() public static IEnumerable BytePoolInstances() { yield return new object[] { ArrayPool.Create() }; - yield return new object[] { ArrayPool.Create(1024*1024, 50) }; - yield return new object[] { ArrayPool.Create(1024*1024, 1) }; + yield return new object[] { ArrayPool.Create(1024 * 1024, 50) }; + yield return new object[] { ArrayPool.Create(1024 * 1024, 1) }; yield return new object[] { ArrayPool.Shared }; } [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] - [InlineData("", "", "2147483647", "8")] - [InlineData("0", "0", "2147483647", "8")] + [InlineData("", "", "2147483647", MaxArraysPerPartitionDefault)] + [InlineData("0", "0", "2147483647", MaxArraysPerPartitionDefault)] [InlineData("1", "2", "1", "2")] [InlineData("2", "1", "2", "1")] [InlineData("4", "123", "4", "123")] @@ -623,7 +624,7 @@ public static IEnumerable BytePoolInstances() " " + " " + " " + - "2" + + "2" + " " + " " + " " + @@ -638,7 +639,7 @@ public static IEnumerable BytePoolInstances() " " + " " + " ", - "2147483647", "8")] + "2147483647", MaxArraysPerPartitionDefault)] public void SharedPool_SetEnvironmentVariables_ValuesRespected( string partitionCount, string maxArraysPerPartition, string expectedPartitionCount, string expectedMaxArraysPerPartition) { @@ -669,14 +670,7 @@ public void SharedPool_SetEnvironmentVariables_ValuesRespected( FieldInfo maxArraysPerPartitionField = staticsType.GetField("s_maxArraysPerPartition", BindingFlags.NonPublic | BindingFlags.Static); Assert.NotNull(maxArraysPerPartitionField); int maxArraysPerPartitionValue = (int)maxArraysPerPartitionField.GetValue(null); - if (int.Parse(expectedMaxArraysPerPartition) > 0) - { - Assert.Equal(int.Parse(expectedMaxArraysPerPartition), maxArraysPerPartitionValue); - } - else - { - Assert.Equal(8, maxArraysPerPartitionValue); - } + Assert.Equal(int.Parse(int.Parse(expectedMaxArraysPerPartition) > 0 ? expectedMaxArraysPerPartition : MaxArraysPerPartitionDefault), maxArraysPerPartitionValue); // Make sure the pool is still usable for (int i = 0; i < 2; i++) diff --git a/src/libraries/System.Private.CoreLib/src/System/Buffers/SharedArrayPool.cs b/src/libraries/System.Private.CoreLib/src/System/Buffers/SharedArrayPool.cs index 47115b431efc3..bfa50207dc3df 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Buffers/SharedArrayPool.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Buffers/SharedArrayPool.cs @@ -536,12 +536,12 @@ private static int GetPartitionCount() } /// Gets the maximum number of arrays of a given size allowed to be cached per partition. - /// Defaults to 8. This does not factor in or impact the number of arrays cached per thread in TLS (currently only 1). + /// Defaults to 32. This does not factor in or impact the number of arrays cached per thread in TLS (currently only 1). private static int GetMaxArraysPerPartition() { return TryGetInt32EnvironmentVariable("DOTNET_SYSTEM_BUFFERS_SHAREDARRAYPOOL_MAXARRAYSPERPARTITION", out int result) && result > 0 ? result : - 8; // arbitrary limit + 32; // arbitrary limit } /// Look up an environment variable and try to parse it as an Int32.