From a8b466b32140da1ba367fa30dab8743993ba454c Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Wed, 19 Apr 2023 14:31:23 +0100 Subject: [PATCH 1/3] http.sys; opt-in support for kernel-mode response buffering --- src/Servers/HttpSys/src/HttpSysOptions.cs | 13 ++++++ .../HttpSys/src/PublicAPI.Unshipped.txt | 2 + .../HttpSys/src/RequestProcessing/Response.cs | 2 - .../src/RequestProcessing/ResponseBody.cs | 9 ++++ .../test/FunctionalTests/ResponseBodyTests.cs | 42 +++++++++++++++---- 5 files changed, 59 insertions(+), 9 deletions(-) diff --git a/src/Servers/HttpSys/src/HttpSysOptions.cs b/src/Servers/HttpSys/src/HttpSysOptions.cs index 636c4aa2b6c9..c6667e622f96 100644 --- a/src/Servers/HttpSys/src/HttpSysOptions.cs +++ b/src/Servers/HttpSys/src/HttpSysOptions.cs @@ -34,6 +34,10 @@ public class HttpSysOptions /// public HttpSysOptions() { + // this feature is being back-ported to net6/net7 via an app-context switch; respect that even on net8+ to avoid + // surprises when upgrading TFMs + const string EnableKernelResponseBufferingSwitch = "Microsoft.AspNetCore.Server.HttpSys.EnableKernelResponseBuffering"; + EnableKernelResponseBuffering = AppContext.TryGetSwitch(EnableKernelResponseBufferingSwitch, out var enabled) && enabled; } /// @@ -110,6 +114,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. + /// + public bool EnableKernelResponseBuffering { get; set; } // internal via app-context for non-public release + /// /// Gets or sets the maximum number of concurrent connections to accept. Set `-1` for infinite. /// Set to `null` to use the registry's machine-wide setting. diff --git a/src/Servers/HttpSys/src/PublicAPI.Unshipped.txt b/src/Servers/HttpSys/src/PublicAPI.Unshipped.txt index 4ce3298c1f62..ffe1c5366991 100644 --- a/src/Servers/HttpSys/src/PublicAPI.Unshipped.txt +++ b/src/Servers/HttpSys/src/PublicAPI.Unshipped.txt @@ -1,4 +1,6 @@ #nullable enable +Microsoft.AspNetCore.Server.HttpSys.HttpSysOptions.EnableKernelResponseBuffering.get -> bool +Microsoft.AspNetCore.Server.HttpSys.HttpSysOptions.EnableKernelResponseBuffering.set -> void Microsoft.AspNetCore.Server.HttpSys.HttpSysRequestTimingType Microsoft.AspNetCore.Server.HttpSys.HttpSysRequestTimingType.ConnectionStart = 0 -> Microsoft.AspNetCore.Server.HttpSys.HttpSysRequestTimingType Microsoft.AspNetCore.Server.HttpSys.HttpSysRequestTimingType.DataStart = 1 -> Microsoft.AspNetCore.Server.HttpSys.HttpSysRequestTimingType diff --git a/src/Servers/HttpSys/src/RequestProcessing/Response.cs b/src/Servers/HttpSys/src/RequestProcessing/Response.cs index 1b7e9745b0e0..b10de647511d 100644 --- a/src/Servers/HttpSys/src/RequestProcessing/Response.cs +++ b/src/Servers/HttpSys/src/RequestProcessing/Response.cs @@ -275,8 +275,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(ref UnmanagedBufferAllocator allocator, Span dataChunks, ResponseStreamAsyncResult? asyncResult, diff --git a/src/Servers/HttpSys/src/RequestProcessing/ResponseBody.cs b/src/Servers/HttpSys/src/RequestProcessing/ResponseBody.cs index cdb2205f1ac5..b848318e6b5d 100644 --- a/src/Servers/HttpSys/src/RequestProcessing/ResponseBody.cs +++ b/src/Servers/HttpSys/src/RequestProcessing/ResponseBody.cs @@ -39,6 +39,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 @@ -496,6 +498,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 5fe09a3865c8..43fe220588d1 100644 --- a/src/Servers/HttpSys/test/FunctionalTests/ResponseBodyTests.cs +++ b/src/Servers/HttpSys/test/FunctionalTests/ResponseBodyTests.cs @@ -1,18 +1,11 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System; -using System.Collections.Generic; -using System.IO; -using System.Linq; using System.Net.Http; using System.Text; -using System.Threading; -using System.Threading.Tasks; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Testing; -using Xunit; namespace Microsoft.AspNetCore.Server.HttpSys; @@ -133,6 +126,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 cf21829226d575e097d188c16f4cba78f394d4aa Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Wed, 19 Apr 2023 16:33:16 +0100 Subject: [PATCH 2/3] remove obsolete comment fragment --- 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 c6667e622f96..be8d34714f1f 100644 --- a/src/Servers/HttpSys/src/HttpSysOptions.cs +++ b/src/Servers/HttpSys/src/HttpSysOptions.cs @@ -121,7 +121,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. /// - public bool EnableKernelResponseBuffering { get; set; } // internal via app-context for non-public release + public bool EnableKernelResponseBuffering { get; set; } /// /// Gets or sets the maximum number of concurrent connections to accept. Set `-1` for infinite. From 3fca63b4897f5b606722de9d2ae33f19026791e5 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Thu, 20 Apr 2023 15:05:36 +0100 Subject: [PATCH 3/3] address PR feedback --- src/Servers/HttpSys/src/HttpSysOptions.cs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/Servers/HttpSys/src/HttpSysOptions.cs b/src/Servers/HttpSys/src/HttpSysOptions.cs index be8d34714f1f..44bc0bc5faa8 100644 --- a/src/Servers/HttpSys/src/HttpSysOptions.cs +++ b/src/Servers/HttpSys/src/HttpSysOptions.cs @@ -34,10 +34,6 @@ public class HttpSysOptions /// public HttpSysOptions() { - // this feature is being back-ported to net6/net7 via an app-context switch; respect that even on net8+ to avoid - // surprises when upgrading TFMs - const string EnableKernelResponseBufferingSwitch = "Microsoft.AspNetCore.Server.HttpSys.EnableKernelResponseBuffering"; - EnableKernelResponseBuffering = AppContext.TryGetSwitch(EnableKernelResponseBufferingSwitch, out var enabled) && enabled; } /// @@ -115,7 +111,7 @@ public string? RequestQueueName public bool ThrowWriteExceptions { get; set; } /// - /// Enable buffering of response data in the Kernel. + /// Enable buffering of response data in the Kernel. The default value is false. /// 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.