MemoryMappedViewStreams cannot Read(Span<byte>) to destination without intermediate buffer overhead via System.IO.Stream.Read(Span<byte>)'s ArrayPool calls

[miniksa]

Description

When using a MemoryMappedViewStream and calling the .Read(Span<byte>) method on it, we will end up thunking down through System.IO.Stream.Read(Span<byte>) which utilizes an intermediate byte[] buffer out of ArrayPool<byte>.Shared instead of simply block transferring bytes directly from the memory mapped section into the Span that was given.

runtime/src/libraries/System.Private.CoreLib/src/System/IO/Stream.cs

Lines 790 to 809 in 1d1bf92

	public virtual int Read(Span<byte> buffer)
	{
	byte[] sharedBuffer = ArrayPool<byte>.Shared.Rent(buffer.Length);
	try
	{
	int numRead = Read(sharedBuffer, 0, buffer.Length);
	if ((uint)numRead > (uint)buffer.Length)
	{
	throw new IOException(SR.IO_StreamTooLong);
	}
	new ReadOnlySpan<byte>(sharedBuffer, 0, numRead).CopyTo(buffer);
	return numRead;
	}
	finally
	{
	ArrayPool<byte>.Shared.Return(sharedBuffer);
	}
	}

This seems to be because the code in UnmanagedMemoryStream, which is the base class for MemoryMappedViewStream is falling back to base.Read(Span<byte>) in any circumstance where the class type isn't exactly itself.

runtime/src/libraries/System.Private.CoreLib/src/System/IO/UnmanagedMemoryStream.cs

Lines 344 to 354 in 1d1bf92

	if (GetType() == typeof(UnmanagedMemoryStream))
	{
	return ReadCore(buffer);
	}
	else
	{
	// UnmanagedMemoryStream is not sealed, and a derived type may have overridden Read(byte[], int, int) prior
	// to this Read(Span<byte>) overload being introduced. In that case, this Read(Span<byte>) overload
	// should use the behavior of Read(byte[],int,int) overload.
	return base.Read(buffer);
	}

However, MemoryMappedViewStream doesn't implement anything else for .Read() here at all and likely wants to be hitting UnmanagedMemoryStream.ReadCore(Span<byte>) for this activity so the high-performance unsafe raw memory move is performed without any intermediate allocations or fluff.

runtime/src/libraries/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedViewStream.cs

Lines 9 to 69 in 1d1bf92

	public sealed class MemoryMappedViewStream : UnmanagedMemoryStream
	{
	private readonly MemoryMappedView _view;
	internal MemoryMappedViewStream(MemoryMappedView view)
	{
	Debug.Assert(view != null, "view is null");
	_view = view;
	Initialize(_view.ViewHandle, _view.PointerOffset, _view.Size, MemoryMappedFile.GetFileAccess(_view.Access));
	}
	public SafeMemoryMappedViewHandle SafeMemoryMappedViewHandle => _view.ViewHandle;
	public long PointerOffset => _view.PointerOffset;
	public override void SetLength(long value)
	{
	ArgumentOutOfRangeException.ThrowIfNegative(value);
	throw new NotSupportedException(SR.NotSupported_MMViewStreamsFixedLength);
	}
	protected override void Dispose(bool disposing)
	{
	try
	{
	if (disposing && !_view.IsClosed && CanWrite)
	{
	Flush();
	}
	}
	finally
	{
	try
	{
	_view.Dispose();
	}
	finally
	{
	base.Dispose(disposing);
	}
	}
	}
	// Flushes the changes such that they are in sync with the FileStream bits (ones obtained
	// with the win32 ReadFile and WriteFile functions). Need to call FileStream's Flush to
	// flush to the disk.
	// NOTE: This will flush all bytes before and after the view up until an offset that is a
	// multiple of SystemPageSize.
	public override void Flush()
	{
	if (!CanSeek)
	{
	throw new ObjectDisposedException(null, SR.ObjectDisposed_StreamIsClosed);
	}
	_view.Flush((UIntPtr)Capacity);
	}
	}

I think the error is that it goes straight to base.Read(Span<byte>) here. However, there's a conundrum. You cannot just bare call Read(Span<byte>) to make it try to resolve to any overload because then it will probably stack overflow here when it's not overloaded. And there's no real good way to go backwards from Span<byte> to byte[] such that we could instead go for the Read(byte[], int, int) variant.

For now, my workaround is to simply create the MemoryMappedViewStream and hold onto that so it maintains the lifetime of all the underlying gunk and then make myself a second UnmanagedMemoryStream by forwarding the relevant parameters out of the MemoryMappedViewStream directly into the UnmanagedMemoryStream constructor following what happens in the Initialize call here:

runtime/src/libraries/System.IO.MemoryMappedFiles/src/System/IO/MemoryMappedFiles/MemoryMappedViewStream.cs

Line 18 in 1d1bf92

Initialize(_view.ViewHandle, _view.PointerOffset, _view.Size, MemoryMappedFile.GetFileAccess(_view.Access));

Configuration

I'm targeting .NET 8.0 and my SDK installed is 8.0.411.

Regression?

No.

Data

I can't share this publicly. I ran the profiler on non-public source, but I don't think this is necessary because not allocating or borrowing an array is always cheaper/faster than doing so.

Analysis

I'm pretty sure that UnmanagedMemoryStream.Read(Span<byte>) needs to be adjusted to less aggressively forward calls down to the base class. I'm just not sure exactly how.

[dotnet-policy-service]

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

[Rob-Hague]

MemoryMappedViewStream is sealed so public override int Read(Span<byte> buffer) => base.ReadCore(buffer);?

[miniksa]

MemoryMappedViewStream is sealed so public override int Read(Span<byte> buffer) => base.ReadCore(buffer);?

See, I thought that, but that really only resolves this one particular derivation and not the underlying issue.

The fact that it's UnmanagedMemoryStream implies to me that just about any derivation would strongly prefer to have block data copied here without the intermediate.

Additionally, the comment inside UnmanagedMemoryStream.Write is suspect.

runtime/src/libraries/System.Private.CoreLib/src/System/IO/UnmanagedMemoryStream.cs

Lines 618 to 620 in 1d1bf92

	// UnmanagedMemoryStream is not sealed, and a derived type may have overridden Write(byte[], int, int) prior
	// to this Write(Span<byte>) overload being introduced. In that case, this Write(Span<byte>) overload
	// should use the behavior of Write(byte[],int,int) overload.

It says that it should be using the three-part overload but then it goes ahead and doesn't do that.

I suppose https://github.com/dotnet/runtime/blob/1d1bf92fcf43aa6981804dc53c5174445069c9e4/src/libraries/System.Private.CoreLib/src/System/IO/Stream.cs#L922-934 in the base class Stream makes the bare call back to the three-part Write which could then be resolved to the derivation's override.

OK, you've convinced me. The "safest" fix is probably to just fix MemoryMappedViewStream then because who knows what other derivations of UnmanagedMemoryStream are in the wild pre-introduction of Span<T> overloads.

[miniksa]

Ugh, doesn't work because ReadCore is internal on the UnmanagedMemoryStream and in a different namespace. Thinking....

[teo-tsirpanis]

One way to fix this is to have UnmanagedMemoryStream.Read(Span<byte>) forward to Read(byte[], int, int) only when the byte array overload is overridden but the span overload is not (so that when both are overridden, base.Read(span) is efficient).

Stream already has some checks for whether BeginRead and BeginWrite are overridden, but their implementation is non-trivial and requires cooperation from the runtime. Maybe it will be simpler to implement once instance function pointers are added to the language.

c.c. @stephentoub