Skip to content
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

http.sys (6.0 backport): new option for response buffering #48072

Merged
merged 2 commits into from
May 8, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions src/Servers/HttpSys/src/HttpSysOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ public class HttpSysOptions
/// </summary>
public HttpSysOptions()
{
const string EnableKernelResponseBufferingSwitch = "Microsoft.AspNetCore.Server.HttpSys.EnableKernelResponseBuffering";
EnableKernelResponseBuffering = AppContext.TryGetSwitch(EnableKernelResponseBufferingSwitch, out var enabled) && enabled;
}

/// <summary>
Expand Down Expand Up @@ -109,6 +111,15 @@ public string? RequestQueueName
/// </summary>
public bool ThrowWriteExceptions { get; set; }

/// <summary>
/// Enable buffering of response data in the Kernel.
/// It should be used by an application doing synchronous I/O or by an application doing asynchronous I/O with
/// no more than one outstanding write at a time, and can significantly improve throughput over high-latency connections.
/// Applications that use asynchronous I/O and that may have more than one send outstanding at a time should not use this flag.
/// Enabling this can results in higher CPU and memory usage by Http.Sys.
/// </summary>
internal bool EnableKernelResponseBuffering { get; set; } // internal via app-context for non-public release
mgravell marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// Gets or sets the maximum number of concurrent connections to accept, -1 for infinite, or null to
/// use the machine wide setting from the registry. The default value is null.
Expand Down
2 changes: 0 additions & 2 deletions src/Servers/HttpSys/src/RequestProcessing/Response.cs
Original file line number Diff line number Diff line change
Expand Up @@ -280,8 +280,6 @@ actual HTTP header will be generated by the application and sent as entity body.
// This will give us more control of the bytes that hit the wire, including encodings, HTTP 1.0, etc..
// It may also be faster to do this work in managed code and then pass down only one buffer.
// What would we loose by bypassing HttpSendHttpResponse?
//
// TODO: Consider using the HTTP_SEND_RESPONSE_FLAG_BUFFER_DATA flag for most/all responses rather than just Opaque.
internal unsafe uint SendHeaders(HttpApiTypes.HTTP_DATA_CHUNK[]? dataChunks,
ResponseStreamAsyncResult? asyncResult,
HttpApiTypes.HTTP_FLAGS flags,
Expand Down
9 changes: 9 additions & 0 deletions src/Servers/HttpSys/src/RequestProcessing/ResponseBody.cs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ internal RequestContext RequestContext

internal bool ThrowWriteExceptions => RequestContext.Server.Options.ThrowWriteExceptions;

internal bool EnableKernelResponseBuffering => RequestContext.Server.Options.EnableKernelResponseBuffering;

internal bool IsDisposed => _disposed;

public override bool CanSeek
Expand Down Expand Up @@ -466,6 +468,13 @@ private HttpApiTypes.HTTP_FLAGS ComputeLeftToWrite(long writeCount, bool endOfRe
{
flags |= HttpApiTypes.HTTP_FLAGS.HTTP_SEND_RESPONSE_FLAG_MORE_DATA;
}
if (EnableKernelResponseBuffering)
{
// "When this flag is set, it should also be used consistently in calls to the HttpSendResponseEntityBody function."
// so: make sure we add it in *all* scenarios where it applies - our "close" could be at the end of a bunch
// of buffered chunks
flags |= HttpApiTypes.HTTP_FLAGS.HTTP_SEND_RESPONSE_FLAG_BUFFER_DATA;
}

// Update _leftToWrite now so we can queue up additional async writes.
if (_leftToWrite > 0)
Expand Down
35 changes: 35 additions & 0 deletions src/Servers/HttpSys/test/FunctionalTests/ResponseBodyTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,41 @@ public async Task ResponseBody_WriteNoHeaders_SetsChunked()
}
}

[ConditionalTheory]
[InlineData(true)]
[InlineData(false)]
public async Task ResponseBody_WriteNoHeaders_SetsChunked_LargeBody(bool enableKernelBuffering)
{
const int WriteSize = 1024 * 1024;
const int NumWrites = 32;

string address;
using (Utilities.CreateHttpServer(
baseAddress: out address,
configureOptions: options => { options.EnableKernelResponseBuffering = enableKernelBuffering; },
JamesNK marked this conversation as resolved.
Show resolved Hide resolved
app: async httpContext =>
{
httpContext.Features.Get<IHttpBodyControlFeature>().AllowSynchronousIO = true;
for (int i = 0; i < NumWrites - 1; i++)
{
httpContext.Response.Body.Write(new byte[WriteSize], 0, WriteSize);
}
await httpContext.Response.Body.WriteAsync(new byte[WriteSize], 0, WriteSize);
}))
{
var response = await SendRequestAsync(address);
Assert.Equal(200, (int)response.StatusCode);
Assert.Equal(new Version(1, 1), response.Version);
IEnumerable<string> ignored;
Assert.False(response.Content.Headers.TryGetValues("content-length", out ignored), "Content-Length");
Assert.True(response.Headers.TransferEncodingChunked.HasValue, "Chunked");

var bytes = await response.Content.ReadAsByteArrayAsync();
Assert.Equal(WriteSize * NumWrites, bytes.Length);
Assert.True(bytes.All(b => b == 0));
}
}

[ConditionalFact]
public async Task ResponseBody_WriteNoHeadersAndFlush_DefaultsToChunked()
{
Expand Down