Skip to content

Commit 010cbfe

Browse files
jozkeeadamsitnik
andauthored
FileStream optimizations (#49975)
* don't verify OS handle position * track the file offset in memory, don't use expensive syscalls to synchronize it * there is no need to set the Length since we are now tracking the offset in memory * Cache GetFileInformationByHandleEx (Length) when FileShare does not allow other processes to modify it Co-authored-by: Adam Sitnik <[email protected]>
1 parent 411c8a1 commit 010cbfe

File tree

11 files changed

+143
-142
lines changed

11 files changed

+143
-142
lines changed

src/libraries/Common/src/Interop/Windows/Kernel32/Interop.ReadFile_SafeHandle_NativeOverlapped.cs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,5 +16,13 @@ internal static extern unsafe int ReadFile(
1616
int numBytesToRead,
1717
IntPtr numBytesRead_mustBeZero,
1818
NativeOverlapped* overlapped);
19+
20+
[DllImport(Libraries.Kernel32, SetLastError = true)]
21+
internal static extern unsafe int ReadFile(
22+
SafeHandle handle,
23+
byte* bytes,
24+
int numBytesToRead,
25+
out int numBytesRead,
26+
NativeOverlapped* overlapped);
1927
}
2028
}

src/libraries/Common/src/Interop/Windows/Kernel32/Interop.WriteFile_SafeHandle_NativeOverlapped.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,5 +17,8 @@ internal static partial class Kernel32
1717
// and pass in an address for the numBytesRead parameter.
1818
[DllImport(Libraries.Kernel32, SetLastError = true)]
1919
internal static extern unsafe int WriteFile(SafeHandle handle, byte* bytes, int numBytesToWrite, IntPtr numBytesWritten_mustBeZero, NativeOverlapped* lpOverlapped);
20+
21+
[DllImport(Libraries.Kernel32, SetLastError = true)]
22+
internal static extern unsafe int WriteFile(SafeHandle handle, byte* bytes, int numBytesToWrite, out int numBytesWritten, NativeOverlapped* lpOverlapped);
2023
}
2124
}

src/libraries/System.IO.FileSystem/tests/FileStream/SafeFileHandle.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,13 +66,13 @@ public void AccessFlushesFileClosesHandle()
6666
}
6767
}
6868

69-
[Fact]
69+
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsLegacyFileStreamEnabled))]
7070
public async Task ThrowWhenHandlePositionIsChanged_sync()
7171
{
7272
await ThrowWhenHandlePositionIsChanged(useAsync: false);
7373
}
7474

75-
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))]
75+
[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported), nameof(PlatformDetection.IsLegacyFileStreamEnabled))]
7676
public async Task ThrowWhenHandlePositionIsChanged_async()
7777
{
7878
await ThrowWhenHandlePositionIsChanged(useAsync: true);

src/libraries/System.Private.CoreLib/src/System/IO/FileStream.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ public FileStream(IntPtr handle, FileAccess access, bool ownsHandle, int bufferS
4747
{
4848
ValidateHandle(safeHandle, access, bufferSize, isAsync);
4949

50-
_strategy = FileStreamHelpers.ChooseStrategy(this, safeHandle, access, bufferSize, isAsync);
50+
_strategy = FileStreamHelpers.ChooseStrategy(this, safeHandle, access, DefaultShare, bufferSize, isAsync);
5151
}
5252
catch
5353
{
@@ -101,7 +101,7 @@ public FileStream(SafeFileHandle handle, FileAccess access, int bufferSize, bool
101101
{
102102
ValidateHandle(handle, access, bufferSize, isAsync);
103103

104-
_strategy = FileStreamHelpers.ChooseStrategy(this, handle, access, bufferSize, isAsync);
104+
_strategy = FileStreamHelpers.ChooseStrategy(this, handle, access, DefaultShare, bufferSize, isAsync);
105105
}
106106

107107
public FileStream(string path, FileMode mode)

src/libraries/System.Private.CoreLib/src/System/IO/Strategies/AsyncWindowsFileStreamStrategy.cs

Lines changed: 23 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@ internal sealed partial class AsyncWindowsFileStreamStrategy : WindowsFileStream
1313
private PreAllocatedOverlapped? _preallocatedOverlapped; // optimization for async ops to avoid per-op allocations
1414
private FileStreamCompletionSource? _currentOverlappedOwner; // async op currently using the preallocated overlapped
1515

16-
internal AsyncWindowsFileStreamStrategy(SafeFileHandle handle, FileAccess access)
17-
: base(handle, access)
16+
internal AsyncWindowsFileStreamStrategy(SafeFileHandle handle, FileAccess access, FileShare share)
17+
: base(handle, access, share)
1818
{
1919
}
2020

@@ -98,7 +98,6 @@ protected override void OnInit()
9898
if (_fileHandle.ThreadPoolBinding == null)
9999
{
100100
// We should close the handle so that the handle is not open until SafeFileHandle GC
101-
Debug.Assert(!_exposedHandle, "Are we closing handle that we exposed/not own, how?");
102101
_fileHandle.Dispose();
103102
}
104103
}
@@ -142,18 +141,16 @@ private unsafe Task<int> ReadAsyncInternal(Memory<byte> destination, Cancellatio
142141
NativeOverlapped* intOverlapped = completionSource.Overlapped;
143142

144143
// Calculate position in the file we should be at after the read is done
144+
long positionBefore = _filePosition;
145145
if (CanSeek)
146146
{
147147
long len = Length;
148148

149-
// Make sure we are reading from the position that we think we are
150-
VerifyOSHandlePosition();
151-
152-
if (destination.Length > len - _filePosition)
149+
if (positionBefore + destination.Length > len)
153150
{
154-
if (_filePosition <= len)
151+
if (positionBefore <= len)
155152
{
156-
destination = destination.Slice(0, (int)(len - _filePosition));
153+
destination = destination.Slice(0, (int)(len - positionBefore));
157154
}
158155
else
159156
{
@@ -163,23 +160,17 @@ private unsafe Task<int> ReadAsyncInternal(Memory<byte> destination, Cancellatio
163160

164161
// Now set the position to read from in the NativeOverlapped struct
165162
// For pipes, we should leave the offset fields set to 0.
166-
intOverlapped->OffsetLow = unchecked((int)_filePosition);
167-
intOverlapped->OffsetHigh = (int)(_filePosition >> 32);
163+
intOverlapped->OffsetLow = unchecked((int)positionBefore);
164+
intOverlapped->OffsetHigh = (int)(positionBefore >> 32);
168165

169166
// When using overlapped IO, the OS is not supposed to
170167
// touch the file pointer location at all. We will adjust it
171-
// ourselves. This isn't threadsafe.
172-
173-
// WriteFile should not update the file pointer when writing
174-
// in overlapped mode, according to MSDN. But it does update
175-
// the file pointer when writing to a UNC path!
176-
// So changed the code below to seek to an absolute
177-
// location, not a relative one. ReadFile seems consistent though.
178-
SeekCore(_fileHandle, destination.Length, SeekOrigin.Current);
168+
// ourselves, but only in memory. This isn't threadsafe.
169+
_filePosition += destination.Length;
179170
}
180171

181172
// queue an async ReadFile operation and pass in a packed overlapped
182-
int r = FileStreamHelpers.ReadFileNative(_fileHandle, destination.Span, intOverlapped, out int errorCode);
173+
int r = FileStreamHelpers.ReadFileNative(_fileHandle, destination.Span, false, intOverlapped, out int errorCode);
183174

184175
// ReadFile, the OS version, will return 0 on failure. But
185176
// my ReadFileNative wrapper returns -1. My wrapper will return
@@ -208,7 +199,7 @@ private unsafe Task<int> ReadAsyncInternal(Memory<byte> destination, Cancellatio
208199
{
209200
if (!_fileHandle.IsClosed && CanSeek) // Update Position - It could be anywhere.
210201
{
211-
SeekCore(_fileHandle, 0, SeekOrigin.Current);
202+
_filePosition = positionBefore;
212203
}
213204

214205
completionSource.ReleaseNativeResource();
@@ -269,32 +260,23 @@ private unsafe Task WriteAsyncInternalCore(ReadOnlyMemory<byte> source, Cancella
269260
FileStreamCompletionSource completionSource = FileStreamCompletionSource.Create(this, _preallocatedOverlapped, 0, source);
270261
NativeOverlapped* intOverlapped = completionSource.Overlapped;
271262

263+
long positionBefore = _filePosition;
272264
if (CanSeek)
273265
{
274-
// Make sure we set the length of the file appropriately.
275-
long len = Length;
276-
277-
// Make sure we are writing to the position that we think we are
278-
VerifyOSHandlePosition();
279-
280-
if (_filePosition + source.Length > len)
281-
{
282-
SetLengthCore(_filePosition + source.Length);
283-
}
284-
285266
// Now set the position to read from in the NativeOverlapped struct
286267
// For pipes, we should leave the offset fields set to 0.
287-
intOverlapped->OffsetLow = (int)_filePosition;
288-
intOverlapped->OffsetHigh = (int)(_filePosition >> 32);
268+
intOverlapped->OffsetLow = (int)positionBefore;
269+
intOverlapped->OffsetHigh = (int)(positionBefore >> 32);
289270

290271
// When using overlapped IO, the OS is not supposed to
291272
// touch the file pointer location at all. We will adjust it
292-
// ourselves. This isn't threadsafe.
293-
SeekCore(_fileHandle, source.Length, SeekOrigin.Current);
273+
// ourselves, but only in memory. This isn't threadsafe.
274+
_filePosition += source.Length;
275+
UpdateLengthOnChangePosition();
294276
}
295277

296278
// queue an async WriteFile operation and pass in a packed overlapped
297-
int r = FileStreamHelpers.WriteFileNative(_fileHandle, source.Span, intOverlapped, out int errorCode);
279+
int r = FileStreamHelpers.WriteFileNative(_fileHandle, source.Span, false, intOverlapped, out int errorCode);
298280

299281
// WriteFile, the OS version, will return 0 on failure. But
300282
// my WriteFileNative wrapper returns -1. My wrapper will return
@@ -320,7 +302,7 @@ private unsafe Task WriteAsyncInternalCore(ReadOnlyMemory<byte> source, Cancella
320302
{
321303
if (!_fileHandle.IsClosed && CanSeek) // Update Position - It could be anywhere.
322304
{
323-
SeekCore(_fileHandle, 0, SeekOrigin.Current);
305+
_filePosition = positionBefore;
324306
}
325307

326308
completionSource.ReleaseNativeResource();
@@ -382,24 +364,18 @@ private async Task AsyncModeCopyToAsync(Stream destination, int bufferSize, Canc
382364
Debug.Assert(!_fileHandle.IsClosed, "!_handle.IsClosed");
383365
Debug.Assert(CanRead, "_parent.CanRead");
384366

385-
bool canSeek = CanSeek;
386-
if (canSeek)
387-
{
388-
VerifyOSHandlePosition();
389-
}
390-
391367
try
392368
{
393369
await FileStreamHelpers
394-
.AsyncModeCopyToAsync(_fileHandle, _path, canSeek, _filePosition, destination, bufferSize, cancellationToken)
370+
.AsyncModeCopyToAsync(_fileHandle, _path, CanSeek, _filePosition, destination, bufferSize, cancellationToken)
395371
.ConfigureAwait(false);
396372
}
397373
finally
398374
{
399375
// Make sure the stream's current position reflects where we ended up
400-
if (!_fileHandle.IsClosed && canSeek)
376+
if (!_fileHandle.IsClosed && CanSeek)
401377
{
402-
SeekCore(_fileHandle, 0, SeekOrigin.End);
378+
_filePosition = Length;
403379
}
404380
}
405381
}

src/libraries/System.Private.CoreLib/src/System/IO/Strategies/FileStreamHelpers.Unix.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ namespace System.IO.Strategies
1414
internal static partial class FileStreamHelpers
1515
{
1616
// in the future we are most probably going to introduce more strategies (io_uring etc)
17-
private static FileStreamStrategy ChooseStrategyCore(SafeFileHandle handle, FileAccess access, int bufferSize, bool isAsync)
17+
private static FileStreamStrategy ChooseStrategyCore(SafeFileHandle handle, FileAccess access, FileShare share, int bufferSize, bool isAsync)
1818
=> new LegacyFileStreamStrategy(handle, access, bufferSize, isAsync);
1919

2020
private static FileStreamStrategy ChooseStrategyCore(string path, FileMode mode, FileAccess access, FileShare share, int bufferSize, FileOptions options)

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

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,16 +19,16 @@ internal static partial class FileStreamHelpers
1919
private const int ERROR_HANDLE_EOF = 38;
2020
private const int ERROR_IO_PENDING = 997;
2121

22-
private static FileStreamStrategy ChooseStrategyCore(SafeFileHandle handle, FileAccess access, int bufferSize, bool isAsync)
22+
private static FileStreamStrategy ChooseStrategyCore(SafeFileHandle handle, FileAccess access, FileShare share, int bufferSize, bool isAsync)
2323
{
2424
if (UseLegacyStrategy)
2525
{
2626
return new LegacyFileStreamStrategy(handle, access, bufferSize, isAsync);
2727
}
2828

2929
WindowsFileStreamStrategy strategy = isAsync
30-
? new AsyncWindowsFileStreamStrategy(handle, access)
31-
: new SyncWindowsFileStreamStrategy(handle, access);
30+
? new AsyncWindowsFileStreamStrategy(handle, access, share)
31+
: new SyncWindowsFileStreamStrategy(handle, access, share);
3232

3333
return EnableBufferingIfNeeded(strategy, bufferSize);
3434
}
@@ -333,7 +333,7 @@ internal static unsafe void SetFileLength(SafeFileHandle handle, string? path, l
333333
}
334334

335335
// __ConsoleStream also uses this code.
336-
internal static unsafe int ReadFileNative(SafeFileHandle handle, Span<byte> bytes, NativeOverlapped* overlapped, out int errorCode)
336+
internal static unsafe int ReadFileNative(SafeFileHandle handle, Span<byte> bytes, bool syncUsingOverlapped, NativeOverlapped* overlapped, out int errorCode)
337337
{
338338
Debug.Assert(handle != null, "handle != null");
339339

@@ -343,13 +343,24 @@ internal static unsafe int ReadFileNative(SafeFileHandle handle, Span<byte> byte
343343
fixed (byte* p = &MemoryMarshal.GetReference(bytes))
344344
{
345345
r = overlapped != null ?
346-
Interop.Kernel32.ReadFile(handle, p, bytes.Length, IntPtr.Zero, overlapped) :
347-
Interop.Kernel32.ReadFile(handle, p, bytes.Length, out numBytesRead, IntPtr.Zero);
346+
(syncUsingOverlapped
347+
? Interop.Kernel32.ReadFile(handle, p, bytes.Length, out numBytesRead, overlapped)
348+
: Interop.Kernel32.ReadFile(handle, p, bytes.Length, IntPtr.Zero, overlapped))
349+
: Interop.Kernel32.ReadFile(handle, p, bytes.Length, out numBytesRead, IntPtr.Zero);
348350
}
349351

350352
if (r == 0)
351353
{
352354
errorCode = GetLastWin32ErrorAndDisposeHandleIfInvalid(handle);
355+
356+
if (syncUsingOverlapped && errorCode == Interop.Errors.ERROR_HANDLE_EOF)
357+
{
358+
// https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-readfile#synchronization-and-file-position :
359+
// "If lpOverlapped is not NULL, then when a synchronous read operation reaches the end of a file,
360+
// ReadFile returns FALSE and GetLastError returns ERROR_HANDLE_EOF"
361+
return numBytesRead;
362+
}
363+
353364
return -1;
354365
}
355366
else
@@ -359,7 +370,7 @@ internal static unsafe int ReadFileNative(SafeFileHandle handle, Span<byte> byte
359370
}
360371
}
361372

362-
internal static unsafe int WriteFileNative(SafeFileHandle handle, ReadOnlySpan<byte> buffer, NativeOverlapped* overlapped, out int errorCode)
373+
internal static unsafe int WriteFileNative(SafeFileHandle handle, ReadOnlySpan<byte> buffer, bool syncUsingOverlapped, NativeOverlapped* overlapped, out int errorCode)
363374
{
364375
Debug.Assert(handle != null, "handle != null");
365376

@@ -369,8 +380,10 @@ internal static unsafe int WriteFileNative(SafeFileHandle handle, ReadOnlySpan<b
369380
fixed (byte* p = &MemoryMarshal.GetReference(buffer))
370381
{
371382
r = overlapped != null ?
372-
Interop.Kernel32.WriteFile(handle, p, buffer.Length, IntPtr.Zero, overlapped) :
373-
Interop.Kernel32.WriteFile(handle, p, buffer.Length, out numBytesWritten, IntPtr.Zero);
383+
(syncUsingOverlapped
384+
? Interop.Kernel32.WriteFile(handle, p, buffer.Length, out numBytesWritten, overlapped)
385+
: Interop.Kernel32.WriteFile(handle, p, buffer.Length, IntPtr.Zero, overlapped))
386+
: Interop.Kernel32.WriteFile(handle, p, buffer.Length, out numBytesWritten, IntPtr.Zero);
374387
}
375388

376389
if (r == 0)
@@ -460,7 +473,7 @@ internal static async Task AsyncModeCopyToAsync(SafeFileHandle handle, string? p
460473
}
461474

462475
// Kick off the read.
463-
synchronousSuccess = ReadFileNative(handle, copyBuffer, readAwaitable._nativeOverlapped, out errorCode) >= 0;
476+
synchronousSuccess = ReadFileNative(handle, copyBuffer, false, readAwaitable._nativeOverlapped, out errorCode) >= 0;
464477
}
465478

466479
// If the operation did not synchronously succeed, it either failed or initiated the asynchronous operation.

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@ private static bool GetLegacyFileStreamSetting()
2323
: bool.IsTrueStringIgnoreCase(envVar) || envVar.Equals("1");
2424
}
2525

26-
internal static FileStreamStrategy ChooseStrategy(FileStream fileStream, SafeFileHandle handle, FileAccess access, int bufferSize, bool isAsync)
27-
=> WrapIfDerivedType(fileStream, ChooseStrategyCore(handle, access, bufferSize, isAsync));
26+
internal static FileStreamStrategy ChooseStrategy(FileStream fileStream, SafeFileHandle handle, FileAccess access, FileShare share, int bufferSize, bool isAsync)
27+
=> WrapIfDerivedType(fileStream, ChooseStrategyCore(handle, access, share, bufferSize, isAsync));
2828

2929
internal static FileStreamStrategy ChooseStrategy(FileStream fileStream, string path, FileMode mode, FileAccess access, FileShare share, int bufferSize, FileOptions options)
3030
=> WrapIfDerivedType(fileStream, ChooseStrategyCore(path, mode, access, share, bufferSize, options));

src/libraries/System.Private.CoreLib/src/System/IO/Strategies/LegacyFileStreamStrategy.Windows.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1079,14 +1079,14 @@ private unsafe int ReadFileNative(SafeFileHandle handle, Span<byte> bytes, Nativ
10791079
{
10801080
Debug.Assert((_useAsyncIO && overlapped != null) || (!_useAsyncIO && overlapped == null), "Async IO and overlapped parameters inconsistent in call to ReadFileNative.");
10811081

1082-
return FileStreamHelpers.ReadFileNative(handle, bytes, overlapped, out errorCode);
1082+
return FileStreamHelpers.ReadFileNative(handle, bytes, false, overlapped, out errorCode);
10831083
}
10841084

10851085
private unsafe int WriteFileNative(SafeFileHandle handle, ReadOnlySpan<byte> buffer, NativeOverlapped* overlapped, out int errorCode)
10861086
{
10871087
Debug.Assert((_useAsyncIO && overlapped != null) || (!_useAsyncIO && overlapped == null), "Async IO and overlapped parameters inconsistent in call to WriteFileNative.");
10881088

1089-
return FileStreamHelpers.WriteFileNative(handle, buffer, overlapped, out errorCode);
1089+
return FileStreamHelpers.WriteFileNative(handle, buffer, false, overlapped, out errorCode);
10901090
}
10911091

10921092
public override Task CopyToAsync(Stream destination, int bufferSize, CancellationToken cancellationToken)

0 commit comments

Comments
 (0)