From 2e8d61f9d01c8eb7700a37bbf19a73296293be98 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 18 May 2021 19:44:28 +0200 Subject: [PATCH 1/3] extend FileStreamOptions with BufferSize --- .../tests/FileStream/ctor_options_as.cs | 12 ++++++++++++ .../src/System/IO/FileStream.cs | 6 ++++-- .../src/System/IO/FileStreamOptions.cs | 5 +++++ src/libraries/System.Runtime/ref/System.Runtime.cs | 1 + 4 files changed, 22 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/tests/FileStream/ctor_options_as.cs b/src/libraries/System.IO.FileSystem/tests/FileStream/ctor_options_as.cs index d98090b37f67e..a202814d7f256 100644 --- a/src/libraries/System.IO.FileSystem/tests/FileStream/ctor_options_as.cs +++ b/src/libraries/System.IO.FileSystem/tests/FileStream/ctor_options_as.cs @@ -27,6 +27,18 @@ protected override FileStream CreateFileStream(string path, FileMode mode, FileA PreallocationSize = PreallocationSize }); + protected override FileStream CreateFileStream(string path, FileMode mode, FileAccess access, FileShare share, int bufferSize, FileOptions options) + => new FileStream(path, + new FileStreamOptions + { + Mode = mode, + Access = access, + Share = share, + BufferSize = bufferSize, + Options = options, + PreallocationSize = PreallocationSize + }); + protected FileStreamOptions GetOptions(FileMode mode, FileAccess access, FileShare share, FileOptions options, long preAllocationSize) => new FileStreamOptions { diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileStream.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileStream.cs index 26e90f4467d0b..c9ababf9255a6 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileStream.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileStream.cs @@ -149,7 +149,9 @@ public FileStream(string path, FileMode mode, FileAccess access, FileShare share /// -or- /// refers to a non-file device, such as CON:, COM1:, LPT1:, etc. in an NTFS environment. /// refers to a non-file device, such as CON:, COM1:, LPT1:, etc. in a non-NTFS environment. - /// is negative. + /// is negative. + /// -or- + /// is negative. /// -or- /// , , or contain an invalid value. /// The file cannot be found, such as when is or , and the file specified by does not exist. The file must already exist in these modes. @@ -167,7 +169,7 @@ public FileStream(string path, FileMode mode, FileAccess access, FileShare share /// is specified for , but file encryption is not supported on the current platform. /// The specified path, file name, or both exceed the system-defined maximum length. public FileStream(string path, FileStreamOptions options) - : this(path, options.Mode, options.Access, options.Share, DefaultBufferSize, options.Options, options.PreallocationSize) + : this(path, options.Mode, options.Access, options.Share, options.BufferSize, options.Options, options.PreallocationSize) { } diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileStreamOptions.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileStreamOptions.cs index c7dccfd216109..afb78900c3677 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileStreamOptions.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileStreamOptions.cs @@ -27,5 +27,10 @@ public sealed class FileStreamOptions /// In other cases (including the default 0 value), it's ignored. /// public long PreallocationSize { get; set; } + /// + /// The size of the buffer used by for buffering. The default buffer size is 4096. + /// 0 or 1 means that buffering should be disabled. Negative values are not allowed. + /// + public int BufferSize { get; set; } = 4096; } } diff --git a/src/libraries/System.Runtime/ref/System.Runtime.cs b/src/libraries/System.Runtime/ref/System.Runtime.cs index 12c4068842242..24ade3a67ab62 100644 --- a/src/libraries/System.Runtime/ref/System.Runtime.cs +++ b/src/libraries/System.Runtime/ref/System.Runtime.cs @@ -7307,6 +7307,7 @@ public sealed class FileStreamOptions public System.IO.FileShare Share { get; set; } public System.IO.FileOptions Options { get; set; } public long PreallocationSize { get; set; } + public int BufferSize { get; set; } } public partial class FileStream : System.IO.Stream { From 2d304b40b09440fdb0374b03e5c0de5b12a715a5 Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Tue, 18 May 2021 20:04:32 +0200 Subject: [PATCH 2/3] allow to disable the buffering by setting bufferSize to 0 --- .../tests/FileStream/FileStreamConformanceTests.cs | 14 ++++++++++++-- .../tests/FileStream/ctor_str_fm_fa_fs_buffer.cs | 7 ++++--- .../src/System/IO/FileStream.cs | 2 +- .../System/IO/Strategies/FileStreamHelpers.Unix.cs | 4 ++-- .../IO/Strategies/FileStreamHelpers.Windows.cs | 8 +++++--- 5 files changed, 24 insertions(+), 11 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/tests/FileStream/FileStreamConformanceTests.cs b/src/libraries/System.IO.FileSystem/tests/FileStream/FileStreamConformanceTests.cs index f086c2cc23609..0fbc771104569 100644 --- a/src/libraries/System.IO.FileSystem/tests/FileStream/FileStreamConformanceTests.cs +++ b/src/libraries/System.IO.FileSystem/tests/FileStream/FileStreamConformanceTests.cs @@ -112,7 +112,7 @@ public async Task NoDataIsLostWhenWritingToFile(ReadWriteMode mode) filePath = stream.Name; // the following buffer fits into internal FileStream buffer - byte[] small = Enumerable.Repeat(byte.MinValue, BufferSize - 1).ToArray(); + byte[] small = Enumerable.Repeat(byte.MinValue, Math.Max(1, BufferSize - 1)).ToArray(); // the following buffer does not fit into internal FileStream buffer byte[] big = Enumerable.Repeat(byte.MaxValue, BufferSize + 1).ToArray(); // in this test we are selecting a random buffer and write it to file @@ -175,7 +175,7 @@ public async Task WriteByteFlushesTheBufferWhenItBecomesFull() stream.WriteByte(0); writtenBytes.Add(0); - byte[] bytes = new byte[BufferSize - 1]; + byte[] bytes = new byte[Math.Max(0, BufferSize - 1)]; stream.Write(bytes.AsSpan()); writtenBytes.AddRange(bytes); @@ -191,7 +191,12 @@ public async Task WriteByteFlushesTheBufferWhenItBecomesFull() public class UnbufferedSyncFileStreamStandaloneConformanceTests : FileStreamStandaloneConformanceTests { protected override FileOptions Options => FileOptions.None; + +#if RELEASE // since buffering can be now disabled by setting the buffer size to 0 or 1, let's test 0 in one config and 1 in the other + protected override int BufferSize => 0; +#else protected override int BufferSize => 1; +#endif } public class BufferedSyncFileStreamStandaloneConformanceTests : FileStreamStandaloneConformanceTests @@ -205,7 +210,12 @@ public class BufferedSyncFileStreamStandaloneConformanceTests : FileStreamStanda public class UnbufferedAsyncFileStreamStandaloneConformanceTests : FileStreamStandaloneConformanceTests { protected override FileOptions Options => FileOptions.Asynchronous; + +#if RELEASE + protected override int BufferSize => 0; +#else protected override int BufferSize => 1; +#endif } [ActiveIssue("https://github.com/dotnet/runtime/issues/34583", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)] diff --git a/src/libraries/System.IO.FileSystem/tests/FileStream/ctor_str_fm_fa_fs_buffer.cs b/src/libraries/System.IO.FileSystem/tests/FileStream/ctor_str_fm_fa_fs_buffer.cs index 3885149161cad..621586be7fb7d 100644 --- a/src/libraries/System.IO.FileSystem/tests/FileStream/ctor_str_fm_fa_fs_buffer.cs +++ b/src/libraries/System.IO.FileSystem/tests/FileStream/ctor_str_fm_fa_fs_buffer.cs @@ -26,10 +26,11 @@ public void NegativeBufferSizeThrows() } [Fact] - public void ZeroBufferSizeThrows() + public void ZeroBufferSizeDoesNotThrow() { - // Unfortunate pre-existing behavior of FileStream, we should look into enabling this sometime. - AssertExtensions.Throws("bufferSize", () => CreateFileStream(GetTestFilePath(), FileMode.Create, FileAccess.ReadWrite, FileShare.Read, 0)); + using (CreateFileStream(GetTestFilePath(), FileMode.Create, FileAccess.ReadWrite, FileShare.Read, 0)) + { + } } [Fact] diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileStream.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileStream.cs index c9ababf9255a6..b3b2791d84367 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileStream.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileStream.cs @@ -211,7 +211,7 @@ private FileStream(string path, FileMode mode, FileAccess access, FileShare shar { throw new ArgumentOutOfRangeException(nameof(options), SR.ArgumentOutOfRange_Enum); } - else if (bufferSize <= 0) + else if (bufferSize < 0) { throw new ArgumentOutOfRangeException(nameof(bufferSize), SR.ArgumentOutOfRange_NeedPosNum); } diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/FileStreamHelpers.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/FileStreamHelpers.Unix.cs index 176429b41fe50..1fd6456f3eeb5 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/FileStreamHelpers.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/FileStreamHelpers.Unix.cs @@ -15,10 +15,10 @@ internal static partial class FileStreamHelpers { // in the future we are most probably going to introduce more strategies (io_uring etc) private static FileStreamStrategy ChooseStrategyCore(SafeFileHandle handle, FileAccess access, FileShare share, int bufferSize, bool isAsync) - => new Net5CompatFileStreamStrategy(handle, access, bufferSize, isAsync); + => new Net5CompatFileStreamStrategy(handle, access, bufferSize == 0 ? 1 : bufferSize, isAsync); private static FileStreamStrategy ChooseStrategyCore(string path, FileMode mode, FileAccess access, FileShare share, int bufferSize, FileOptions options, long preallocationSize) - => new Net5CompatFileStreamStrategy(path, mode, access, share, bufferSize, options, preallocationSize); + => new Net5CompatFileStreamStrategy(path, mode, access, share, bufferSize == 0 ? 1 : bufferSize, options, preallocationSize); internal static SafeFileHandle OpenHandle(string path, FileMode mode, FileAccess access, FileShare share, FileOptions options, long preallocationSize) { diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/FileStreamHelpers.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/FileStreamHelpers.Windows.cs index 13dcc7cfb17e4..efdc550574ed1 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/FileStreamHelpers.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/FileStreamHelpers.Windows.cs @@ -32,7 +32,9 @@ private static FileStreamStrategy ChooseStrategyCore(SafeFileHandle handle, File { if (UseNet5CompatStrategy) { - return new Net5CompatFileStreamStrategy(handle, access, bufferSize, isAsync); + // The .NET 5 Compat strategy does not support bufferSize == 0. + // To minimize the risk of introducing bugs to it, we just pass 1 to disable the buffering. + return new Net5CompatFileStreamStrategy(handle, access, bufferSize == 0 ? 1 : bufferSize, isAsync); } WindowsFileStreamStrategy strategy = isAsync @@ -46,7 +48,7 @@ private static FileStreamStrategy ChooseStrategyCore(string path, FileMode mode, { if (UseNet5CompatStrategy) { - return new Net5CompatFileStreamStrategy(path, mode, access, share, bufferSize, options, preallocationSize); + return new Net5CompatFileStreamStrategy(path, mode, access, share, bufferSize == 0 ? 1 : bufferSize, options, preallocationSize); } WindowsFileStreamStrategy strategy = (options & FileOptions.Asynchronous) != 0 @@ -57,7 +59,7 @@ private static FileStreamStrategy ChooseStrategyCore(string path, FileMode mode, } internal static FileStreamStrategy EnableBufferingIfNeeded(WindowsFileStreamStrategy strategy, int bufferSize) - => bufferSize == 1 ? strategy : new BufferedFileStreamStrategy(strategy, bufferSize); + => bufferSize <= 1 ? strategy : new BufferedFileStreamStrategy(strategy, bufferSize); internal static SafeFileHandle OpenHandle(string path, FileMode mode, FileAccess access, FileShare share, FileOptions options, long preallocationSize) => CreateFileOpenHandle(path, mode, access, share, options, preallocationSize); From ad3513639e3344d76cb8db4eead96342ce4abaed Mon Sep 17 00:00:00 2001 From: Adam Sitnik Date: Wed, 19 May 2021 07:12:10 +0200 Subject: [PATCH 3/3] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: David Cantú --- .../System.Private.CoreLib/src/System/IO/FileStreamOptions.cs | 2 +- .../src/System/IO/Strategies/FileStreamHelpers.Windows.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileStreamOptions.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileStreamOptions.cs index afb78900c3677..b2279d4c9256e 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileStreamOptions.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileStreamOptions.cs @@ -31,6 +31,6 @@ public sealed class FileStreamOptions /// The size of the buffer used by for buffering. The default buffer size is 4096. /// 0 or 1 means that buffering should be disabled. Negative values are not allowed. /// - public int BufferSize { get; set; } = 4096; + public int BufferSize { get; set; } = FileStream.DefaultBufferSize; } } diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/FileStreamHelpers.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/FileStreamHelpers.Windows.cs index efdc550574ed1..f9f22b4d698f0 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/FileStreamHelpers.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/Strategies/FileStreamHelpers.Windows.cs @@ -59,7 +59,7 @@ private static FileStreamStrategy ChooseStrategyCore(string path, FileMode mode, } internal static FileStreamStrategy EnableBufferingIfNeeded(WindowsFileStreamStrategy strategy, int bufferSize) - => bufferSize <= 1 ? strategy : new BufferedFileStreamStrategy(strategy, bufferSize); + => bufferSize > 1 ? new BufferedFileStreamStrategy(strategy, bufferSize) : strategy; internal static SafeFileHandle OpenHandle(string path, FileMode mode, FileAccess access, FileShare share, FileOptions options, long preallocationSize) => CreateFileOpenHandle(path, mode, access, share, options, preallocationSize);