StreamPipeReader may have a potential memory leak risk.

[Vincent-X-Zhang]

Recently, I have been using PipeReader to parse large files. There is a ReadOnlySequence with a length greater than 800 * 1024 * 1024 that needs to be converted to an array using ToArray. Even after properly calling Advance and Complete on the reader, I still found a large amount of Gen2 memory fragmentation. The following issue was discovered in the source code:

private BufferSegment AllocateSegment(int? minimumSize = null)
{
        BufferSegment nextSegment = CreateSegmentUnsynchronized();

        var bufferSize = minimumSize ?? BufferSize;
        int maxSize = !_options.IsDefaultSharedMemoryPool ? _options.Pool.MaxBufferSize : -1;

         if (bufferSize <= maxSize)
         {
                // Use the specified pool as it fits.
                int sizeToRequest = GetSegmentSize(bufferSize, maxSize);
                nextSegment.SetOwnedMemory(_options.Pool.Rent(sizeToRequest));
         }
        else
        {
                // Use the array pool
                int sizeToRequest = GetSegmentSize(bufferSize, MaxBufferSize);
                nextSegment.SetOwnedMemory(ArrayPool<byte>.Shared.Rent(sizeToRequest));
        }

        return nextSegment;
}

nextSegment.SetOwnedMemory(ArrayPool<byte>.Shared.Rent(sizeToRequest)); rent the array from ArrayPool<byte>. But i cannot found when and where it return back to the pool. I mean it may lead to memory leak.

[EgorBo]

I mean it may lead to memory leak.

Not returning a rented array to its pool doesn't cause memory leaks alone, just makes it useless (compared to normal gc allocation).

The array seems to be returning to the pool inside BufferSegment.ResetMemory()

[Vincent-X-Zhang]

You're right! I shouldn't try to put all the data into a single array.
But can you give me some good advice on the data type for those chunks? I mean, BufferSegment is internal, and List<byte[]> is not very convenient to use. There's no existing type that can accurately and simply describe this type.

[Clockwork-Muse]

But can you give me some good advice on the data type for those chunks?

Ideally, nothing; if you can avoid it, don't pass the chunks around at all.

List<byte[]> is not very convenient to use

I mean, using that would negate the entire point, since that would be copies of the data.

No, the output of ReadOnlySequence.Slice(...) is... another ReadOnlySequence. Which is what you want, because the source is over some segmented chunks of memory (regardless of what it's backed by), and you don't want to have to account for that yourself (presuming that the data is longer than a single segment).

So slice from after the length header for that number of bytes, then iterate through that subsequence parsing and processing it as normal. Ideally, you can just use ReadOnlySpans from segments, but occasionally you may need to copy to a temporary pooled array (if data spans two or more segments) - one that should be returned before advancing to the next token. SequenceReader may be helpful here.

The entire point is to have no significant portion as a raw binary array copy.

[Vincent-X-Zhang]

I see!

class prototype

public class Block
{
    public long Length {get;set;}
    public ReadOnlySequence<byte> Data { get; set; }
}

the way to use

var toUse = block.Data.Slice(0, 1000);
await WriteAsync(toUse.ToArray());

or use pool:

var toUse = ArrayPool<byte>.Share.Rent(1000);
block.Data.Slice(0, 1000).CopyTo(toUse);
try
{
    await WriteAsync(toUse);
}
finally
{
    ArrayPool<byte>.Return(toUse);
}

[Clockwork-Muse]

What? No.

You don't need the class (since the slice would already be length-limited), and whatever WriteAsync(...) is should be taking ReadOnlyMemory<byte> which you can get directly from (iterating through) the sequence, so there's no reason to get an array copy of the data either.

Also, in some cases sending an array to another method is supposed to relinquish ownership of it, so it may not be safe to return the array after passing it to another method.

That said:

1. If all you're doing is writing the bytes somewhere, you'd just pass the stream through, or use CopyToAsync(...), and not bother with anything at this point in the pipeline. Using pipes is mostly meant for de/serializing, and if you're not doing that you'd just skip chunking it.
2. What I would expect to write would have some signature similar to:

   private bool TryParseDomainObject(ref ReadOnlySequence<byte> data, out TDomainObject? do)
   {
       // internals complicated
   }

   ... and you keep doing backoff/lengthening the sequence until it's long enough to parse for whatever (portion of) the domain object you're working on.

The entire point is to avoid copies to other buffers (and definitely creation of other arrays), so if you're creating an array of bytes and sending or saving it somewhere you're probably not making the best use of pipelines.

[Vincent-X-Zhang]

Thank you very much for your generous answer!