[API Proposal]: System.Buffers : BuffersExtensions.AsStream

mgravell commented 5 months ago

Background and motivation

This is tangentially related to https://github.com/dotnet/runtime/issues/100290 and the .NET 9 distributed caching epic; the proposed hybid-cache API will use ReadOnlySequence<byte> and IBufferWriter<byte> as the primary serializer APIs, but: not all serializers support these APIs (many do, note) - with Stream being the most common fallback.

API Proposal

  // assembly: System.Memory
  namespace System.Buffers;

  public static class BuffersExtensions // pre-existing
  {
+     public static Stream AsStream(this ReadOnlySequence<byte> value) => new ReadOnlySequenceStream(value);
+     public static Stream AsStream(this IBufferWriter<byte> value) => new BufferWriterStream(value);
  }
+ internal sealed class ReadOnlySequenceStream : Stream {...}
+ internal sealed class BufferWriterStream : Stream {...}

API Usage

ReadOnlySequence<byte> payload = ...
Customer obj = SomeRandomSerializer.Deserialize<Customer>(payload.AsStream());

and

Customer obj = ...
IBufferWriter<byte> target = ...
SomeRandomSerializer.Serialize<Customer>(target.AsStream(), obj);

The implementations would be internal, but:

ReadOnlySequenceStream has a CanRead: true, CanWrite: false implementation that supports seek etc; no additional buffer copies, just a few "where are we" counters, using SequencePosition iteration and a ReadOnlyMemory<byte> snapshot of the current segment
BufferWriterStream has a CanRead: false, CanWrite: true implementation that does not support seek; position and length are read-only and report the bytes written so far; no double-buffering - it is assumed (as a fundamental part of IBufferWriter<byte>) that the underlying IBufferWriter<byte> already does some internal work there when responding to GetSpan/GetMemory (such that they are affordable), so this would be duplicated effort and an additional mem-copy
in both cases, all APIs are synchronous, with the async APIs implemented as async-over-sync as efficiently as possible
no need for fancy recycling, since we don't have any local buffers etc

Alternative Designs

The alternative is to use MemoryStream, which involves multiple additional copy operations and additional byte[] buffers.

Risks

None seen

Additional

I am happy to contribute the implementation effort.

dotnet-policy-service[bot] commented 5 months ago

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

stephentoub commented 5 months ago

The ReadOnlySequence<T> side of this is https://github.com/dotnet/runtime/issues/27156. @Jozkee, where did we land on a design for these extra streams, e.g. AsStream vs dedicated publicly named streams vs etc. Implementation is straightforward, so if we have an agreed upon design, we can make quick forward progress here.

jozkee commented 5 months ago

https://github.com/dotnet/runtime/issues/82801#issuecomment-1481694752

For [ReadOnly]Memory<byte> and ReadOnlySequence<byte>, we want to have static methods that return the Stream wrapping them. If we want to have them as extension methods in Stream we would need the static extensions language feature to have AsStream(this ReadOnlySequence<byte>), or we move ReadOnlySequence down to S.P.CoreLib.

I discarded having Memory<byte> support directly in MemoryStream based on https://github.com/dotnet/runtime/pull/84103.

mgravell commented 5 months ago

Re corelib: that's why I proposed the existing BuffersExtensions as the root (or something in the same package/namespace) - it should have all the right refs.

PaulusParssinen commented 5 months ago

Noting that we now have System.IO.Pipelines available in .NET 9 shared runtime meaning that there are now two types implementing IBufferWriter<byte> in the BCL; PipeWriter and ArrayBufferWriter<byte>.

When I use S.IO.Pipelines I often find myself having to pull CommunityToolkit.HighPerformance in for its AsStream(this IBufferWriter<byte> writer) fallback.

jozkee commented 5 months ago

Considering that static extensions is in the backlog https://github.com/dotnet/csharplang/issues/192, I think we should consider this for 9 while keeping https://github.com/dotnet/runtime/issues/82801 as future.

dotnet / runtime