microsoft / Microsoft.IO.RecyclableMemoryStream

A library to provide pooling for .NET MemoryStream objects to improve application performance.
MIT License
1.98k stars 205 forks source link

How effective is this package for buffers that are usually smaller than 256 bytes? #307

Closed Leonardo-Ferreira closed 11 months ago

Leonardo-Ferreira commented 1 year ago

I can clearly measure the benefit of this package for buffers larger than 8kb, but what about those smaller than 256 bytes?

My scenario is that I use buffers to buff incoming http request/response in order to log the request body, pretty much like this:

private async Task LogRequest(HttpContext context)
{
    context.Request.EnableBuffering();
    await using var requestStream = _recyclableMemoryStreamManager.GetStream();
    await context.Request.Body.CopyToAsync(requestStream);
    _logger.LogInformation(ReadStreamInChunks(requestStream));
    context.Request.Body.Position = 0;
}
private static string ReadStreamInChunks(Stream stream)
{
    const int readChunkBufferLength = 256; //my avg request body size is about 100 bytes and 95% are smaller than 1kb
    stream.Seek(0, SeekOrigin.Begin);
    using var textWriter = new StringWriter();
    using var reader = new StreamReader(stream);
    var readChunk = new char[readChunkBufferLength];
    int readChunkLength;
    do
    {
        readChunkLength = reader.ReadBlock(readChunk, 0, readChunkBufferLength);
        textWriter.Write(readChunk, 0, readChunkLength);
    } while (readChunkLength > 0);
    return textWriter.ToString();
}

A single pod usually handles peaks of about 1000 requests per minute (17 req/s)...

so I was wondering how effective is this on my scenario?

benmwatson commented 1 year ago

The only real answer is that you would have to test and see. 256 bytes is pretty small, but you have a lot of them.

I suspect it would still be beneficial--you'll avoid creating a ton of buffers that will trigger GC. OTOH, as long as their in-use time was quite short, they would be cleaned up in gen0 anyway, so it might be a wash.