diff --git a/src/libraries/System.IO.FileSystem/tests/RandomAccess/NoBuffering.Windows.cs b/src/libraries/System.IO.FileSystem/tests/RandomAccess/NoBuffering.Windows.cs index 8bb39e4b2893f5..ef60a9f223fb28 100644 --- a/src/libraries/System.IO.FileSystem/tests/RandomAccess/NoBuffering.Windows.cs +++ b/src/libraries/System.IO.FileSystem/tests/RandomAccess/NoBuffering.Windows.cs @@ -180,18 +180,17 @@ public async Task WriteAsyncUsingMultipleBuffers(bool asyncOperation, bool async Assert.Equal(content, File.ReadAllBytes(filePath)); } - [Fact] - public async Task ReadWriteAsyncUsingNonPageSizedMultipleBuffers() + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task ReadWriteAsyncUsingMultipleBuffers(bool memoryPageSized) { string filePath = GetTestFilePath(); - // The Windows scatter/gather APIs accept segments that are exactly one page long. - // Combined with the FILE_FLAG_NO_BUFFERING's requirements, the segments must also - // be aligned at page size boundaries and have a size of a multiple of the page size. - // Using segments with a length of twice the page size adheres to the second requirement - // but not the first. The RandomAccess implementation will see it and issue sequential - // read/write syscalls per segment, instead of one scatter/gather syscall. - // This test verifies that fallback behavior. - int bufferSize = Environment.SystemPageSize * 2; + // We test with buffers both one and two memory pages long. In the former case, + // the I/O operations will issue one scatter/gather API call, and in the latter + // case they will issue multiple calls; one per buffer. The buffers must still + // be aligned to comply with FILE_FLAG_NO_BUFFERING's requirements. + int bufferSize = Environment.SystemPageSize * (memoryPageSized ? 1 : 2); int fileSize = bufferSize * 2; byte[] content = RandomNumberGenerator.GetBytes(fileSize); @@ -205,10 +204,22 @@ public async Task ReadWriteAsyncUsingNonPageSizedMultipleBuffers() await RandomAccess.WriteAsync(handle, new ReadOnlyMemory[] { firstHalf, secondHalf }, 0); buffer.GetSpan().Clear(); - await RandomAccess.ReadAsync(handle, new Memory[] { firstHalf, secondHalf }, 0); + long nRead = await RandomAccess.ReadAsync(handle, new Memory[] { firstHalf, secondHalf }, 0); + + Assert.Equal(buffer.GetSpan().Length, nRead); + AssertExtensions.SequenceEqual(buffer.GetSpan(), content.AsSpan()); } + } + + [Fact] + public async Task ReadWriteAsyncUsingEmptyBuffers() + { + string filePath = GetTestFilePath(); + using SafeFileHandle handle = File.OpenHandle(filePath, FileMode.CreateNew, FileAccess.ReadWrite, FileShare.None, FileOptions.Asynchronous | NoBuffering); - Assert.Equal(content, await File.ReadAllBytesAsync(filePath)); + long nRead = await RandomAccess.ReadAsync(handle, Array.Empty>(), 0); + Assert.Equal(0, nRead); + await RandomAccess.WriteAsync(handle, Array.Empty>(), 0); } // when using FileOptions.Asynchronous we are testing Scatter&Gather APIs on Windows (FILE_FLAG_OVERLAPPED requirement) diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Windows.cs index 31adaca38fe5ee..01707557d8a783 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/RandomAccess.Windows.cs @@ -4,6 +4,7 @@ using System.Buffers; using System.Collections.Generic; using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using System.IO.Strategies; using System.Numerics; using System.Runtime.InteropServices; @@ -437,8 +438,8 @@ private static bool CanUseScatterGatherWindowsAPIs(SafeFileHandle handle) // The pinned MemoryHandles and the pointer to the segments must be cleaned-up // with the CleanupScatterGatherBuffers method. private static unsafe bool TryPrepareScatterGatherBuffers(IReadOnlyList buffers, - THandler handler, out MemoryHandle[] handlesToDispose, out IntPtr segmentsPtr, out int totalBytes) - where THandler: struct, IMemoryHandler + THandler handler, [NotNullWhen(true)] out MemoryHandle[]? handlesToDispose, out IntPtr segmentsPtr, out int totalBytes) + where THandler : struct, IMemoryHandler { int pageSize = s_cachedPageSize; Debug.Assert(BitOperations.IsPow2(pageSize), "Page size is not a power of two."); @@ -447,13 +448,11 @@ private static unsafe bool TryPrepareScatterGatherBuffers(IReadOnly long alignedAtPageSizeMask = pageSize - 1; int buffersCount = buffers.Count; - handlesToDispose = new MemoryHandle[buffersCount]; + handlesToDispose = null; segmentsPtr = IntPtr.Zero; totalBytes = 0; - // "The array must contain enough elements to store nNumberOfBytesToWrite bytes of data, and one element for the terminating NULL. " - long* segments = (long*) NativeMemory.Alloc((nuint)(buffersCount + 1), sizeof(long)); - segments[buffersCount] = 0; + long* segments = null; bool success = false; try @@ -469,36 +468,55 @@ private static unsafe bool TryPrepareScatterGatherBuffers(IReadOnly return false; } - MemoryHandle handle = handlesToDispose[i] = handler.Pin(in buffer); - long ptr = segments[i] = (long)handle.Pointer; + MemoryHandle handle = handler.Pin(in buffer); + long ptr = (long)handle.Pointer; if ((ptr & alignedAtPageSizeMask) != 0) { + handle.Dispose(); return false; } + + // We avoid allocations if there are no + // buffers or the first one is unacceptable. + (handlesToDispose ??= new MemoryHandle[buffersCount])[i] = handle; + if (segments == null) + { + // "The array must contain enough elements to store nNumberOfBytesToWrite + // bytes of data, and one element for the terminating NULL." + segments = (long*)NativeMemory.Alloc((nuint)buffersCount + 1, sizeof(long)); + segments[buffersCount] = 0; + } + segments[i] = ptr; } segmentsPtr = (IntPtr)segments; totalBytes = (int)totalBytes64; success = true; - return true; + return handlesToDispose != null; } finally { if (!success) { - CleanupScatterGatherBuffers(handlesToDispose, (IntPtr) segments); + CleanupScatterGatherBuffers(handlesToDispose, (IntPtr)segments); } } } - private static unsafe void CleanupScatterGatherBuffers(MemoryHandle[] handlesToDispose, IntPtr segmentsPtr) + private static unsafe void CleanupScatterGatherBuffers(MemoryHandle[]? handlesToDispose, IntPtr segmentsPtr) { - foreach (MemoryHandle handle in handlesToDispose) + if (handlesToDispose != null) { - handle.Dispose(); + foreach (MemoryHandle handle in handlesToDispose) + { + handle.Dispose(); + } } - NativeMemory.Free((void*) segmentsPtr); + if (segmentsPtr != IntPtr.Zero) + { + NativeMemory.Free((void*)segmentsPtr); + } } private static ValueTask ReadScatterAtOffsetAsync(SafeFileHandle handle, IReadOnlyList> buffers, @@ -510,7 +528,7 @@ private static ValueTask ReadScatterAtOffsetAsync(SafeFileHandle handle, I } if (CanUseScatterGatherWindowsAPIs(handle) - && TryPrepareScatterGatherBuffers(buffers, default(MemoryHandler), out MemoryHandle[] handlesToDispose, out IntPtr segmentsPtr, out int totalBytes)) + && TryPrepareScatterGatherBuffers(buffers, default(MemoryHandler), out MemoryHandle[]? handlesToDispose, out IntPtr segmentsPtr, out int totalBytes)) { return ReadScatterAtOffsetSingleSyscallAsync(handle, handlesToDispose, segmentsPtr, fileOffset, totalBytes, cancellationToken); } @@ -607,7 +625,7 @@ private static ValueTask WriteGatherAtOffsetAsync(SafeFileHandle handle, IReadOn } if (CanUseScatterGatherWindowsAPIs(handle) - && TryPrepareScatterGatherBuffers(buffers, default(ReadOnlyMemoryHandler), out MemoryHandle[] handlesToDispose, out IntPtr segmentsPtr, out int totalBytes)) + && TryPrepareScatterGatherBuffers(buffers, default(ReadOnlyMemoryHandler), out MemoryHandle[]? handlesToDispose, out IntPtr segmentsPtr, out int totalBytes)) { return WriteGatherAtOffsetSingleSyscallAsync(handle, handlesToDispose, segmentsPtr, fileOffset, totalBytes, cancellationToken); } @@ -671,13 +689,12 @@ private static unsafe ValueTask WriteFileGatherAsync(SafeFileHandle handle, IntP private static async ValueTask WriteGatherAtOffsetMultipleSyscallsAsync(SafeFileHandle handle, IReadOnlyList> buffers, long fileOffset, CancellationToken cancellationToken) { - long bytesWritten = 0; int buffersCount = buffers.Count; for (int i = 0; i < buffersCount; i++) { ReadOnlyMemory rom = buffers[i]; - await WriteAtOffsetAsync(handle, rom, fileOffset + bytesWritten, cancellationToken).ConfigureAwait(false); - bytesWritten += rom.Length; + await WriteAtOffsetAsync(handle, rom, fileOffset, cancellationToken).ConfigureAwait(false); + fileOffset += rom.Length; } }