Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[API Proposal]: Add possibility to create ReadOnlySequence<T> with multiple segments. #111570

Open
paulomorgado opened this issue Jan 18, 2025 · 5 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Memory untriaged New issue has not been triaged by the area owner

Comments

@paulomorgado
Copy link
Contributor

paulomorgado commented Jan 18, 2025

Background and motivation

ReadOnlySequence<T> has APIs to create empty or single-segment instances, but there's no on-the-box help for creating multiple-segment instances.

The first thing a user needs to do is create a ReadOnlySequenceSegment<T> derived class.

API Proposal

ASP.NET Core has a few implementations of this:

internal sealed class BufferSegment : ReadOnlySequenceSegment<byte>
{
    public BufferSegment(Memory<byte> memory)
    {
        Memory = memory;
    }
 
    public BufferSegment Append(ReadOnlyMemory<byte> memory)
    {
        var segment = new BufferSegment(memory)
        {
            RunningIndex = RunningIndex + Memory.Length
        };
        Next = segment;
        return segment;
    }
}

Just having a public implementation would go a long way helping users.

I know there are a few more elaborated implementations, but just having this would help a lot.

Adding to that, simple APIs to create from IEnumerable<IEnumerable<T>>

new ReadOnlySequence<T>(IEnumerable<IEnumerable<T>> source);
new ReadOnlySequence<T,U>(U source, Func<U, IEnumerable<IEnumerable<T>> sourceSelector, Func<IEnumerable<T>,ReadOnlyMemory<T>> segmentSelector);

API Usage

ReadOnlySequence<byte> Transform(ReadOnlySequence<byte> source, Func<ReadOnlyMemory<byte>, ReadOnlyMemory<byte>> transform)
{
    var e = source.GetEnumerable();

    if (!e.MoveNext())
    {
        return ReadOnlySequence<byte>.Empty;
    }

    ReadOnlySequenceSegment<T>? first = null;
    ReadOnlySequenceSegment<T> last;

    do
    {
        if (first is null)
        {
            last = first = new BufferSegment(transform(e.Current));
        }
        else
        {
            last = last.Append(transform(e.Current));
        }
    }
    while (e.MoveNext());

    return new ReadOnlySequence<byte>(first, 0, last, last.Memory.Length);
}

Alternative Designs

No response

Risks

No response

@paulomorgado paulomorgado added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Jan 18, 2025
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Jan 18, 2025
Copy link
Contributor

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

@MihaZupan
Copy link
Member

I think you're missing the proposed API part :)

@paulomorgado
Copy link
Contributor Author

I think you're missing the proposed API part :)

Am I?

I didn't provide an implementation, but I did provide a sample API.

This needs to be triaged first. If accepted, I can bootstrap an implementation.

@MihaZupan
Copy link
Member

Image

@teo-tsirpanis
Copy link
Contributor

ReadOnlySequence<T> is a high-performance and advanced API, and in order to fully take advantage of its capabilities and reduce memory allocations, you need a mechanism of reusing the sequence segment objects and maybe storing additional information, which will require subclassing ReadOnlySequenceSegment<T>.

create from IEnumerable<IEnumerable<T>>

If we decide to add a convenience API to create ROS, it should accept IEnumerable<ReadOnlyMemory<T>>, making the second of proposed overload unnecessary. I'm not sure if it is a good idea though, as it will lead people into a performance pit of failure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Memory untriaged New issue has not been triaged by the area owner
Projects
None yet
Development

No branches or pull requests

3 participants