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

Feature request: Create a Stream that reads from a ReadOnlySequence<byte> #27156

Open
AArnott opened this issue Aug 16, 2018 · 12 comments
Open
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Memory
Milestone

Comments

@AArnott
Copy link
Contributor

AArnott commented Aug 16, 2018

I'm re-implementing a protocol in terms of System.IO.Pipelines and striving for minimal allocations, especially of large objects. As part of this, I no longer have to allocate a large array for the entire message, since I read it as it comes in via PipeReader.

At some point I get to the main "content" which I need to deserialize. The deserialization API accepts a TextReader, which I plan to use a StreamReader for. But for that I need a Stream that will read from the ReadOnlySequence<byte> that I got from the PipeReader. I'm planning on writing this up myself in Nerdbank.Streams and exposing it as an Stream ReadOnlySequence<byte>.AsStream() extension method.

Should this be a built-in feature of .NET? I'm imagining perhaps a ReadOnlyMemoryStream class with constructors that accepts ReadOnlyMemory<byte> or ReadOnlySequence<byte> .

@jkotas
Copy link
Member

jkotas commented Aug 16, 2018

Related #26676 #25087
cc @davidfowl

@Wraith2
Copy link
Contributor

Wraith2 commented Sep 6, 2018

@AArnott did you get this implemented in your own package? because I've just reached a point where it would be really useful.

@davidfowl
Copy link
Member

FWIW I think this should exist, but it ends up being a heap allocation. So It kinda sucks.

@Wraith2
Copy link
Contributor

Wraith2 commented Sep 6, 2018

Allocation of the stream? presuming that I can get a section of the input sequence using Slice() I can stand a few allocs to create a container for that to avoid needing to copy the contents of the buffers.

For example i'll have a situation where there will 2Mib+ of compressed data in a pipe surrounded by some framing data. I want to identify the frame and payload limits then push out the payload section using a ROS slice, wrap that in decompression (and possible possibly crypto) stream and then use that as input to a streamed database parameter.

@AArnott
Copy link
Contributor Author

AArnott commented Sep 6, 2018

did you get this implemented in your own package?

Yes, @Wraith2. It's documented here and you can get it from NuGet package

@davidfowl
Copy link
Member

Allocation of the stream? presuming that I can get a section of the input sequence using Slice() I can stand a few allocs to create a container for that to avoid needing to copy the contents of the buffers.

For big buffers I agree. This was a significant amount of allocations if you're sending lots of data.

@davidfowl
Copy link
Member

@AArnott Can you propose an API? I'd like to add this for 3.0, but we need an API proposal here. It gels nicely with the other APIs we'd like to add to 3.0

@stephentoub
Copy link
Member

Whatever we do here, I'd like to see it done in conjunction with https://github.com/dotnet/corefx/issues/22404, which is about creating streams for Memory<T> and ReadOnlyMemory<T>. Note that we already have a ReadOnlyMemoryStream<T> in corefx (https://github.com/dotnet/corefx/blob/6b9440735abc9c4e369376bca097766527971a7a/src/Common/src/System/IO/ReadOnlyMemoryStream.cs), but it's internal, added in support of ReadOnlyMemoryContent for HttpClient (dotnet/corefx#24006). And of course we need to either put it behind a factory or do something about the name, as the writable equivalent would have an obvious conflict.

@AArnott
Copy link
Contributor Author

AArnott commented Oct 19, 2018

How do you all feel about AsStream() extension methods that simply return Stream for these? That's how I do it in nerdbank.streams for most of these and it avoids such problems as additional classes, naming, etc.

The only time it doesn't work is when I need to expose special members. The only one I can think of now is if we create a (writable) stream over Memory<T>, which has fixed size and thus we might want to expose a Stream-derived class that defines a Capacity member (ala MemoryStream) so people understanding why writing may run out of space and can predict that.

@stephentoub
Copy link
Member

How do you all feel about AsStream() extension methods that simply return Stream for these?

Seems reasonable.

@shravan2x
Copy link

Would it be possible to include this in .NET 6?

@tomkerkhove
Copy link
Member

💯

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
Projects
None yet
Development

No branches or pull requests

9 participants