Use FileStream buffering only when it makes sense

[adamsitnik]

Whenever buffering is enabled, we need to allocate BufferedFileStreamStrategy

runtime/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/FileStreamHelpers.Windows.cs

Lines 51 to 52 in 0af5228

	internal static FileStreamStrategy EnableBufferingIfNeeded(WindowsFileStreamStrategy strategy, int bufferSize)
	=> bufferSize == 1 ? strategy : new BufferedFileStreamStrategy(strategy, bufferSize);

which has a finalizer (that is an additional work for the GC):

runtime/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/BufferedFileStreamStrategy.cs

Line 32 in 0af5228

~BufferedFileStreamStrategy()

and aquires lock for every ReadAsync and WriteAsync:

runtime/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/BufferedFileStreamStrategy.cs

Line 346 in 0af5228

SemaphoreSlim semaphore = EnsureAsyncActiveSemaphoreInitialized();

runtime/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/BufferedFileStreamStrategy.cs

Line 641 in 0af5228

SemaphoreSlim semaphore = EnsureAsyncActiveSemaphoreInitialized();

this is of course done for some good reasons, but the thing is that we don't always need the buffering.

A good example is File.WriteAllTextAsync where we create a FileStream with buffering enabled:

runtime/src/libraries/System.IO.FileSystem/src/System/IO/File.cs

Lines 678 to 680 in 0af5228

	FileStream stream = new FileStream(
	path, append ? FileMode.Append : FileMode.Create, FileAccess.Write, FileShare.Read, DefaultBufferSize,
	FileOptions.Asynchronous | FileOptions.SequentialScan);

but later use buffers that are at least as big as the internal FileStream buffer (so we never take advantage of buffering)

runtime/src/libraries/System.IO.FileSystem/src/System/IO/File.cs

Line 979 in 0af5228

buffer = ArrayPool<char>.Shared.Rent(DefaultBufferSize);

to write to the file:

runtime/src/libraries/System.IO.FileSystem/src/System/IO/File.cs

Line 989 in 0af5228

await sw.WriteAsync(new ReadOnlyMemory<char>(buffer, 0, batchSize), cancellationToken).ConfigureAwait(false);

The person who is willing to work on this task should:

1. Search the entire runtime repo for places where buffering is enabled (bufferSize of the FileStream ctor is not set to 1 in an explicit way), but it hurts the performance like in the example above. If possible, please verify that FileStream used for async IO is created with FileOptions.Asynchronous or isAsync: true flag.
2. Contribute benchmarks to the dotnet/performance repo after reading the Microbenchmark Design Guidelines doc. The benchmarks need to cover the code that is about to be changed.
3. Run the benchmarks following the Benchmarking workflow for dotnet/runtime repository doc and ensure the changes are improving perf
4. Contribute the improvements to dotnet/runtime and provide benchmark results

[ghost]

Tagging subscribers to this area: @carlossanlop
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Whenever buffering is enabled, we need to allocate BufferedFileStreamStrategy

runtime/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/FileStreamHelpers.Windows.cs

Lines 51 to 52 in 0af5228

	internal static FileStreamStrategy EnableBufferingIfNeeded(WindowsFileStreamStrategy strategy, int bufferSize)
	=> bufferSize == 1 ? strategy : new BufferedFileStreamStrategy(strategy, bufferSize);

which has a finalizer (that is an additional work for the GC):

runtime/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/BufferedFileStreamStrategy.cs

Line 32 in 0af5228

~BufferedFileStreamStrategy()

and aquires lock for every ReadAsync and WriteAsync:

runtime/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/BufferedFileStreamStrategy.cs

Line 346 in 0af5228

SemaphoreSlim semaphore = EnsureAsyncActiveSemaphoreInitialized();

runtime/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/BufferedFileStreamStrategy.cs

Line 641 in 0af5228

SemaphoreSlim semaphore = EnsureAsyncActiveSemaphoreInitialized();

this is of course done for some good reasons, but the thing is that we don't always need the buffering.

A good example is File.WriteAllTextAsync where we create a FileStream with buffering enabled:

runtime/src/libraries/System.IO.FileSystem/src/System/IO/File.cs

Lines 678 to 680 in 0af5228

	FileStream stream = new FileStream(
	path, append ? FileMode.Append : FileMode.Create, FileAccess.Write, FileShare.Read, DefaultBufferSize,
	FileOptions.Asynchronous | FileOptions.SequentialScan);

but later use buffers that are at least as big as the internal FileStream buffer (so we never take advantage of buffering)

runtime/src/libraries/System.IO.FileSystem/src/System/IO/File.cs

Line 979 in 0af5228

buffer = ArrayPool<char>.Shared.Rent(DefaultBufferSize);

to write to the file:

runtime/src/libraries/System.IO.FileSystem/src/System/IO/File.cs

Line 989 in 0af5228

await sw.WriteAsync(new ReadOnlyMemory<char>(buffer, 0, batchSize), cancellationToken).ConfigureAwait(false);

The person who is willing to work on this task should:

1. Search the entire runtime repo for places where buffering is enabled (bufferSize of the FileStream ctor is not set to 1 in an explicit way), but it hurts the performance like in the example above.
2. Contribute benchmarks to the dotnet/performance repo after reading the Microbenchmark Design Guidelines doc. The benchmarks need to cover the code that is about to be changed.
3. Run the benchmarks following the Benchmarking workflow for dotnet/runtime repository doc and ensure the changes are improving perf
4. Contribute the improvements to dotnet/runtime and provide benchmark results

Author:	adamsitnik
Assignees:	-
Labels:	area-System.IO, tenet-performance, up-for-grabs
Milestone:	6.0.0

[huoyaoyuan]

1. If possible, please verify that FileStream used for async IO is created with FileOptions.Asynchronous or isAsync: true flag.

Related to #45946 (comment) . Currently using isAsync forces the user to provide a buffer size, which doesn't have a help to determine. It's very difficult to learn that 1 is optimal now.

[adamsitnik]

It's very difficult to learn that 1 is optimal now.

I acknowledge the fact that setting bufferSize to 1 is not intuitive and a trick known to very few engineers.

Part of our .NET 6 FileStream work is to publish a doc about performance best practices. We are going to explain what buffering is, how it's different from the buffering done by the OS and how to disable it.

[sakno]

According to the current source code in main branch, this statement

I acknowledge the fact that setting bufferSize to 1 is not intuitive and a trick known to very few engineers.

is incorrect for Unix-like platforms. ChooseStrategy method always creates Net5CompatFileStreamStrategy without analysis of bufferSize argument as for Windows.

[stephentoub]

is incorrect for Unix-like platforms

It's not incorrect. The Net5CompatFileStreamStrategy lazily creates the buffer only if it's needed. With a _bufferSize of 1, any non-empty user buffer will result in skipping the buffer and it won't be lazily created. This is the case for both Windows and Unix in .NET 5, and has been for a long time.