From 75cfe2c3bbcf736f28e93377548668867a6d6abe Mon Sep 17 00:00:00 2001 From: Kristian Hellang Date: Mon, 7 Dec 2015 14:45:35 +0100 Subject: [PATCH 1/7] Moved GetThreadCount into KestrelServerInformation --- .../KestrelServerInformation.cs | 28 +++++++++++++++++-- .../ServerFactory.cs | 27 +----------------- 2 files changed, 27 insertions(+), 28 deletions(-) diff --git a/src/Microsoft.AspNet.Server.Kestrel/KestrelServerInformation.cs b/src/Microsoft.AspNet.Server.Kestrel/KestrelServerInformation.cs index ec31f4fbf6..9619df7c1d 100644 --- a/src/Microsoft.AspNet.Server.Kestrel/KestrelServerInformation.cs +++ b/src/Microsoft.AspNet.Server.Kestrel/KestrelServerInformation.cs @@ -11,10 +11,10 @@ namespace Microsoft.AspNet.Server.Kestrel { public class KestrelServerInformation : IKestrelServerInformation, IServerAddressesFeature { - public KestrelServerInformation(IConfiguration configuration, int threadCount) + public KestrelServerInformation(IConfiguration configuration) { Addresses = GetAddresses(configuration); - ThreadCount = threadCount; + ThreadCount = GetThreadCount(configuration); NoDelay = true; } @@ -39,5 +39,29 @@ namespace Microsoft.AspNet.Server.Kestrel return addresses; } + + private static int GetThreadCount(IConfiguration configuration) + { + // Actual core count would be a better number + // rather than logical cores which includes hyper-threaded cores. + // Divide by 2 for hyper-threading, and good defaults (still need threads to do webserving). + // Can be user overriden using IKestrelServerInformation.ThreadCount + var threadCount = Environment.ProcessorCount >> 1; + + if (threadCount < 1) + { + // Ensure shifted value is at least one + return 1; + } + + if (threadCount > 16) + { + // Receive Side Scaling RSS Processor count currently maxes out at 16 + // would be better to check the NIC's current hardware queues; but xplat... + return 16; + } + + return threadCount; + } } } diff --git a/src/Microsoft.AspNet.Server.Kestrel/ServerFactory.cs b/src/Microsoft.AspNet.Server.Kestrel/ServerFactory.cs index 284823b849..d7f95ce25c 100644 --- a/src/Microsoft.AspNet.Server.Kestrel/ServerFactory.cs +++ b/src/Microsoft.AspNet.Server.Kestrel/ServerFactory.cs @@ -27,36 +27,11 @@ namespace Microsoft.AspNet.Server.Kestrel public IServer CreateServer(IConfiguration configuration) { - var threadCount = GetThreadCount(); - var information = new KestrelServerInformation(configuration, threadCount); + var information = new KestrelServerInformation(configuration); var serverFeatures = new FeatureCollection(); serverFeatures.Set(information); serverFeatures.Set(information); return new KestrelServer(serverFeatures, _appLifetime, _loggerFactory.CreateLogger("Microsoft.AspNet.Server.Kestrel")); } - - private static int GetThreadCount() - { - // Actual core count would be a better number - // rather than logical cores which includes hyper-threaded cores. - // Divide by 2 for hyper-threading, and good defaults (still need threads to do webserving). - // Can be user overriden using IKestrelServerInformation.ThreadCount - var threadCount = Environment.ProcessorCount >> 1; - - if (threadCount < 1) - { - // Ensure shifted value is at least one - return 1; - } - - if (threadCount > 16) - { - // Receive Side Scaling RSS Processor count currently maxes out at 16 - // would be better to check the NIC's current hardware queues; but xplat... - return 16; - } - - return threadCount; - } } } From b6b8ea3c384c61912dd54ea79b0c3b518842a936 Mon Sep 17 00:00:00 2001 From: Kristian Hellang Date: Mon, 7 Dec 2015 14:46:53 +0100 Subject: [PATCH 2/7] Made ThreadCount configurable --- .../KestrelServerInformation.cs | 33 +++++++++++-------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/src/Microsoft.AspNet.Server.Kestrel/KestrelServerInformation.cs b/src/Microsoft.AspNet.Server.Kestrel/KestrelServerInformation.cs index 9619df7c1d..84b2fcab57 100644 --- a/src/Microsoft.AspNet.Server.Kestrel/KestrelServerInformation.cs +++ b/src/Microsoft.AspNet.Server.Kestrel/KestrelServerInformation.cs @@ -42,23 +42,28 @@ namespace Microsoft.AspNet.Server.Kestrel private static int GetThreadCount(IConfiguration configuration) { - // Actual core count would be a better number - // rather than logical cores which includes hyper-threaded cores. - // Divide by 2 for hyper-threading, and good defaults (still need threads to do webserving). - // Can be user overriden using IKestrelServerInformation.ThreadCount - var threadCount = Environment.ProcessorCount >> 1; + var threadCountString = configuration["server.threadCount"]; - if (threadCount < 1) + int threadCount; + if (string.IsNullOrEmpty(threadCountString) || !int.TryParse(threadCountString, out threadCount)) { - // Ensure shifted value is at least one - return 1; - } + // Actual core count would be a better number + // rather than logical cores which includes hyper-threaded cores. + // Divide by 2 for hyper-threading, and good defaults (still need threads to do webserving). + threadCount = Environment.ProcessorCount >> 1; - if (threadCount > 16) - { - // Receive Side Scaling RSS Processor count currently maxes out at 16 - // would be better to check the NIC's current hardware queues; but xplat... - return 16; + if (threadCount < 1) + { + // Ensure shifted value is at least one + return 1; + } + + if (threadCount > 16) + { + // Receive Side Scaling RSS Processor count currently maxes out at 16 + // would be better to check the NIC's current hardware queues; but xplat... + return 16; + } } return threadCount; From 50f95cbbc0737fe3807223385d98283ce4eaaffa Mon Sep 17 00:00:00 2001 From: Kristian Hellang Date: Mon, 7 Dec 2015 15:17:21 +0100 Subject: [PATCH 3/7] Added some tests for ThreadCount --- .../KestrelServerInformationTests.cs | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 test/Microsoft.AspNet.Server.KestrelTests/KestrelServerInformationTests.cs diff --git a/test/Microsoft.AspNet.Server.KestrelTests/KestrelServerInformationTests.cs b/test/Microsoft.AspNet.Server.KestrelTests/KestrelServerInformationTests.cs new file mode 100644 index 0000000000..5a167adf27 --- /dev/null +++ b/test/Microsoft.AspNet.Server.KestrelTests/KestrelServerInformationTests.cs @@ -0,0 +1,51 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Collections.Generic; +using Microsoft.AspNet.Server.Kestrel; +using Microsoft.Extensions.Configuration; +using Xunit; + +namespace Microsoft.AspNet.Server.KestrelTests +{ + public class KestrelServerInformationTests + { + [Fact] + public void SetThreadCountUsingConfiguration() + { + const int expected = 42; + + var values = new Dictionary + { + { "server.threadCount", expected.ToString() } + }; + + var configuration = new ConfigurationBuilder() + .AddInMemoryCollection(values) + .Build(); + + var information = new KestrelServerInformation(configuration); + + Assert.Equal(expected, information.ThreadCount); + } + + [Fact] + public void SetThreadCountUsingProcessorCount() + { + // Ideally we'd mock Environment.ProcessorCount to test edge cases. + var expected = Clamp(Environment.ProcessorCount >> 1, 1, 16); + + var configuration = new ConfigurationBuilder().Build(); + + var information = new KestrelServerInformation(configuration); + + Assert.Equal(expected, information.ThreadCount); + } + + private static int Clamp(int value, int min, int max) + { + return value < min ? min : value > max ? max : value; + } + } +} \ No newline at end of file From bfad32f22321117e6044f3a666a935ab9adc60b2 Mon Sep 17 00:00:00 2001 From: Kristian Hellang Date: Mon, 7 Dec 2015 15:22:39 +0100 Subject: [PATCH 4/7] Added test for Addresses --- .../KestrelServerInformationTests.cs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/test/Microsoft.AspNet.Server.KestrelTests/KestrelServerInformationTests.cs b/test/Microsoft.AspNet.Server.KestrelTests/KestrelServerInformationTests.cs index 5a167adf27..2a3285da9f 100644 --- a/test/Microsoft.AspNet.Server.KestrelTests/KestrelServerInformationTests.cs +++ b/test/Microsoft.AspNet.Server.KestrelTests/KestrelServerInformationTests.cs @@ -43,6 +43,25 @@ namespace Microsoft.AspNet.Server.KestrelTests Assert.Equal(expected, information.ThreadCount); } + [Fact] + public void SetAddressesUsingConfiguration() + { + var expected = new List { "http://localhost:1337", "https://localhost:42" }; + + var values = new Dictionary + { + { "server.urls", string.Join(";", expected) } + }; + + var configuration = new ConfigurationBuilder() + .AddInMemoryCollection(values) + .Build(); + + var information = new KestrelServerInformation(configuration); + + Assert.Equal(expected, information.Addresses); + } + private static int Clamp(int value, int min, int max) { return value < min ? min : value > max ? max : value; From 12ee74c09c78ba8ae2987e8ea338e9e444201baa Mon Sep 17 00:00:00 2001 From: Kristian Hellang Date: Mon, 7 Dec 2015 22:00:03 +0100 Subject: [PATCH 5/7] server.threadCount -> kestre.threadCount --- src/Microsoft.AspNet.Server.Kestrel/KestrelServerInformation.cs | 2 +- .../KestrelServerInformationTests.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.AspNet.Server.Kestrel/KestrelServerInformation.cs b/src/Microsoft.AspNet.Server.Kestrel/KestrelServerInformation.cs index 84b2fcab57..acbfc685e5 100644 --- a/src/Microsoft.AspNet.Server.Kestrel/KestrelServerInformation.cs +++ b/src/Microsoft.AspNet.Server.Kestrel/KestrelServerInformation.cs @@ -42,7 +42,7 @@ namespace Microsoft.AspNet.Server.Kestrel private static int GetThreadCount(IConfiguration configuration) { - var threadCountString = configuration["server.threadCount"]; + var threadCountString = configuration["kestrel.threadCount"]; int threadCount; if (string.IsNullOrEmpty(threadCountString) || !int.TryParse(threadCountString, out threadCount)) diff --git a/test/Microsoft.AspNet.Server.KestrelTests/KestrelServerInformationTests.cs b/test/Microsoft.AspNet.Server.KestrelTests/KestrelServerInformationTests.cs index 2a3285da9f..91e9a419f0 100644 --- a/test/Microsoft.AspNet.Server.KestrelTests/KestrelServerInformationTests.cs +++ b/test/Microsoft.AspNet.Server.KestrelTests/KestrelServerInformationTests.cs @@ -18,7 +18,7 @@ namespace Microsoft.AspNet.Server.KestrelTests var values = new Dictionary { - { "server.threadCount", expected.ToString() } + { "kestrel.threadCount", expected.ToString() } }; var configuration = new ConfigurationBuilder() From 8d6a999bc3c3bee6043abea3b975c4987e54fc3d Mon Sep 17 00:00:00 2001 From: Kristian Hellang Date: Mon, 7 Dec 2015 22:09:04 +0100 Subject: [PATCH 6/7] Made NoDelay configurable --- .../KestrelServerInformation.cs | 20 ++++++++++++++++++- .../KestrelServerInformationTests.cs | 17 ++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.AspNet.Server.Kestrel/KestrelServerInformation.cs b/src/Microsoft.AspNet.Server.Kestrel/KestrelServerInformation.cs index acbfc685e5..222a0991f3 100644 --- a/src/Microsoft.AspNet.Server.Kestrel/KestrelServerInformation.cs +++ b/src/Microsoft.AspNet.Server.Kestrel/KestrelServerInformation.cs @@ -15,7 +15,7 @@ namespace Microsoft.AspNet.Server.Kestrel { Addresses = GetAddresses(configuration); ThreadCount = GetThreadCount(configuration); - NoDelay = true; + NoDelay = GetNoDelay(configuration); } public ICollection Addresses { get; } @@ -68,5 +68,23 @@ namespace Microsoft.AspNet.Server.Kestrel return threadCount; } + + private static bool GetNoDelay(IConfiguration configuration) + { + var noDelayString = configuration["kestrel.noDelay"]; + + if (string.IsNullOrEmpty(noDelayString)) + { + return true; + } + + bool noDelay; + if (bool.TryParse(noDelayString, out noDelay)) + { + return noDelay; + } + + return true; + } } } diff --git a/test/Microsoft.AspNet.Server.KestrelTests/KestrelServerInformationTests.cs b/test/Microsoft.AspNet.Server.KestrelTests/KestrelServerInformationTests.cs index 91e9a419f0..1c56258554 100644 --- a/test/Microsoft.AspNet.Server.KestrelTests/KestrelServerInformationTests.cs +++ b/test/Microsoft.AspNet.Server.KestrelTests/KestrelServerInformationTests.cs @@ -62,6 +62,23 @@ namespace Microsoft.AspNet.Server.KestrelTests Assert.Equal(expected, information.Addresses); } + [Fact] + public void SetNoDelayUsingConfiguration() + { + var values = new Dictionary + { + { "kestrel.noDelay", "false" } + }; + + var configuration = new ConfigurationBuilder() + .AddInMemoryCollection(values) + .Build(); + + var information = new KestrelServerInformation(configuration); + + Assert.False(information.NoDelay); + } + private static int Clamp(int value, int min, int max) { return value < min ? min : value > max ? max : value; From 4c68807a0549d650c231b72f282945abd9699ce2 Mon Sep 17 00:00:00 2001 From: Kristian Hellang Date: Mon, 7 Dec 2015 22:14:05 +0100 Subject: [PATCH 7/7] Split out ProcessorThreadCount, added InvariantCulture to TryParse --- .../KestrelServerInformation.cs | 56 ++++++++++++------- 1 file changed, 36 insertions(+), 20 deletions(-) diff --git a/src/Microsoft.AspNet.Server.Kestrel/KestrelServerInformation.cs b/src/Microsoft.AspNet.Server.Kestrel/KestrelServerInformation.cs index 222a0991f3..3808b6144b 100644 --- a/src/Microsoft.AspNet.Server.Kestrel/KestrelServerInformation.cs +++ b/src/Microsoft.AspNet.Server.Kestrel/KestrelServerInformation.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Globalization; using Microsoft.AspNet.Server.Features; using Microsoft.AspNet.Server.Kestrel.Filter; using Microsoft.Extensions.Configuration; @@ -26,6 +27,32 @@ namespace Microsoft.AspNet.Server.Kestrel public IConnectionFilter ConnectionFilter { get; set; } + private static int ProcessorThreadCount + { + get + { + // Actual core count would be a better number + // rather than logical cores which includes hyper-threaded cores. + // Divide by 2 for hyper-threading, and good defaults (still need threads to do webserving). + var threadCount = Environment.ProcessorCount >> 1; + + if (threadCount < 1) + { + // Ensure shifted value is at least one + return 1; + } + + if (threadCount > 16) + { + // Receive Side Scaling RSS Processor count currently maxes out at 16 + // would be better to check the NIC's current hardware queues; but xplat... + return 16; + } + + return threadCount; + } + } + private static ICollection GetAddresses(IConfiguration configuration) { var addresses = new List(); @@ -44,29 +71,18 @@ namespace Microsoft.AspNet.Server.Kestrel { var threadCountString = configuration["kestrel.threadCount"]; - int threadCount; - if (string.IsNullOrEmpty(threadCountString) || !int.TryParse(threadCountString, out threadCount)) + if (string.IsNullOrEmpty(threadCountString)) { - // Actual core count would be a better number - // rather than logical cores which includes hyper-threaded cores. - // Divide by 2 for hyper-threading, and good defaults (still need threads to do webserving). - threadCount = Environment.ProcessorCount >> 1; - - if (threadCount < 1) - { - // Ensure shifted value is at least one - return 1; - } - - if (threadCount > 16) - { - // Receive Side Scaling RSS Processor count currently maxes out at 16 - // would be better to check the NIC's current hardware queues; but xplat... - return 16; - } + return ProcessorThreadCount; } - return threadCount; + int threadCount; + if (int.TryParse(threadCountString, NumberStyles.Integer, CultureInfo.InvariantCulture, out threadCount)) + { + return threadCount; + } + + return ProcessorThreadCount; } private static bool GetNoDelay(IConfiguration configuration)