Runtime: [API Proposal] Add ReadOnlySequence<T>.GetFirstSpan()

[ahsonkhan]

We have a property on ReadOnlySequence<T> that returns a ReadOnlyMemory<T>.
```C#
public readonly partial struct ReadOnlySequence
{
public ReadOnlyMemory First { get { throw null; } }
}

We should consider adding a `GetFirstSpan() `method as a performance optimization since it can be implemented faster than accessing the property: `First.Span`. This is because slicing a `Span<T>` is faster than slicing a `Memory<T>`.

**API Addition:**
```C#
public readonly partial struct ReadOnlySequence<T>
{
    public ReadOnlySpan<T> GetFirstSpan() { throw null; }
    // OR a property instead?
    public ReadOnlySpan<T> FirstSpan { get { throw null; } }
}

Open Questions:

• Should it be a property like First, or should it be a method?

One scenario where this is useful is BufferReader. It can potentially improve the cost of creating a BufferReader by ~10%.

C# public BufferReader(ReadOnlySequence<T> buffer) { CurrentSpanIndex = 0; Consumed = 0; Sequence = buffer; _currentPosition = buffer.Start; _nextPosition = default; CurrentSpan = buffer.First.Span; // buffer.GetFirstSpan(); ... }

Example implementation diff between get_First and GetFirstSpan():

https://www.diffchecker.com/8BWywQZA

cc @JeremyKuhne, @davidfowl, @pakrym

[davidfowl]

Is that 10% in both array based on multi segment scenarios?

[ahsonkhan]

Is that 10% in both array based on multi segment scenarios?

Depends on the implementation, but this is what I observed based on what the ReadOnlySequence was backed by (when constructing a BufferReader):

• Array: 10% improvement
• ReadOnlySequenceSegment<byte> (multi segment): 10%+ improvement
• String: 10% improvement
• MemoryManager: 5% regression
• All the rest: No difference

[davidfowl]

These changes sound great! I just looked and pipelines uses ReadOnlySequenceSegment so I'm happy.

MemoryManager: 5% regression

Do we have anything concrete that uses these?

[davidfowl]

I think it should be called FirstSpan and be a property.

[ahsonkhan]

These changes sound great! I just looked and pipelines uses ReadOnlySequenceSegment so I'm happy.

I would imagine mutli-segment ROSSegment is the most common case given the whole point of ReadOnlySequence is sequence of segments. I want to optimize the implementation for that case (and maybe array).

Do we have anything concrete that uses these?

Not really. Mainly diagnostics which is why I think it is fine to make the trade-off in terms of perf:
https://github.com/aspnet/KestrelHttpServer/blob/1f3524787e62db9890547b33e8a450441b14810b/shared/Microsoft.Extensions.Buffers.MemoryPool.Sources/DiagnosticPoolBlock.cs#L14

[pakrym]

FirstSpan sounds good.

[terrajobst]

Having an API returning the span directly seems fine, we should probably make it a property though, otherwise

```C#
var a = sequence.First.Span;
var b = sequence.GetFirstSpan();

The first line looks cheaper than the second one, so:

```C#
var a = sequence.First.Span;
var b = sequence.FirstSpan;

Still not ideal, but we shipped First already.

[scalablecory]

Attempting to optimize this sequence use myself and found a nice internal method GetFirstSpan... Would love to see this change get into the 3.0 release.

I like the existing GetFirstSpan() API more than the one proposed here, and think it supports using a method rather than a property:

void GetFirstSpan(out ReadOnlySpan<T> first, out SequencePosition next)

Though I might flip the parameter positions for consistency with TryGet. Another thought is return a bool indicating if the sequence is empty or not.

I might augment the API with an overload of TryGet too, that outputs a Span:

void TryGet(ref SequencePosition next, out Span<T> span)

This allow usage such as:

if(sequence.TryGetFirstSpan(out ReadOnlySpan<T> span, out SequencePosition nextPosition))
{
    do
    {
        // fast path
    }
    while(sequence.TryGet(ref nextPosition, out span));
}

[ahsonkhan]

Attempting to optimize this sequence use myself and found a nice internal method GetFirstSpan... Would love to see this change get into the 3.0 release.

Can you file a new issue for that please. We should consider it separately.

cc @JeremyKuhne

[davidfowl]

We didn't add this to ns2.1 😢

[davidfowl]

cc @terrajobst