From ea52899e119bff9b1759eedf0749128b60a97f06 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Mon, 3 Apr 2023 17:04:32 +0000 Subject: [PATCH 1/2] Merged PR 30089: HttpSys: new option for response buffering # {PR title} HttpSys: new option for response buffering ## Description {Detail} As per [old PR 418](https://github.com/aspnet/HttpSysServer/pull/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 --- src/Servers/HttpSys/src/HttpSysOptions.cs | 11 ++++++ .../HttpSys/src/RequestProcessing/Response.cs | 2 -- .../src/RequestProcessing/ResponseBody.cs | 9 +++++ .../test/FunctionalTests/ResponseBodyTests.cs | 35 +++++++++++++++++++ 4 files changed, 55 insertions(+), 2 deletions(-) diff --git a/src/Servers/HttpSys/src/HttpSysOptions.cs b/src/Servers/HttpSys/src/HttpSysOptions.cs index 76e4a9009b70..b89b2288e4aa 100644 --- a/src/Servers/HttpSys/src/HttpSysOptions.cs +++ b/src/Servers/HttpSys/src/HttpSysOptions.cs @@ -35,6 +35,8 @@ public class HttpSysOptions /// public HttpSysOptions() { + const string EnableKernelResponseBufferingSwitch = "Microsoft.AspNetCore.Server.HttpSys.EnableKernelResponseBuffering"; + EnableKernelResponseBuffering = AppContext.TryGetSwitch(EnableKernelResponseBufferingSwitch, out var enabled) && enabled; } /// @@ -109,6 +111,15 @@ public string? RequestQueueName /// public bool ThrowWriteExceptions { get; set; } + /// + /// 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. + /// + internal bool EnableKernelResponseBuffering { get; set; } // internal via app-context for non-public release + /// /// 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. diff --git a/src/Servers/HttpSys/src/RequestProcessing/Response.cs b/src/Servers/HttpSys/src/RequestProcessing/Response.cs index e7ad52826825..e602693769ea 100644 --- a/src/Servers/HttpSys/src/RequestProcessing/Response.cs +++ b/src/Servers/HttpSys/src/RequestProcessing/Response.cs @@ -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, diff --git a/src/Servers/HttpSys/src/RequestProcessing/ResponseBody.cs b/src/Servers/HttpSys/src/RequestProcessing/ResponseBody.cs index c8a9ffffd825..b81d4cac6bc7 100644 --- a/src/Servers/HttpSys/src/RequestProcessing/ResponseBody.cs +++ b/src/Servers/HttpSys/src/RequestProcessing/ResponseBody.cs @@ -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 @@ -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) diff --git a/src/Servers/HttpSys/test/FunctionalTests/ResponseBodyTests.cs b/src/Servers/HttpSys/test/FunctionalTests/ResponseBodyTests.cs index 061aa6cdf629..b8358ae3c0b3 100644 --- a/src/Servers/HttpSys/test/FunctionalTests/ResponseBodyTests.cs +++ b/src/Servers/HttpSys/test/FunctionalTests/ResponseBodyTests.cs @@ -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().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 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() { From 4d4ce88ae3373716a9576253effc38d1160ad46d Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Sat, 6 May 2023 00:25:33 +0100 Subject: [PATCH 2/2] Update src/Servers/HttpSys/src/HttpSysOptions.cs Co-authored-by: Aditya Mandaleeka --- src/Servers/HttpSys/src/HttpSysOptions.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Servers/HttpSys/src/HttpSysOptions.cs b/src/Servers/HttpSys/src/HttpSysOptions.cs index b89b2288e4aa..ed4b7a92cdc7 100644 --- a/src/Servers/HttpSys/src/HttpSysOptions.cs +++ b/src/Servers/HttpSys/src/HttpSysOptions.cs @@ -118,7 +118,7 @@ public string? RequestQueueName /// 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. /// - internal bool EnableKernelResponseBuffering { get; set; } // internal via app-context for non-public release + internal bool EnableKernelResponseBuffering { get; set; } // internal via app-context in pre-8.0 /// /// Gets or sets the maximum number of concurrent connections to accept, -1 for infinite, or null to