From d4a38d9f89999c642f312b8375d26067cbdbc7d7 Mon Sep 17 00:00:00 2001 From: Brennan Date: Thu, 6 Aug 2020 15:41:57 -0700 Subject: [PATCH 1/5] Add inline scheduler option for Sockets transport --- .../src/Client/SocketConnectionFactory.cs | 3 ++- .../src/Internal/SocketConnection.cs | 19 ++++++++++++++----- .../src/SocketConnectionListener.cs | 3 ++- .../src/SocketTransportOptions.cs | 2 ++ 4 files changed, 20 insertions(+), 7 deletions(-) diff --git a/src/Servers/Kestrel/Transport.Sockets/src/Client/SocketConnectionFactory.cs b/src/Servers/Kestrel/Transport.Sockets/src/Client/SocketConnectionFactory.cs index 6f95d242a591..122ad8808f7c 100644 --- a/src/Servers/Kestrel/Transport.Sockets/src/Client/SocketConnectionFactory.cs +++ b/src/Servers/Kestrel/Transport.Sockets/src/Client/SocketConnectionFactory.cs @@ -63,7 +63,8 @@ public async ValueTask ConnectAsync(EndPoint endpoint, Cancel _trace, _options.MaxReadBufferSize, _options.MaxWriteBufferSize, - _options.WaitForDataBeforeAllocatingBuffer); + _options.WaitForDataBeforeAllocatingBuffer, + _options.UnsafeInlineScheduling); socketConnection.Start(); return socketConnection; diff --git a/src/Servers/Kestrel/Transport.Sockets/src/Internal/SocketConnection.cs b/src/Servers/Kestrel/Transport.Sockets/src/Internal/SocketConnection.cs index d8f6146d37e5..73e5e6f1e4c8 100644 --- a/src/Servers/Kestrel/Transport.Sockets/src/Internal/SocketConnection.cs +++ b/src/Servers/Kestrel/Transport.Sockets/src/Internal/SocketConnection.cs @@ -37,11 +37,12 @@ internal sealed class SocketConnection : TransportConnection internal SocketConnection(Socket socket, MemoryPool memoryPool, - PipeScheduler scheduler, + PipeScheduler transportScheduler, ISocketsTrace trace, long? maxReadBufferSize = null, long? maxWriteBufferSize = null, - bool waitForData = true) + bool waitForData = true, + bool useInlineSchedulers = false) { Debug.Assert(socket != null); Debug.Assert(memoryPool != null); @@ -60,7 +61,15 @@ internal SocketConnection(Socket socket, // On *nix platforms, Sockets already dispatches to the ThreadPool. // Yes, the IOQueues are still used for the PipeSchedulers. This is intentional. // https://github.com/aspnet/KestrelHttpServer/issues/2573 - var awaiterScheduler = IsWindows ? scheduler : PipeScheduler.Inline; + var awaiterScheduler = IsWindows ? transportScheduler : PipeScheduler.Inline; + + var applicationScheduler = PipeScheduler.ThreadPool; + if (useInlineSchedulers) + { + transportScheduler = PipeScheduler.Inline; + awaiterScheduler = PipeScheduler.Inline; + applicationScheduler = PipeScheduler.Inline; + } _receiver = new SocketReceiver(_socket, awaiterScheduler); _sender = new SocketSender(_socket, awaiterScheduler); @@ -68,8 +77,8 @@ internal SocketConnection(Socket socket, maxReadBufferSize ??= 0; maxWriteBufferSize ??= 0; - var inputOptions = new PipeOptions(MemoryPool, PipeScheduler.ThreadPool, scheduler, maxReadBufferSize.Value, maxReadBufferSize.Value / 2, useSynchronizationContext: false); - var outputOptions = new PipeOptions(MemoryPool, scheduler, PipeScheduler.ThreadPool, maxWriteBufferSize.Value, maxWriteBufferSize.Value / 2, useSynchronizationContext: false); + var inputOptions = new PipeOptions(MemoryPool, applicationScheduler, transportScheduler, maxReadBufferSize.Value, maxReadBufferSize.Value / 2, useSynchronizationContext: false); + var outputOptions = new PipeOptions(MemoryPool, transportScheduler, applicationScheduler, maxWriteBufferSize.Value, maxWriteBufferSize.Value / 2, useSynchronizationContext: false); var pair = DuplexPipe.CreateConnectionPair(inputOptions, outputOptions); diff --git a/src/Servers/Kestrel/Transport.Sockets/src/SocketConnectionListener.cs b/src/Servers/Kestrel/Transport.Sockets/src/SocketConnectionListener.cs index a98ec0436315..8961dbee3bb3 100644 --- a/src/Servers/Kestrel/Transport.Sockets/src/SocketConnectionListener.cs +++ b/src/Servers/Kestrel/Transport.Sockets/src/SocketConnectionListener.cs @@ -124,7 +124,8 @@ public async ValueTask AcceptAsync(CancellationToken cancella } var connection = new SocketConnection(acceptSocket, _memoryPool, _schedulers[_schedulerIndex], _trace, - _options.MaxReadBufferSize, _options.MaxWriteBufferSize, _options.WaitForDataBeforeAllocatingBuffer); + _options.MaxReadBufferSize, _options.MaxWriteBufferSize, _options.WaitForDataBeforeAllocatingBuffer, + _options.UnsafeInlineScheduling); connection.Start(); diff --git a/src/Servers/Kestrel/Transport.Sockets/src/SocketTransportOptions.cs b/src/Servers/Kestrel/Transport.Sockets/src/SocketTransportOptions.cs index 957876ca5953..14427bb360fa 100644 --- a/src/Servers/Kestrel/Transport.Sockets/src/SocketTransportOptions.cs +++ b/src/Servers/Kestrel/Transport.Sockets/src/SocketTransportOptions.cs @@ -44,6 +44,8 @@ public class SocketTransportOptions public long? MaxWriteBufferSize { get; set; } = 64 * 1024; + public bool UnsafeInlineScheduling { get; set; } + internal Func> MemoryPoolFactory { get; set; } = System.Buffers.SlabMemoryPoolFactory.Create; } } From 7c71deca697f345e9f6ac53c9fbb02f66228bc00 Mon Sep 17 00:00:00 2001 From: BrennanConroy Date: Thu, 6 Aug 2020 17:04:24 -0700 Subject: [PATCH 2/5] summary --- .../Kestrel/Transport.Sockets/src/SocketTransportOptions.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/Servers/Kestrel/Transport.Sockets/src/SocketTransportOptions.cs b/src/Servers/Kestrel/Transport.Sockets/src/SocketTransportOptions.cs index 14427bb360fa..2fef7f97099d 100644 --- a/src/Servers/Kestrel/Transport.Sockets/src/SocketTransportOptions.cs +++ b/src/Servers/Kestrel/Transport.Sockets/src/SocketTransportOptions.cs @@ -44,6 +44,12 @@ public class SocketTransportOptions public long? MaxWriteBufferSize { get; set; } = 64 * 1024; + /// + /// Inline application and transport continuations instead of dispatching to the threadpool. + /// + /// + /// This will run application code on the IO thread which is why this is unsafe. + /// public bool UnsafeInlineScheduling { get; set; } internal Func> MemoryPoolFactory { get; set; } = System.Buffers.SlabMemoryPoolFactory.Create; From 6e9e472b7fe30f8a6cd52026be2bd7f68ebbfdf2 Mon Sep 17 00:00:00 2001 From: BrennanConroy Date: Thu, 13 Aug 2020 13:42:53 -0700 Subject: [PATCH 3/5] comment --- .../Kestrel/Transport.Sockets/src/SocketTransportOptions.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Servers/Kestrel/Transport.Sockets/src/SocketTransportOptions.cs b/src/Servers/Kestrel/Transport.Sockets/src/SocketTransportOptions.cs index 2fef7f97099d..7248da725c66 100644 --- a/src/Servers/Kestrel/Transport.Sockets/src/SocketTransportOptions.cs +++ b/src/Servers/Kestrel/Transport.Sockets/src/SocketTransportOptions.cs @@ -49,8 +49,12 @@ public class SocketTransportOptions /// /// /// This will run application code on the IO thread which is why this is unsafe. + /// We recommend setting the DOTNET_SYSTEM_NET_SOCKETS_INLINE_COMPLETIONS environment variable to '1' when using this setting to also inline the completions + /// at the runtime layer as well. + /// This setting can make make perf worse if you have expensive work that will end up holding onto the IO thread for longer than needed. As always, test your + /// scenarios to make sure this setting helps you. /// - public bool UnsafeInlineScheduling { get; set; } + public bool UnsafePreferInlineScheduling { get; set; } internal Func> MemoryPoolFactory { get; set; } = System.Buffers.SlabMemoryPoolFactory.Create; } From 1100c2b1ca1bccb09012240bbab977746844c4cb Mon Sep 17 00:00:00 2001 From: BrennanConroy Date: Thu, 13 Aug 2020 15:14:23 -0700 Subject: [PATCH 4/5] whoops --- .../Transport.Sockets/src/Client/SocketConnectionFactory.cs | 2 +- .../Kestrel/Transport.Sockets/src/SocketConnectionListener.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Servers/Kestrel/Transport.Sockets/src/Client/SocketConnectionFactory.cs b/src/Servers/Kestrel/Transport.Sockets/src/Client/SocketConnectionFactory.cs index 122ad8808f7c..a3275335a6e2 100644 --- a/src/Servers/Kestrel/Transport.Sockets/src/Client/SocketConnectionFactory.cs +++ b/src/Servers/Kestrel/Transport.Sockets/src/Client/SocketConnectionFactory.cs @@ -64,7 +64,7 @@ public async ValueTask ConnectAsync(EndPoint endpoint, Cancel _options.MaxReadBufferSize, _options.MaxWriteBufferSize, _options.WaitForDataBeforeAllocatingBuffer, - _options.UnsafeInlineScheduling); + _options.UnsafePreferInlineScheduling); socketConnection.Start(); return socketConnection; diff --git a/src/Servers/Kestrel/Transport.Sockets/src/SocketConnectionListener.cs b/src/Servers/Kestrel/Transport.Sockets/src/SocketConnectionListener.cs index 8961dbee3bb3..103a06c4d9e1 100644 --- a/src/Servers/Kestrel/Transport.Sockets/src/SocketConnectionListener.cs +++ b/src/Servers/Kestrel/Transport.Sockets/src/SocketConnectionListener.cs @@ -125,7 +125,7 @@ public async ValueTask AcceptAsync(CancellationToken cancella var connection = new SocketConnection(acceptSocket, _memoryPool, _schedulers[_schedulerIndex], _trace, _options.MaxReadBufferSize, _options.MaxWriteBufferSize, _options.WaitForDataBeforeAllocatingBuffer, - _options.UnsafeInlineScheduling); + _options.UnsafePreferInlineScheduling); connection.Start(); From 0b3599e08188fa210ff4805b7c53e98ce531bbe6 Mon Sep 17 00:00:00 2001 From: BrennanConroy Date: Mon, 17 Aug 2020 12:46:04 -0700 Subject: [PATCH 5/5] wording? --- .../Kestrel/Transport.Sockets/src/SocketTransportOptions.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Servers/Kestrel/Transport.Sockets/src/SocketTransportOptions.cs b/src/Servers/Kestrel/Transport.Sockets/src/SocketTransportOptions.cs index 7248da725c66..dc48442f6a2a 100644 --- a/src/Servers/Kestrel/Transport.Sockets/src/SocketTransportOptions.cs +++ b/src/Servers/Kestrel/Transport.Sockets/src/SocketTransportOptions.cs @@ -49,10 +49,10 @@ public class SocketTransportOptions /// /// /// This will run application code on the IO thread which is why this is unsafe. - /// We recommend setting the DOTNET_SYSTEM_NET_SOCKETS_INLINE_COMPLETIONS environment variable to '1' when using this setting to also inline the completions + /// It is recommended to set the DOTNET_SYSTEM_NET_SOCKETS_INLINE_COMPLETIONS environment variable to '1' when using this setting to also inline the completions /// at the runtime layer as well. - /// This setting can make make perf worse if you have expensive work that will end up holding onto the IO thread for longer than needed. As always, test your - /// scenarios to make sure this setting helps you. + /// This setting can make performance worse if there is expensive work that will end up holding onto the IO thread for longer than needed. + /// Test to make sure this setting helps performance. /// public bool UnsafePreferInlineScheduling { get; set; }