Skip to content

Commit

Permalink
Remove some dead code from SharedArrayPool (dotnet#101410)
Browse files Browse the repository at this point in the history 
In Trim, for high memory pressure, we're already setting the number of arrays to trim equal to the max number possible, so the subsequent logic that would increase the count wasn't useful. Deleting that also deleted the last use of _elementSize, which in turn allowed for the path that provided that value to be cleaned up.
  • Loading branch information
@stephentoub @matouskozak
stephentoub authored and matouskozak committed Apr 30, 2024
1 parent c5304c2 commit 9b04188
Showing 1 changed file with 15 additions and 53 deletions.
68 changes: 15 additions & 53 deletions src/libraries/System.Private.CoreLib/src/System/Buffers/SharedArrayPool.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,7 @@ internal sealed partial class SharedArrayPool<T> : ArrayPool<T>
/// <summary>Allocate a new <see cref="SharedArrayPoolPartitions"/> and try to store it into the <see cref="_buckets"/> array.</summary>
private unsafe SharedArrayPoolPartitions CreatePerCorePartitions(int bucketIndex)
{
#pragma warning disable 8500 // sizeof of managed types
int elementSize = sizeof(T);
#pragma warning restore 8500
var inst = new SharedArrayPoolPartitions(elementSize);
var inst = new SharedArrayPoolPartitions();
return Interlocked.CompareExchange(ref _buckets[bucketIndex], inst, null) ?? inst;
}

Expand Down Expand Up @@ -199,7 +196,7 @@ public bool Trim()
SharedArrayPoolPartitions?[] perCoreBuckets = _buckets;
for (int i = 0; i < perCoreBuckets.Length; i++)
{
perCoreBuckets[i]?.Trim(currentMilliseconds, Id, pressure, Utilities.GetMaxSizeForBucket(i));
perCoreBuckets[i]?.Trim(currentMilliseconds, Id, pressure);
}

// Trim each of the TLS buckets. Note that threads may be modifying their TLS slots concurrently with
Expand Down Expand Up @@ -323,14 +320,13 @@ internal sealed class SharedArrayPoolPartitions
private readonly Partition[] _partitions;

/// <summary>Initializes the partitions.</summary>
/// <param name="elementSize">The size of the elements stored in arrays.</param>
public SharedArrayPoolPartitions(int elementSize)
public SharedArrayPoolPartitions()
{
// Create the partitions. We create as many as there are processors, limited by our max.
var partitions = new Partition[SharedArrayPoolStatics.s_partitionCount];
for (int i = 0; i < partitions.Length; i++)
{
partitions[i] = new Partition(elementSize);
partitions[i] = new Partition();
}
_partitions = partitions;
}
Expand Down Expand Up @@ -374,23 +370,20 @@ public bool TryPush(Array array)
return null;
}

public void Trim(int currentMilliseconds, int id, Utilities.MemoryPressure pressure, int bucketSize)
public void Trim(int currentMilliseconds, int id, Utilities.MemoryPressure pressure)
{
Partition[] partitions = _partitions;
for (int i = 0; i < partitions.Length; i++)
{
partitions[i].Trim(currentMilliseconds, id, pressure, bucketSize);
partitions[i].Trim(currentMilliseconds, id, pressure);
}
}

/// <summary>Provides a simple, bounded stack of arrays, protected by a lock.</summary>
/// <param name="elementSize">The size of the elements stored in arrays.</param>
private sealed class Partition(int elementSize)
private sealed class Partition
{
/// <summary>The arrays in the partition.</summary>
private readonly Array?[] _arrays = new Array[SharedArrayPoolStatics.s_maxArraysPerPartition];
/// <summary>The size of the elements stored in arrays.</summary>
private readonly int _elementSize = elementSize;
/// <summary>Number of arrays stored in <see cref="_arrays"/>.</summary>
private int _count;
/// <summary>Timestamp set by Trim when it sees this as 0.</summary>
Expand Down Expand Up @@ -437,20 +430,11 @@ public bool TryPush(Array array)
return arr;
}

public void Trim(int currentMilliseconds, int id, Utilities.MemoryPressure pressure, int bucketSize)
public void Trim(int currentMilliseconds, int id, Utilities.MemoryPressure pressure)
{
const int TrimAfterMS = 60 * 1000; // Trim after 60 seconds for low/moderate pressure
const int HighTrimAfterMS = 10 * 1000; // Trim after 10 seconds for high pressure

const int LargeBucket = 16384; // If the bucket is larger than this we'll trim an extra when under high pressure

const int ModerateTypeSize = 16; // If T is larger than this we'll trim an extra when under high pressure
const int LargeTypeSize = 32; // If T is larger than this we'll trim an extra (additional) when under high pressure

const int LowTrimCount = 1; // Trim one item when pressure is low
const int MediumTrimCount = 2; // Trim two items when pressure is moderate
int HighTrimCount = SharedArrayPoolStatics.s_maxArraysPerPartition; // Trim all items when pressure is high

if (_count == 0)
{
return;
Expand All @@ -477,38 +461,16 @@ public void Trim(int currentMilliseconds, int id, Utilities.MemoryPressure press
}

// We've elapsed enough time since the first item went into the partition.
// Drop the top item so it can be collected and make the partition look a little newer.
// Drop the top item(s) so they can be collected.

ArrayPoolEventSource log = ArrayPoolEventSource.Log;
int trimCount = LowTrimCount;
switch (pressure)
int trimCount = pressure switch
{
case Utilities.MemoryPressure.High:
trimCount = HighTrimCount;

// When pressure is high, aggressively trim larger arrays.
if (bucketSize > LargeBucket)
{
trimCount++;
}

if (_elementSize > ModerateTypeSize)
{
trimCount++;

if (_elementSize > LargeTypeSize)
{
trimCount++;
}
}

break;

case Utilities.MemoryPressure.Medium:
trimCount = MediumTrimCount;
break;
}
Utilities.MemoryPressure.High => SharedArrayPoolStatics.s_maxArraysPerPartition,
Utilities.MemoryPressure.Medium => 2,
_ => 1,
};

ArrayPoolEventSource log = ArrayPoolEventSource.Log;
while (_count > 0 && trimCount-- > 0)
{
Array? array = _arrays[--_count];
Expand Down

0 comments on commit 9b04188

Please sign in to comment.