-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Closed
Labels
api-approvedAPI was approved in API review, it can be implementedAPI was approved in API review, it can be implementedarea-System.IOtenet-performancePerformance related issuePerformance related issue
Milestone
Description
Background and Motivation
We have recently approved one new FileStream constructor argument: long allocationSize (#45946 (comment)) that got implemented, but has not been merged yet (#51111).
As pointed by @carlossanlop and @stephentoub, we should consider adding options bag instead.
An extra motivation for adding the option bag is a new feature request (#52321) for allowing to pass Memory<byte> as a buffer. This would help us to solve #15088 and literally remove the last problematic allocation from FileStream.
Proposed API
namespace System.IO
{
+ public readonly struct FileStreamOptions
+ {
+ public string Path { get; init; }
+ public FileMode Mode { get; init; }
+ public FileAccess Access { get; init; }
+ public FileShare Share { get; init; }
+ public Memory<byte>? Buffer { get; init; } // default value == null => use default buffer size and allocate the buffer (current behaviour)
+ public FileOptions Options { get; init; }
+ public long PreAllocationSize { get; init; }
+ }
public class FileStream : Stream
{
public FileStream(string path, FileMode mode)
public FileStream(string path, FileMode mode, FileAccess access)
public FileStream(string path, FileMode mode, FileAccess access, FileShare share)
public FileStream(string path, FileMode mode, FileAccess access, FileShare share, int bufferSize)
public FileStream(string path, FileMode mode, FileAccess access, FileShare share, int bufferSize, bool useAsync)
public FileStream(string path, FileMode mode, FileAccess access, FileShare share, int bufferSize, FileOptions options)
+ public FileStream(FileStreamOptions options)
+ [EditorBrowsable(EditorBrowsableState.Never)]
[Obsolete("This constructor has been deprecated. Please use new FileStream(SafeFileHandle handle, FileAccess access) instead. https://go.microsoft.com/fwlink/?linkid=14202")]
public FileStream(IntPtr handle, FileAccess access)
+ [EditorBrowsable(EditorBrowsableState.Never)]
[Obsolete("This constructor has been deprecated. Please use new FileStream(SafeFileHandle handle, FileAccess access) instead, and optionally make a new SafeFileHandle with ownsHandle=false if needed. https://go.microsoft.com/fwlink/?linkid=14202")]
public FileStream(IntPtr handle, FileAccess access, bool ownsHandle)
+ [EditorBrowsable(EditorBrowsableState.Never)]
[Obsolete("This constructor has been deprecated. Please use new FileStream(SafeFileHandle handle, FileAccess access, int bufferSize) instead, and optionally make a new SafeFileHandle with ownsHandle=false if needed. https://go.microsoft.com/fwlink/?linkid=14202")]
public FileStream(IntPtr handle, FileAccess access, bool ownsHandle, int bufferSize)
+ [EditorBrowsable(EditorBrowsableState.Never)]
[Obsolete("This constructor has been deprecated. Please use new FileStream(SafeFileHandle handle, FileAccess access, int bufferSize, bool isAsync) instead, and optionally make a new SafeFileHandle with ownsHandle=false if needed. https://go.microsoft.com/fwlink/?linkid=14202")]
public FileStream(IntPtr handle, FileAccess access, bool ownsHandle, int bufferSize, bool isAsync)
}Usage Examples
// Opening file for Read:
var basic = new FileStreamOptions
{
Path = @"C:\FrameworkDesignGuidelines.pdf",
Mode = FileMode.Open,
Access = FileAccess.Read,
};
using FileStream read = new FileStream(basic);
// creating new file for async write with allocation size and buffer provided by the user
byte[] array = ArrayPool<byte>.Shared.Rent(4096);
var advanced = new FileStreamOptions
{
Path = @"C:\copy.pdf",
Mode = FileMode.CreateNew,
Access = FileAccess.Write,
Share = FileShare.None,
PreAllocationSize = read.Length,
Options = FileOptions.Asynchronous | FileOptions.WriteThrough,
Buffer = array
};
using FileStream write = new FileStream(advanced);
read.CopyTo(write);
ArrayPool<byte>.Shared.Return(array);
// To disable the buffering, users would have to pass a default or empty `Memory<byte>`:
var noBuffering = new FileStreamOptions
{
Buffer = default(Memory<byte>) // Array.Empty<byte>() would also work
};Alternative Designs
Don't let the user provide the buffer (to minimize risk of misuse), but instead provide bufferSize and extend FileOptions with PoolBuffer:
namespace System.IO
{
+ public readonly struct FileStreamOptions
+ {
+ public string Path { get; init; }
+ public FileMode Mode { get; init; }
+ public FileAccess Access { get; init; }
+ public FileShare Share { get; init; }
+ public int BufferSize { get; init; } // the difference
+ public FileOptions Options { get; init; }
+ public long PreAllocationSize { get; init; }
+ }
public class FileStream : Stream
{
public FileStream(string path, FileMode mode)
public FileStream(string path, FileMode mode, FileAccess access)
public FileStream(string path, FileMode mode, FileAccess access, FileShare share)
public FileStream(string path, FileMode mode, FileAccess access, FileShare share, int bufferSize)
public FileStream(string path, FileMode mode, FileAccess access, FileShare share, int bufferSize, bool useAsync)
public FileStream(string path, FileMode mode, FileAccess access, FileShare share, int bufferSize, FileOptions options)
+ public FileStream(FileStreamOptions options)
}
public enum FileOptions
{
WriteThrough,
None,
Encrypted,
DeleteOnClose,
SequentialScan,
RandomAccess,
Asynchronous,
+ PoolBuffer // new option
}Risks
Allowing the users to pass the buffer creates the risk of misusing the buffer by the user:
- not returning a rented array to the pool and exhausting the
ArrayPool - freeing the native memory that
Memory<byte>wraps when it's still being used byFileStream
saucecontrol, sakno, dmex, danmoseley, antiufo and 6 more
Metadata
Metadata
Assignees
Labels
api-approvedAPI was approved in API review, it can be implementedAPI was approved in API review, it can be implementedarea-System.IOtenet-performancePerformance related issuePerformance related issue