dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.45k stars 4.76k forks source link

[Optimization] BufferedFileStreamStrategy creates buffer, but can use ArrayPool<byte>.Shared #107260

Open TrayanZapryanov opened 2 months ago

TrayanZapryanov commented 2 months ago

I've found that this strategy popups quite high in my memory profiling when generating zips. Is it possible to replace with renting from pool and in Dispose method to return it back?

https://github.com/dotnet/runtime/blob/4d501d90ec35ae9872e711760cead299e2da22bf/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/BufferedFileStreamStrategy.cs#L1037

Here it is how my allocations looks like: image

dotnet-policy-service[bot] commented 2 months ago

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

vcsjones commented 2 months ago

There is some prior discussion on pooling here: https://github.com/dotnet/runtime/issues/64632#issuecomment-1066744359

and why it was not implemented.

TrayanZapryanov commented 2 months ago

Thank you very much. But is it possible to add internal ctor with buffer and reuse it within ZipArchive functionality? This way at least the number of instances will be equal to the number of zips.

adamsitnik commented 2 months ago

But is it possible to add internal ctor with buffer and reuse it within ZipArchive functionality?

FileStream and compression-related tyoes live in separate libraries, so we can't use internal ctors.

But... perhaps we should measure how much perf we would gain/loose if ZipArchive has simply disabled the FileStream buffering?

@TrayanZapryanov is there any chance you would be interesting in running such benchmarks?

Here we have a doc that describes how to benchmark local build of dotnet/runtime: https://github.com/dotnet/performance/blob/main/docs/benchmarking-workflow-dotnet-runtime.md

TrayanZapryanov commented 2 months ago

Sure, I will measure. Just not until 15th, as I will be in a vacation.