From 9dfc8544fc48f12c444d61f7517b7db35c14d354 Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Mon, 26 Jul 2021 13:18:30 -0400 Subject: [PATCH 1/3] Enable trimming TLS arrays in ArrayPool.Shared Today arrays stored in `ArrayPool.Shared`'s per-core buckets have trimming applied, but arrays stored in the per-thread buckets are only trimmed when there's high memory pressure. This change enables all buffers to be trimmed (eventually). Every time our gen2 callback runs, it ensures any non-timestamped buffers have a timestamp, and also ensures that any timestamped buffers are still timely... if any aren't, they're eligible for trimming. The timestamp is reset for TLS arrays when they're stored, and for per-core buckets when they transition from empty to non-empty; the latter is just a tweak on the current behavior, which incurs the cost of Environment.TickCount upon that transition, whereas now we only pay it as part of the trimming pass. --- .../tests/ArrayPool/CollectionTests.cs | 78 +++++-- .../TlsOverPerCoreLockedStacksArrayPool.cs | 221 ++++++++++++------ 2 files changed, 205 insertions(+), 94 deletions(-) diff --git a/src/libraries/System.Buffers/tests/ArrayPool/CollectionTests.cs b/src/libraries/System.Buffers/tests/ArrayPool/CollectionTests.cs index 6b39f5409bdea6..b4b9685ff887a9 100644 --- a/src/libraries/System.Buffers/tests/ArrayPool/CollectionTests.cs +++ b/src/libraries/System.Buffers/tests/ArrayPool/CollectionTests.cs @@ -23,48 +23,51 @@ public void BuffersAreCollectedWhenStale() const int BufferCount = 8; const int BufferSize = 1025; - // Get the pool and check our trim setting - var pool = ArrayPool.Shared; - List rentedBuffers = new List(); // Rent and return a set of buffers for (int i = 0; i < BufferCount; i++) { - rentedBuffers.Add(pool.Rent(BufferSize)); + rentedBuffers.Add(ArrayPool.Shared.Rent(BufferSize)); } for (int i = 0; i < BufferCount; i++) { - pool.Return(rentedBuffers[i]); + ArrayPool.Shared.Return(rentedBuffers[i]); } // Rent what we returned and ensure they are the same for (int i = 0; i < BufferCount; i++) { - var buffer = pool.Rent(BufferSize); + var buffer = ArrayPool.Shared.Rent(BufferSize); Assert.Contains(rentedBuffers, item => ReferenceEquals(item, buffer)); } for (int i = 0; i < BufferCount; i++) { - pool.Return(rentedBuffers[i]); + ArrayPool.Shared.Return(rentedBuffers[i]); + } + + // Trigger a few Gen2 GCs to make sure the pool has appropriately time stamped buffers. + for (int i = 0; i < 2; i++) + { + GC.Collect(); + GC.WaitForPendingFinalizers(); } // Now wait a little over a minute and force a GC to get some buffers returned Console.WriteLine("Waiting a minute for buffers to go stale..."); Thread.Sleep(61 * 1000); - GC.Collect(2); + GC.Collect(); GC.WaitForPendingFinalizers(); bool foundNewBuffer = false; for (int i = 0; i < BufferCount; i++) { - var buffer = pool.Rent(BufferSize); + var buffer = ArrayPool.Shared.Rent(BufferSize); if (!rentedBuffers.Any(item => ReferenceEquals(item, buffer))) { foundNewBuffer = true; } } - // Should only have found a new buffer if we're trimming Assert.True(foundNewBuffer); }, 3 * 60 * 1000); // This test has to wait for the buffers to go stale (give it three minutes) } @@ -78,21 +81,18 @@ public unsafe void ThreadLocalIsCollectedUnderHighPressure() { RemoteInvokeWithTrimming(() => { - // Get the pool and check our trim setting - var pool = ArrayPool.Shared; - // Create our buffer, return it, re-rent it and ensure we have the same one const int BufferSize = 4097; - var buffer = pool.Rent(BufferSize); - pool.Return(buffer); - Assert.Same(buffer, pool.Rent(BufferSize)); + byte[] buffer = ArrayPool.Shared.Rent(BufferSize); + ArrayPool.Shared.Return(buffer); + Assert.Same(buffer, ArrayPool.Shared.Rent(BufferSize)); // Return it and put memory pressure on to get it cleared - pool.Return(buffer); + ArrayPool.Shared.Return(buffer); const int AllocSize = 1024 * 1024 * 64; int PageSize = Environment.SystemPageSize; - var pressureMethod = pool.GetType().GetMethod("GetMemoryPressure", BindingFlags.Static | BindingFlags.NonPublic); + var pressureMethod = ArrayPool.Shared.GetType().GetMethod("GetMemoryPressure", BindingFlags.Static | BindingFlags.NonPublic); do { Span native = new Span(Marshal.AllocHGlobal(AllocSize).ToPointer(), AllocSize); @@ -109,10 +109,45 @@ public unsafe void ThreadLocalIsCollectedUnderHighPressure() GC.WaitForPendingFinalizers(); // Should have a new buffer now - Assert.NotSame(buffer, pool.Rent(BufferSize)); + Assert.NotSame(buffer, ArrayPool.Shared.Rent(BufferSize)); }); } + // This test can cause problems for other tests run in parallel (from other assemblies) as + // it pushes the physical memory usage above 80% temporarily. + [OuterLoop("This is a long running test (over 2 minutes)")] + [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + public unsafe void ThreadLocalIsCollectedUnderNormalPressure() + { + RemoteInvokeWithTrimming(() => + { + // Create our buffer, return it, re-rent it and ensure we have the same one + const int BufferSize = 4097; + byte[] buffer = ArrayPool.Shared.Rent(BufferSize); + ArrayPool.Shared.Return(buffer); + Assert.Same(buffer, ArrayPool.Shared.Rent(BufferSize)); + + // Return it and put memory pressure on to get it cleared + ArrayPool.Shared.Return(buffer); + + // Make sure buffer gets time stamped + for (int i = 0; i < 2; i++) + { + GC.Collect(); + GC.WaitForPendingFinalizers(); + } + + // Now wait for enough time to pass and force a GC to get buffers dropped + Console.WriteLine("Waiting a minute for buffers to go stale..."); + Thread.Sleep(61 * 1000); + GC.Collect(); + GC.WaitForPendingFinalizers(); + + // Should have a new buffer now + Assert.NotSame(buffer, ArrayPool.Shared.Rent(BufferSize)); + }, 3 * 60 * 1000); // This test has to wait for the buffers to go stale (give it three minutes) + } + private static bool IsPreciseGcSupportedAndRemoteExecutorSupported => PlatformDetection.IsPreciseGcSupported && RemoteExecutor.IsSupported; [ActiveIssue("https://github.com/dotnet/runtime/issues/44037")] @@ -121,9 +156,8 @@ public void PollingEventFires() { RemoteInvokeWithTrimming(() => { - var pool = ArrayPool.Shared; bool pollEventFired = false; - var buffer = pool.Rent(10); + float[] buffer = ArrayPool.Shared.Rent(10); // Polling doesn't start until the thread locals are created for a pool. // Try before the return then after. @@ -141,7 +175,7 @@ public void PollingEventFires() }); Assert.False(pollEventFired, "collection isn't hooked up until the first item is returned"); - pool.Return(buffer); + ArrayPool.Shared.Return(buffer); RunWithListener(() => { 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 fd4d242463c321..c38811038986c4 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Buffers/TlsOverPerCoreLockedStacksArrayPool.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Buffers/TlsOverPerCoreLockedStacksArrayPool.cs @@ -36,9 +36,9 @@ internal sealed partial class TlsOverPerCoreLockedStacksArrayPool : ArrayPool /// A per-thread array of arrays, to cache one array per array size per thread. [ThreadStatic] - private static T[]?[]? t_tlsBuckets; + private static ThreadLocalArray[]? t_tlsBuckets; /// Used to keep track of all thread local buckets for trimming if needed. - private readonly ConditionalWeakTable _allTlsBuckets = new ConditionalWeakTable(); + private readonly ConditionalWeakTable _allTlsBuckets = new ConditionalWeakTable(); /// /// An array of per-core array stacks. The slots are lazily initialized to avoid creating /// lots of overhead for unused array sizes. @@ -67,13 +67,13 @@ public override T[] Rent(int minimumLength) int bucketIndex = Utilities.SelectBucketIndex(minimumLength); // First, try to get an array from TLS if possible. - T[]?[]? tlsBuckets = t_tlsBuckets; + ThreadLocalArray[]? tlsBuckets = t_tlsBuckets; if (tlsBuckets is not null && (uint)bucketIndex < (uint)tlsBuckets.Length) { - buffer = tlsBuckets[bucketIndex]; + buffer = tlsBuckets[bucketIndex].Array; if (buffer is not null) { - tlsBuckets[bucketIndex] = null; + tlsBuckets[bucketIndex].Array = null; if (log.IsEnabled()) { log.BufferRented(buffer.GetHashCode(), buffer.Length, Id, bucketIndex); @@ -143,7 +143,7 @@ public override void Return(T[] array, bool clearArray = false) // this if the array being returned is erroneous or too large for the pool, but the // former condition is an error we don't need to optimize for, and the latter is incredibly // rare, given a max size of 1B elements. - T[]?[] tlsBuckets = t_tlsBuckets ?? InitializeTlsBucketsAndTrimming(); + ThreadLocalArray[] tlsBuckets = t_tlsBuckets ?? InitializeTlsBucketsAndTrimming(); bool haveBucket = false; bool returned = true; @@ -166,8 +166,9 @@ public override void Return(T[] array, bool clearArray = false) // Store the array into the TLS bucket. If there's already an array in it, // push that array down into the per-core stacks, preferring to keep the latest // one in TLS for better locality. - T[]? prev = tlsBuckets[bucketIndex]; - tlsBuckets[bucketIndex] = array; + ref ThreadLocalArray tla = ref tlsBuckets[bucketIndex]; + T[]? prev = tla.Array; + tla = new ThreadLocalArray(array); if (prev is not null) { PerCoreLockedStacks stackBucket = _buckets[bucketIndex] ?? CreatePerCoreLockedStacks(bucketIndex); @@ -194,43 +195,89 @@ public bool Trim() int milliseconds = Environment.TickCount; Utilities.MemoryPressure pressure = Utilities.GetMemoryPressure(); + // Log that we're trimming. ArrayPoolEventSource log = ArrayPoolEventSource.Log; if (log.IsEnabled()) { log.BufferTrimPoll(milliseconds, (int)pressure); } + // Trim each of the per-core buckets. PerCoreLockedStacks?[] perCoreBuckets = _buckets; for (int i = 0; i < perCoreBuckets.Length; i++) { - perCoreBuckets[i]?.Trim((uint)milliseconds, Id, pressure, Utilities.GetMaxSizeForBucket(i)); + perCoreBuckets[i]?.Trim(milliseconds, Id, pressure, Utilities.GetMaxSizeForBucket(i)); } + // Trim each of the TLS buckets. Note that threads may be modifying their TLS slots concurrently with + // this trimming happening. We do not force synchronization with those operations, so we accept the fact + // that we may end up firing a trimming event even if an array wasn't trimmed, and potentially + // trim an array we didn't need to. Both of these should be rare occurrences. + if (pressure == Utilities.MemoryPressure.High) { - // Under high pressure, release all thread locals - if (log.IsEnabled()) + // Under high pressure, release all thread locals. + if (!log.IsEnabled()) + { + foreach (KeyValuePair tlsBuckets in _allTlsBuckets) + { + Array.Clear(tlsBuckets.Key); + } + } + else { - foreach (KeyValuePair tlsBuckets in _allTlsBuckets) + foreach (KeyValuePair tlsBuckets in _allTlsBuckets) { - T[]?[] buckets = tlsBuckets.Key; + ThreadLocalArray[] buckets = tlsBuckets.Key; for (int i = 0; i < buckets.Length; i++) { - T[]? buffer = Interlocked.Exchange(ref buckets[i], null); - if (buffer is not null) + if (Interlocked.Exchange(ref buckets[i].Array, null) is T[] buffer) { - // As we don't want to take a perf hit in the rent path it - // is possible that a buffer could be rented as we "free" it. log.BufferTrimmed(buffer.GetHashCode(), buffer.Length, Id); } } } } - else + } + else + { + // Otherwise, release thread locals based on how long we've observed them to be stored. This time is + // approximate, with the time set not when the array is stored but when we see it during a Trim, so it + // takes at least two Trim calls (and thus two gen2 GCs) to drop an array, unless we're in high memory + // pressure. These values have been set arbitrarily; we could tune them in the future. + uint millisecondsThreshold = pressure switch { - foreach (KeyValuePair tlsBuckets in _allTlsBuckets) + Utilities.MemoryPressure.Medium => 15_000, + _ => 30_000, + }; + + foreach (KeyValuePair tlsBuckets in _allTlsBuckets) + { + ThreadLocalArray[] buckets = tlsBuckets.Key; + for (int i = 0; i < buckets.Length; i++) { - Array.Clear(tlsBuckets.Key); + if (buckets[i].Array is null) + { + continue; + } + + // We treat 0 to mean it hasn't yet been seen in a Trim call. In the very rare case where Trim records 0, + // it'll take an extra Trim call to remove the array. + int lastSeen = buckets[i].TimeStamp; + if (lastSeen == 0) + { + buckets[i].TimeStamp = milliseconds; + } + else if ((milliseconds < lastSeen) || (milliseconds - lastSeen) >= millisecondsThreshold) + { + // Time noticeably wrapped, or we've surpassed the threshold. + // Clear out the array, and log its being trimmed if desired. + if (Interlocked.Exchange(ref buckets[i].Array, null) is T[] buffer && + log.IsEnabled()) + { + log.BufferTrimmed(buffer.GetHashCode(), buffer.Length, Id); + } + } } } } @@ -238,11 +285,11 @@ public bool Trim() return true; } - private T[]?[] InitializeTlsBucketsAndTrimming() + private ThreadLocalArray[] InitializeTlsBucketsAndTrimming() { Debug.Assert(t_tlsBuckets is null); - T[]?[]? tlsBuckets = new T[NumBuckets][]; + var tlsBuckets = new ThreadLocalArray[NumBuckets]; t_tlsBuckets = tlsBuckets; _allTlsBuckets.Add(tlsBuckets, null); @@ -307,7 +354,7 @@ public bool TryPush(T[] array) return null; } - public void Trim(uint tickCount, int id, Utilities.MemoryPressure pressure, int bucketSize) + public void Trim(int tickCount, int id, Utilities.MemoryPressure pressure, int bucketSize) { LockedStack[] stacks = _perCoreStacks; for (int i = 0; i < stacks.Length; i++) @@ -320,9 +367,12 @@ public void Trim(uint tickCount, int id, Utilities.MemoryPressure pressure, int /// Provides a simple, bounded stack of arrays, protected by a lock. private sealed class LockedStack { + /// The arrays in the stack. private readonly T[]?[] _arrays = new T[MaxBuffersPerArraySizePerCore][]; + /// Number of arrays stored in . private int _count; - private uint _firstStackItemMS; + /// Timestamp set by Trim when it sees this as 0. + private int _timestamp; [MethodImpl(MethodImplOptions.AggressiveInlining)] public bool TryPush(T[] array) @@ -335,8 +385,9 @@ public bool TryPush(T[] array) { if (count == 0) { - // Stash the time the bottom of the stack was filled - _firstStackItemMS = (uint)Environment.TickCount; + // Reset the time stamp now that we're transitioning from empty to non-empty. + // Trim will see this as 0 and initialize it to the current time when Trim is called. + _timestamp = 0; } arrays[count] = array; @@ -364,11 +415,11 @@ public bool TryPush(T[] array) return arr; } - public void Trim(uint tickCount, int id, Utilities.MemoryPressure pressure, int bucketSize) + public void Trim(int tickCount, int id, Utilities.MemoryPressure pressure, int bucketSize) { - const uint StackTrimAfterMS = 60 * 1000; // Trim after 60 seconds for low/moderate pressure - const uint StackHighTrimAfterMS = 10 * 1000; // Trim after 10 seconds for high pressure - const uint StackRefreshMS = StackTrimAfterMS / 4; // Time bump after trimming (1/4 trim time) + const int StackTrimAfterMS = 60 * 1000; // Trim after 60 seconds for low/moderate pressure + const int StackHighTrimAfterMS = 10 * 1000; // Trim after 10 seconds for high pressure + const int StackRefreshMS = StackTrimAfterMS / 4; // Time bump after trimming (1/4 trim time) const int StackLowTrimCount = 1; // Trim one item when pressure is low const int StackMediumTrimCount = 2; // Trim two items when pressure is moderate const int StackHighTrimCount = MaxBuffersPerArraySizePerCore; // Trim all items when pressure is high @@ -381,63 +432,89 @@ public void Trim(uint tickCount, int id, Utilities.MemoryPressure pressure, int return; } - uint trimTicks = pressure == Utilities.MemoryPressure.High ? StackHighTrimAfterMS : StackTrimAfterMS; + int trimTicks = pressure == Utilities.MemoryPressure.High ? StackHighTrimAfterMS : StackTrimAfterMS; lock (this) { - if (_count > 0 && _firstStackItemMS > tickCount || (tickCount - _firstStackItemMS) > trimTicks) + if (_count == 0) { - // We've wrapped the tick count or elapsed enough time since the - // first item went into the stack. Drop the top item so it can - // be collected and make the stack look a little newer. + return; + } - ArrayPoolEventSource log = ArrayPoolEventSource.Log; - int trimCount = StackLowTrimCount; - switch (pressure) - { - case Utilities.MemoryPressure.High: - trimCount = StackHighTrimCount; - - // When pressure is high, aggressively trim larger arrays. - if (bucketSize > StackLargeBucket) - { - trimCount++; - } - if (Unsafe.SizeOf() > StackModerateTypeSize) - { - trimCount++; - } - if (Unsafe.SizeOf() > StackLargeTypeSize) - { - trimCount++; - } - break; - - case Utilities.MemoryPressure.Medium: - trimCount = StackMediumTrimCount; - break; - } + if (_timestamp == 0) + { + _timestamp = tickCount; + return; + } - while (_count > 0 && trimCount-- > 0) - { - T[]? array = _arrays[--_count]; - Debug.Assert(array is not null, "No nulls should have been present in slots < _count."); - _arrays[_count] = null; + if (_timestamp <= tickCount && (tickCount - _timestamp) <= trimTicks) + { + return; + } + + // We've wrapped the tick count or elapsed enough time since the + // first item went into the stack. Drop the top item so it can + // be collected and make the stack look a little newer. - if (log.IsEnabled()) + ArrayPoolEventSource log = ArrayPoolEventSource.Log; + int trimCount = StackLowTrimCount; + switch (pressure) + { + case Utilities.MemoryPressure.High: + trimCount = StackHighTrimCount; + + // When pressure is high, aggressively trim larger arrays. + if (bucketSize > StackLargeBucket) { - log.BufferTrimmed(array.GetHashCode(), array.Length, id); + trimCount++; } - } + if (Unsafe.SizeOf() > StackModerateTypeSize) + { + trimCount++; + } + if (Unsafe.SizeOf() > StackLargeTypeSize) + { + trimCount++; + } + break; - if (_count > 0 && _firstStackItemMS < uint.MaxValue - StackRefreshMS) + case Utilities.MemoryPressure.Medium: + trimCount = StackMediumTrimCount; + break; + } + + while (_count > 0 && trimCount-- > 0) + { + T[]? array = _arrays[--_count]; + Debug.Assert(array is not null, "No nulls should have been present in slots < _count."); + _arrays[_count] = null; + + if (log.IsEnabled()) { - // Give the remaining items a bit more time - _firstStackItemMS += StackRefreshMS; + log.BufferTrimmed(array.GetHashCode(), array.Length, id); } } + + _timestamp = _count > 0 ? + _timestamp + StackRefreshMS : // Give the remaining items a bit more time + 0; } } } + + /// Wrapper for arrays stored in ThreadStatic buckets. + private struct ThreadLocalArray + { + /// The stored array. + public T[]? Array; + /// Environment.TickCount timestamp for when this array was observed by Trim. + public int TimeStamp; + + public ThreadLocalArray(T[] array) + { + Array = array; + TimeStamp = 0; + } + } } } From de909a1834c060b72e7baf3bdbcb3f1d3c864acb Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Wed, 28 Jul 2021 10:51:42 -0400 Subject: [PATCH 2/3] Address PR feedback --- .../TlsOverPerCoreLockedStacksArrayPool.cs | 52 ++++++++----------- 1 file changed, 22 insertions(+), 30 deletions(-) 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 c38811038986c4..44d09634927fbb 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Buffers/TlsOverPerCoreLockedStacksArrayPool.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Buffers/TlsOverPerCoreLockedStacksArrayPool.cs @@ -21,12 +21,6 @@ namespace System.Buffers /// 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. - // - 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 = 27; // Utilities.SelectBucketIndex(1024 * 1024 * 1024 + 1) /// Maximum number of per-core stacks to use per array size. @@ -192,21 +186,21 @@ public override void Return(T[] array, bool clearArray = false) public bool Trim() { - int milliseconds = Environment.TickCount; + int currentMilliseconds = Environment.TickCount; Utilities.MemoryPressure pressure = Utilities.GetMemoryPressure(); // Log that we're trimming. ArrayPoolEventSource log = ArrayPoolEventSource.Log; if (log.IsEnabled()) { - log.BufferTrimPoll(milliseconds, (int)pressure); + log.BufferTrimPoll(currentMilliseconds, (int)pressure); } // Trim each of the per-core buckets. PerCoreLockedStacks?[] perCoreBuckets = _buckets; for (int i = 0; i < perCoreBuckets.Length; i++) { - perCoreBuckets[i]?.Trim(milliseconds, Id, pressure, Utilities.GetMaxSizeForBucket(i)); + perCoreBuckets[i]?.Trim(currentMilliseconds, Id, pressure, Utilities.GetMaxSizeForBucket(i)); } // Trim each of the TLS buckets. Note that threads may be modifying their TLS slots concurrently with @@ -263,12 +257,12 @@ public bool Trim() // We treat 0 to mean it hasn't yet been seen in a Trim call. In the very rare case where Trim records 0, // it'll take an extra Trim call to remove the array. - int lastSeen = buckets[i].TimeStamp; + int lastSeen = buckets[i].MillisecondsTimeStamp; if (lastSeen == 0) { - buckets[i].TimeStamp = milliseconds; + buckets[i].MillisecondsTimeStamp = currentMilliseconds; } - else if ((milliseconds < lastSeen) || (milliseconds - lastSeen) >= millisecondsThreshold) + else if ((currentMilliseconds - lastSeen) >= millisecondsThreshold) { // Time noticeably wrapped, or we've surpassed the threshold. // Clear out the array, and log its being trimmed if desired. @@ -287,7 +281,7 @@ public bool Trim() private ThreadLocalArray[] InitializeTlsBucketsAndTrimming() { - Debug.Assert(t_tlsBuckets is null); + Debug.Assert(t_tlsBuckets is null, $"Non-null {nameof(t_tlsBuckets)}"); var tlsBuckets = new ThreadLocalArray[NumBuckets]; t_tlsBuckets = tlsBuckets; @@ -354,12 +348,12 @@ public bool TryPush(T[] array) return null; } - public void Trim(int tickCount, int id, Utilities.MemoryPressure pressure, int bucketSize) + public void Trim(int currentMilliseconds, int id, Utilities.MemoryPressure pressure, int bucketSize) { LockedStack[] stacks = _perCoreStacks; for (int i = 0; i < stacks.Length; i++) { - stacks[i].Trim(tickCount, id, pressure, bucketSize); + stacks[i].Trim(currentMilliseconds, id, pressure, bucketSize); } } } @@ -372,7 +366,7 @@ private sealed class LockedStack /// Number of arrays stored in . private int _count; /// Timestamp set by Trim when it sees this as 0. - private int _timestamp; + private int _millisecondsTimestamp; [MethodImpl(MethodImplOptions.AggressiveInlining)] public bool TryPush(T[] array) @@ -387,7 +381,7 @@ public bool TryPush(T[] array) { // Reset the time stamp now that we're transitioning from empty to non-empty. // Trim will see this as 0 and initialize it to the current time when Trim is called. - _timestamp = 0; + _millisecondsTimestamp = 0; } arrays[count] = array; @@ -415,11 +409,10 @@ public bool TryPush(T[] array) return arr; } - public void Trim(int tickCount, int id, Utilities.MemoryPressure pressure, int bucketSize) + public void Trim(int currentMilliseconds, int id, Utilities.MemoryPressure pressure, int bucketSize) { const int StackTrimAfterMS = 60 * 1000; // Trim after 60 seconds for low/moderate pressure const int StackHighTrimAfterMS = 10 * 1000; // Trim after 10 seconds for high pressure - const int StackRefreshMS = StackTrimAfterMS / 4; // Time bump after trimming (1/4 trim time) const int StackLowTrimCount = 1; // Trim one item when pressure is low const int StackMediumTrimCount = 2; // Trim two items when pressure is moderate const int StackHighTrimCount = MaxBuffersPerArraySizePerCore; // Trim all items when pressure is high @@ -432,7 +425,7 @@ public void Trim(int tickCount, int id, Utilities.MemoryPressure pressure, int b return; } - int trimTicks = pressure == Utilities.MemoryPressure.High ? StackHighTrimAfterMS : StackTrimAfterMS; + int trimMilliseconds = pressure == Utilities.MemoryPressure.High ? StackHighTrimAfterMS : StackTrimAfterMS; lock (this) { @@ -441,20 +434,19 @@ public void Trim(int tickCount, int id, Utilities.MemoryPressure pressure, int b return; } - if (_timestamp == 0) + if (_millisecondsTimestamp == 0) { - _timestamp = tickCount; + _millisecondsTimestamp = currentMilliseconds; return; } - if (_timestamp <= tickCount && (tickCount - _timestamp) <= trimTicks) + if ((currentMilliseconds - _millisecondsTimestamp) <= trimMilliseconds) { return; } - // We've wrapped the tick count or elapsed enough time since the - // first item went into the stack. Drop the top item so it can - // be collected and make the stack look a little newer. + // We've elapsed enough time since the first item went into the stack. + // Drop the top item so it can be collected and make the stack look a little newer. ArrayPoolEventSource log = ArrayPoolEventSource.Log; int trimCount = StackLowTrimCount; @@ -495,8 +487,8 @@ public void Trim(int tickCount, int id, Utilities.MemoryPressure pressure, int b } } - _timestamp = _count > 0 ? - _timestamp + StackRefreshMS : // Give the remaining items a bit more time + _millisecondsTimestamp = _count > 0 ? + _millisecondsTimestamp + (trimMilliseconds / 4) : // Give the remaining items a bit more time 0; } } @@ -508,12 +500,12 @@ private struct ThreadLocalArray /// The stored array. public T[]? Array; /// Environment.TickCount timestamp for when this array was observed by Trim. - public int TimeStamp; + public int MillisecondsTimeStamp; public ThreadLocalArray(T[] array) { Array = array; - TimeStamp = 0; + MillisecondsTimeStamp = 0; } } } From 4ea83d2b4ede5c3487bb8f222dd8fe26a2b61dee Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Sat, 31 Jul 2021 06:53:23 -0400 Subject: [PATCH 3/3] Work around bad linker transform --- .../TlsOverPerCoreLockedStacksArrayPool.cs | 30 +++++++++++-------- 1 file changed, 18 insertions(+), 12 deletions(-) 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 44d09634927fbb..d38bda960e7528 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Buffers/TlsOverPerCoreLockedStacksArrayPool.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Buffers/TlsOverPerCoreLockedStacksArrayPool.cs @@ -208,17 +208,11 @@ public bool Trim() // that we may end up firing a trimming event even if an array wasn't trimmed, and potentially // trim an array we didn't need to. Both of these should be rare occurrences. + // Under high pressure, release all thread locals. if (pressure == Utilities.MemoryPressure.High) { - // Under high pressure, release all thread locals. - if (!log.IsEnabled()) - { - foreach (KeyValuePair tlsBuckets in _allTlsBuckets) - { - Array.Clear(tlsBuckets.Key); - } - } - else +#if !MONO // TODO https://github.com/mono/linker/issues/2181: Remove !MONO ifdefs in this method once is fixed + if (log.IsEnabled()) { foreach (KeyValuePair tlsBuckets in _allTlsBuckets) { @@ -232,6 +226,14 @@ public bool Trim() } } } + else +#endif + { + foreach (KeyValuePair tlsBuckets in _allTlsBuckets) + { + Array.Clear(tlsBuckets.Key); + } + } } else { @@ -266,10 +268,14 @@ public bool Trim() { // Time noticeably wrapped, or we've surpassed the threshold. // Clear out the array, and log its being trimmed if desired. - if (Interlocked.Exchange(ref buckets[i].Array, null) is T[] buffer && - log.IsEnabled()) + if (Interlocked.Exchange(ref buckets[i].Array, null) is T[] buffer) { - log.BufferTrimmed(buffer.GetHashCode(), buffer.Length, Id); +#if !MONO + if (log.IsEnabled()) + { + log.BufferTrimmed(buffer.GetHashCode(), buffer.Length, Id); + } +#endif } } }