-
Notifications
You must be signed in to change notification settings - Fork 10.4k
Use PipeWriter to write files #24851
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,7 @@ | |
using System.Buffers; | ||
using System.Diagnostics; | ||
using System.IO; | ||
using System.IO.Pipelines; | ||
using System.Threading; | ||
using System.Threading.Tasks; | ||
|
||
|
@@ -13,7 +14,7 @@ namespace Microsoft.AspNetCore.Http | |
// FYI: In most cases the source will be a FileStream and the destination will be to the network. | ||
internal static class StreamCopyOperationInternal | ||
{ | ||
private const int DefaultBufferSize = 4096; | ||
private const int DefaultBufferSize = 4 * 1024; | ||
|
||
/// <summary>Asynchronously reads the given number of bytes from the source stream and writes them to another stream.</summary> | ||
/// <returns>A task that represents the asynchronous copy operation.</returns> | ||
|
@@ -42,13 +43,13 @@ public static async Task CopyToAsync(Stream source, Stream destination, long? co | |
{ | ||
Debug.Assert(source != null); | ||
Debug.Assert(destination != null); | ||
Debug.Assert(!bytesRemaining.HasValue || bytesRemaining.GetValueOrDefault() >= 0); | ||
Debug.Assert(!bytesRemaining.HasValue || bytesRemaining.Value >= 0); | ||
Debug.Assert(buffer != null); | ||
|
||
while (true) | ||
{ | ||
// The natural end of the range. | ||
if (bytesRemaining.HasValue && bytesRemaining.GetValueOrDefault() <= 0) | ||
if (bytesRemaining.HasValue && bytesRemaining.Value <= 0) | ||
{ | ||
return; | ||
} | ||
|
@@ -58,7 +59,7 @@ public static async Task CopyToAsync(Stream source, Stream destination, long? co | |
int readLength = buffer.Length; | ||
if (bytesRemaining.HasValue) | ||
{ | ||
readLength = (int)Math.Min(bytesRemaining.GetValueOrDefault(), (long)readLength); | ||
readLength = (int)Math.Min(bytesRemaining.Value, (long)readLength); | ||
} | ||
int read = await source.ReadAsync(buffer, 0, readLength, cancel); | ||
|
||
|
@@ -83,5 +84,66 @@ public static async Task CopyToAsync(Stream source, Stream destination, long? co | |
ArrayPool<byte>.Shared.Return(buffer); | ||
} | ||
} | ||
|
||
/// <summary>Asynchronously reads the given number of bytes from the source stream and writes them using pipe writer.</summary> | ||
/// <returns>A task that represents the asynchronous copy operation.</returns> | ||
/// <param name="source">The stream from which the contents will be copied.</param> | ||
/// <param name="writer">The PipeWriter to which the contents of the current stream will be copied.</param> | ||
/// <param name="count">The count of bytes to be copied.</param> | ||
/// <param name="cancel">The token to monitor for cancellation requests.</param> | ||
public static Task CopyToAsync(Stream source, PipeWriter writer, long? count, CancellationToken cancel) | ||
{ | ||
if (count == null) | ||
{ | ||
// No length, do a copy with the default buffer size (based on whatever the pipe settings are, default is 4K) | ||
return source.CopyToAsync(writer, cancel); | ||
} | ||
|
||
static async Task CopyToAsync(Stream source, PipeWriter writer, long bytesRemaining, CancellationToken cancel) | ||
{ | ||
// The array pool likes powers of 2 | ||
const int maxBufferSize = 64 * 1024; | ||
const int minBufferSize = 1024; | ||
|
||
// We know exactly how much we're going to copy | ||
while (bytesRemaining > 0) | ||
{ | ||
var bufferSize = (int)Math.Clamp(bytesRemaining, minBufferSize, maxBufferSize); | ||
|
||
// The natural end of the range. | ||
var memory = writer.GetMemory(bufferSize); | ||
|
||
if (memory.Length > bytesRemaining) | ||
{ | ||
memory = memory.Slice(0, (int)bytesRemaining); | ||
} | ||
|
||
var read = await source.ReadAsync(memory, cancel); | ||
|
||
bytesRemaining -= read; | ||
|
||
// End of the source stream. | ||
if (read == 0) | ||
{ | ||
break; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should you advance here to get the writer in a working state? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that only matters if you write something. At least in the Pipe implementation. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I thought if you call GetMemory, you can't call GetMemory afterwards without advancing. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nope. |
||
} | ||
|
||
writer.Advance(read); | ||
|
||
var result = await writer.FlushAsync(cancel); | ||
|
||
if (result.IsCompleted) | ||
{ | ||
break; | ||
} | ||
} | ||
} | ||
|
||
Debug.Assert(source != null); | ||
Debug.Assert(writer != null); | ||
Debug.Assert(count >= 0); | ||
|
||
return CopyToAsync(source, writer, count.Value, cancel); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
@ECHO OFF | ||
|
||
%~dp0..\..\..\startvs.cmd %~dp0StaticFiles.slnf |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -360,19 +360,19 @@ protected static ILogger CreateLogger<T>(ILoggerFactory factory) | |
|
||
protected static async Task WriteFileAsync(HttpContext context, Stream fileStream, RangeItemHeaderValue range, long rangeLength) | ||
{ | ||
var outputStream = context.Response.Body; | ||
using (fileStream) | ||
var outputPipeWriter = context.Response.BodyWriter; | ||
await using (fileStream) | ||
{ | ||
try | ||
{ | ||
if (range == null) | ||
{ | ||
await StreamCopyOperation.CopyToAsync(fileStream, outputStream, count: null, bufferSize: BufferSize, cancel: context.RequestAborted); | ||
await StreamCopyOperation.CopyToAsync(fileStream, outputPipeWriter, count: null, cancel: context.RequestAborted); | ||
} | ||
else | ||
{ | ||
fileStream.Seek(range.From.Value, SeekOrigin.Begin); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does here also need a check for |
||
await StreamCopyOperation.CopyToAsync(fileStream, outputStream, rangeLength, BufferSize, context.RequestAborted); | ||
await StreamCopyOperation.CopyToAsync(fileStream, outputPipeWriter, rangeLength, context.RequestAborted); | ||
} | ||
} | ||
catch (OperationCanceledException) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did you change these to
Value
? Value checks whether the value is null and throws an error if it is.GetValueOrDefault
returns whatever the value is, making it faster.