Consider adding Scatter/Gather IO on Stream

[geoffkizer]

The Socket class has this already, in the form of Send/Receive operations that take an IList<ArraySegment<byte>>. This isn't exposed up through Stream classes, however.

One specific scenario where this would be useful is writing chunked-encoded request bodies in SocketsHttpHandler. When the user calls the request Stream's Write[Async] method, we need to first write a chunk header with the chunk length followed by \r\n, then write the chunk contents from the user's buffer to the underlying stream (which is either SslStream or NetworkStream). This requires us to either do two writes (which means two kernel calls and means the chunk header will likely be sent in a separate packet), or to copy into a local buffer so we can do a single write. It would be more performant (and simpler) to do a gather write where we pass two buffers, the first containing the chunk header, the second containing the chunk data, passed directly through.

There are various examples in HTTP2 where scatter/gather IO would be useful as well. (Mostly gather, since gather helps deal with the "framing" scenario above. Scatter reads are less useful since you don't control the framing yourself.)

Proposed API:

        public virtual int Read(IReadOnlyList<Memory<byte>> buffers);
        public virtual void Write(IReadOnlyList<ReadOnlyMemory<byte>> buffers);
        public virtual ValueTask<int> ReadAsync(IReadOnlyList<Memory<byte>> buffers, CancellationToken cancellationToken = default);
        public virtual ValueTask WriteAsync(IReadOnlyList<ReadOnlyMemory<byte>> buffers, CancellationToken cancellationToken = default);

The base Stream class would provide default implementations on top of existing Read/Write APIs.

[scalablecory]

It doesn't help with Content-Length encoding

Sure it does; you have one potential I/O for headers and one for content?

[geoffkizer]

Sure it does; you have one potential I/O for headers and one for content?

Yeah, you are right, of course.

[geoffkizer]

Hrm that's strange; it maxes out a core 100% on mine.

It maxes out two cores on mine (out of 16) -- one is hard pegged at 100%, the other bounces off it. Other CPUs show significant usage but aren't maxed out.

Overall CPU usage is only about 33%. The client and server processes seem to account for something like 10-12%, and are presumably what's causing those two CPUs to be maxed out or close. That means something like 2/3rds of the CPU usage in this test is actually in kernel mode -- which is good, since the test code isn't doing that much work.

(Data is just a hot read of Task Manager so take it with a grain of salt.)

But this points out another reason why we should be maxing total CPU and measuring throughput -- that will better accoutn for all the work happening on other threads in the kernel.

[davidfowl]

Yea, we need to run this with crank.

cc @sebastienros

[geoffkizer]

I'm hacking something together to run in crank -- been meaning to learn more about crank anyway...

[geoffkizer]

FYI, my ScatterGatherServer code is here: https://github.com/geoffkizer/netperf/tree/master/ScatterGatherServer

Feedback welcome. I'm working with @sebastienros to get perf results using this.

This implements three variations of sending a Content-Length encoded response of a configurable length:

• Mode SendMultiple sends response headers and response body in separate Send calls.
• Mode BufferSends copies the headers and body into a single buffer and does a single Send.
• Mode GatherSends sends headers and body in a single gathered Send.

[adamsitnik]

On Windows we could emulate it trivially for files.

FWIW On Windows there is ReadFileScatter and WriteFileGather. The problem is that it requires FILE_FLAG_OVERLAPPED (async file handle) and FILE_FLAG_NO_BUFFERING (not exposed by .NET yet, requires inputs to be aligned and of a certain size).

https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-readfilescatter https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-writefilegather

We are most likely going to implement that as part of https://github.com/dotnet/runtime/issues/24847 as SafeFileHandle extension methods

[geoffkizer]

So, #24847 is defining a version of scatter/gather for offset-based file IO. We should follow the same pattern for Stream (and Socket as well).

[geoffkizer]

To match the pattern approved in #24847, I think these APIs would look like this:

  public virtual long Read(IReadOnlyList<Memory<byte>> buffers);
  public virtual void Write(IReadOnlyList<ReadOnlyMemory<byte>> buffers);
  public virtual ValueTask<long> ReadAsync(IReadOnlyList<Memory<byte>> buffers, CancellationToken cancellationToken = default);
  public virtual ValueTask WriteAsync(IReadOnlyList<ReadOnlyMemory<byte>> buffers, CancellationToken cancellationToken = default);

The only change here from above is returning long instead of int for the read ops.

[adamsitnik]

@geoffkizer how about using the Gather and Scatter prefixes in method names?

[geoffkizer]

how about using the Gather and Scatter prefixes in method names?

I don't have a strong opinion either way...

Socket has some existing scatter/gather support on Send and Receive; it doesn't use the terms Scatter/Gather as part of the API there. But we don't need to be consistent with that.

[stephentoub]

how about using the Gather and Scatter prefixes in method names?

I'm very much against that. These should be overloads of Read/Write{Async}. These are just different representations of data, just as are span, array, and memory. If it makes sense for a given stream, it's perfectly valid for a stream to implement the existing apis using OS scatter/gather functionality, e.g. splitting a span into multiple pieces. Conversely, it's valid for these new overloads to be implemented with a single buffer, e.g. copying all write buffers to a single rented array and writing that out. These really are just overloads with different representations of the data. On top of that, there's no precedent for the scatter/gather terminology in our libraries, and in fact there's precedent for not using it (e.g. Socket). I don't want to force all developers using Stream to learn this terminology, which adds no benefit, just extra mental complexity.