Make ArrayMemoryPoolBuffer a struct for less GC pressure.

[Danielku15]

Hey there.

While troubleshooting the memory allocations in my networking code, I realized that the MemoryPool<byte>.Shared allocates new heap objects on every Rent.

runtime/src/libraries/System.Memory/src/System/Buffers/ArrayMemoryPool.cs

Line 19 in 83d4dcd

return new ArrayMemoryPoolBuffer(minimumBufferSize);

runtime/src/libraries/System.Memory/src/System/Buffers/ArrayMemoryPool.ArrayMemoryPoolBuffer.cs

Line 8 in 83d4dcd

private sealed class ArrayMemoryPoolBuffer : IMemoryOwner<T>

I wonder if this is really by-design, or just a historical oversight. When using System.IO.Pipelines.PipeWriter the underlying pipe will allocate objects from the configured MemoryPool triggering heap allocations on every write to the pipe.

runtime/src/libraries/System.IO.Pipelines/src/System/IO/Pipelines/Pipe.cs

Lines 229 to 257 in 83d4dcd

	private void RentMemory(BufferSegment segment, int sizeHint)
	{
	// Segment should be new or reset, otherwise a memory leak could occur
	Debug.Assert(segment.MemoryOwner is null);
	Debug.Assert(sizeHint >= 0);
	MemoryPool<byte>? pool = null;
	int maxSize = -1;
	if (!_options.IsDefaultSharedMemoryPool)
	{
	pool = _options.Pool;
	maxSize = pool.MaxBufferSize;
	}
	if (sizeHint <= maxSize)
	{
	// Use the specified pool as it fits. Specified pool is not null as maxSize == -1 if _pool is null.
	segment.SetOwnedMemory(pool!.Rent(GetSegmentSize(sizeHint, maxSize)));
	}
	else
	{
	// Use the array pool
	int sizeToRequest = GetSegmentSize(sizeHint);
	segment.SetOwnedMemory(ArrayPool<byte>.Shared.Rent(sizeToRequest));
	}
	_writingHeadMemory = segment.AvailableMemory;
	}

Even Kestrel is affected by this: https://github.com/dotnet/aspnetcore/blob/8d9545796d3a6e0a7a21610d234ff5b28bada6d9/src/Servers/Kestrel/shared/DefaultSimpleMemoryPoolFactory.cs#L9-L17

For my own sake I now created an own MemoryPool<T> which I pass into the related code areas and there I use a struct ArrayMemoryPoolBuffer : IMemoryOwner<T>.

[huoyaoyuan]

It is passed in the IMemoryOwner<T> interface. Making it a struct has no effect because it will always be boxed.

[Danielku15]

🤦‍♂️ of course, you're right. That makes the whole situation trickier than I thought.

I think for my project I'll use some ObjectPool<PooledMemoryOwner> approach to reuse the wrapped objects.

Nevertheless I think my concern stays valid: there is room for improvement regarding these objects. Just looking at GC/Memory performance shifting to structs would eliminate some overhead here with a direct benefit for various components.

Regarding the breaking changes and compatibility with some other use cases, there might be need for a different approach to this.

[huoyaoyuan]

MemoryPool<T> is used less than ArrayPool<T>. If you don't need IMemoryOwner, then using ArrayPool<T> just works.

[Danielku15]

Yes, in my own code I go for ArrayPool directly. But sometimes it is part of the default library behaviors like specifically System.IO.Pipelines uses the memory pool:

1. Pipe uses the memory pool here.
2. The memory pool might come from outside.
3. Kestrel also loads the memory pool as configured https://github.com/dotnet/aspnetcore/blob/03d43ac61779148acb14ff0abd32a079a28ba42d/src/Servers/Kestrel/Transport.Sockets/src/WebHostBuilderSocketExtensions.cs#L36

The memorypool might be reconfigured through a variety of DI injections. Also kestrel itself seem to change it to PinnedBlockMemoryPoolFactory by default.

In my memory profiles I could see a lot of objects created when many users are connected to the web application simultaneously and receiving data. Somehow the non-default MemoryPools make it through. It's easily happening that a different memory pool is not the default.