Runtime: Read variable-length spans from SequenceReader

[FiniteReality]

SequenceReader currently has no way of reading an arbitrary number of bytes, which in some cases is extremely useful. For example, reading length-prefixed strings or other non-delimited data chunks of fixed length. This is important when dealing with information from over the network using System.IO.Pipelines, as SequenceReader implements a lot of boilerplate logic which is extremely useful, such as cross-buffer reads and automatic advancement within the sequence.

Rationale

Implementing this in user code is extremely hard as it will cause compiler error CS8350, due to passing a Span along with a SequenceReader, which are both ref structs. The only work around I know of is to pass the reader as in and then pin the SequenceReader as a pointer so it can be converted back to a ref, to make it mutable again. For example:

public unsafe static bool TryReadSpan(
    in this SequenceReader<byte> reader, Span<byte> span)
{
    fixed (SequenceReader<byte>* tmpReader = &reader)
    {
        ref var mutableReader = ref *tmpReader;
        for (int i = 0; i < span.Length; i++)
        {
            if (!mutableReader.TryRead(out byte value))
                return false;
            span[i] = value;
        }
        return true;
    }
}

My current use case is reading a length-prefixed string, while implementing a network protocol as a client. There are no delimiters, which makes it impossible to use the existing methods such as TryReadTo or TryAdvanceTo.

Proposed API

public partial ref struct SequenceReader<T>
{
    public bool TryReadSpan(Span<byte> span);
}

[tannergooding]

CC. @layomia, @JeremyKuhne, @ahsonkhan

[tannergooding]

@FiniteReality, for reference the API review process is detailed here: https://github.com/dotnet/corefx/blob/master/Documentation/project-docs/api-review-process.md#steps

It would be great if you could update the original post to follow the good example template given under step 1 of that process.

You have basically all the content there already; but it is helpful to section it out and have an explicit listing of the requested API, which in this case looks to be:

public partial ref struct SequenceReader<T>
{
    public bool TryReadSpan(Span<byte> span);
}

[tannergooding]

I'll let the area owners comment , but it looks like there is also a TryPeek method that would likely be desirable to overload simultaneously.

Since this deals in bytes, it might be good to have explicit big vs little endian overloads...

There is also an existing TryCopyTo(Span<T>) method to consider.

[FiniteReality]

I did try TryCopyTo(Span<T>) but it also appears to be affected by CS8350. Maybe this requires language changes to be usable?

For reference, the code I tried was:

Span<byte> data = stackalloc byte[20];
reader.TryCopyTo(data); // or reader.TryReadSpan using 'ref' instead of 'in'

[tannergooding]

It's likely because the compiler can't determine you aren't stashing the span internally; which requires you to have a readonly ref struct, an in extension method, or a instance method that is explicitly marked readonly (new feature).

At least at first glance, it looks like TryCopyTo and TryPeek aren't mutating the underlying instance; unlike TryRead which can if the data splits a segment, so they might be applicable to mark as non-mutating (assuming having that as part of the contract is desirable; since it would be a breaking change to remove it later if mutating became desirable).

[xqrzd]

I also needed something like this (read length prefixed string). You can work around the compiler error with unsafe code,

byte* buffer = stackalloc byte[length];
// Hack because the compiler thinks reader.TryCopyTo could store the span.
var tempSpan = new Span<byte>(buffer, length);

Below is the full method I wrote, mostly based on existing code in SequenceReader. It works specifically with strings,

public static bool TryReadUtf8String(
    ref this SequenceReader<byte> reader, int length, out string value)
{
    ReadOnlySpan<byte> span = reader.UnreadSpan;
    if (span.Length < length)
        return TryReadMultisegmentUtf8String(ref reader, length, out value);

    ReadOnlySpan<byte> slice = span.Slice(0, length);
    value = Encoding.UTF8.GetString(slice);
    reader.Advance(length);
    return true;
}

private static unsafe bool TryReadMultisegmentUtf8String(
    ref SequenceReader<byte> reader, int length, out string value)
{
    Debug.Assert(reader.UnreadSpan.Length < length);

    // Not enough data in the current segment, try to peek for the data we need.
    // In my use case, these strings cannot be more than 64kb, so stack memory is fine.
    byte* buffer = stackalloc byte[length];
    // Hack because the compiler thinks reader.TryCopyTo could store the span.
    var tempSpan = new Span<byte>(buffer, length);

    if (!reader.TryCopyTo(tempSpan))
    {
        value = default;
        return false;
    }

    value = Encoding.UTF8.GetString(tempSpan);
    reader.Advance(length);
    return true;
}

[JeremyKuhne]

@jaredpar, here is another example of where local would be extremely useful.

[BrunoZell]

How is this different from SequenceReader<T>.TryCopyTo(Span<T>)introduced in .NET Core 3.0? That method tries to fill the passed span and returns true if the reader has enough data available starting from the current positions. Returns false if there is not enough data.

28496 talks about that method not advancing the current position of the reader, but an extension method is provided in the comments that takes care of that:

public static class SequenceReaderExtensions {
    [MethodImpl(MethodImplOptions.AggressiveInlining)]
    public static bool TryCopyToA<T>(ref this SequenceReader<T> reader, Span<T> destination)
        where T : unmanaged, IEquatable<T> {
            if (reader.TryCopyTo(destination)) {
                reader.Advance(destination.Length);
                return true;
            }
            return false;
        }
}

[BrunoZell]

Well, this does not work when the span we want to copy to is stack-allocated. 😐