Skip to content

Commit

Permalink
http.sys (6.0 backport): new option for response buffering (#48072)
Browse files Browse the repository at this point in the history
* Merged PR 30089: HttpSys: new option for response buffering

# {PR title}

HttpSys: new option for response buffering

## Description

{Detail}

As per [old PR 418](aspnet/HttpSysServer#418) (direct copy), enables kernel mode response-buffering in http.sys - with modification to use app-context switch rather than public API change

## Customer Impact

Without this buffer, writes are sent as-written direct thru http.sys; this has two problems:

- fragmentation if writes are small
- issues with latency

## Regression?

- [ ] Yes
- [x] No

[If yes, specify the version the behavior has regressed from]

## Risk

- [ ] High
- [ ] Medium
- [x] Low - opt in, no impact to consumers not using this flag

[Justify the selection above]

## Verification

- [x] Manual (required) - will try to verify with internal MSFT consumer (targeting net6.0)
- [ ] Automated

## Packaging changes reviewed?

- [ ] Yes
- [ ] No
- [x] N/A

----

## When servicing release/2.1

- [ ] Make necessary changes in eng/PatchConfig.props

* Update src/Servers/HttpSys/src/HttpSysOptions.cs

Co-authored-by: Aditya Mandaleeka <adityamandaleeka@users.noreply.github.com>

---------

Co-authored-by: Marc Gravell <marcgravell@microsoft.com>
Co-authored-by: Aditya Mandaleeka <adityamandaleeka@users.noreply.github.com>
  • Loading branch information
3 people authored May 8, 2023
1 parent d3043cf commit 832ccdc
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 2 deletions.
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 in pre-8.0

/// <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; },
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

0 comments on commit 832ccdc

Please sign in to comment.