I think that Partition.Trim(...) from ArraySharedPool<T> may be doing some unnecessary work

[Enderlook]

Hi, I was reading the source of SharedArrayPool<T> for curiosity and I think I may have found that it contains some old code which is irrelevant in the current implementation.

Here the array length inside the Partition class is defined:

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

Line 391 in fa1164c

private readonly Array?[] _arrays = new Array[SharedArrayPoolStatics.s_maxArraysPerPartition];

Here the trimming says that when there is high memory pressure it should clean a certain amount of elements:

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

Line 452 in fa1164c

int HighTrimCount = SharedArrayPoolStatics.s_maxArraysPerPartition; // Trim all items when pressure is high

You can realize that the cleaning amount is the same value used to declare the array length, in other words, it cleans the entire array.

That is used here:

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

Lines 483 to 487 in fa1164c

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

Then, why does the next line add additional elements to trim based on the rented array lengths and element size?

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

Lines 483 to 505 in fa1164c

	int trimCount = LowTrimCount;
	switch (pressure)
	{
	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;

It is already trimming the entire array, asking to trim more won't do anything as far as I know.

If this is not the case, could you please explain me the reasoning under that code? Thanks

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-runtime
See info in area-owners.md if you want to be subscribed.

[stephentoub]

@JeremyKuhne, it looks like this originates from dotnet/coreclr#17078. Do you remember what was the intention here?

[JeremyKuhne]

Presumably I didn't start with trim everything and missed that I upped it to everything? Or meant to pull it back from everything?

[stephentoub]

Ok, thanks