From b8a1c04ffbeee6c60a74766142ff3e2e6779d701 Mon Sep 17 00:00:00 2001 From: David Fowler Date: Sun, 3 Dec 2017 13:27:36 -0800 Subject: [PATCH] Make the HttpParser a singleton (#2203) - It's completely stateless so make it a singleton - Fixed tests --- .../Http1ConnectionParsingOverheadBenchmark.cs | 2 +- .../Http1WritingBenchmark.cs | 2 +- .../HttpProtocolFeatureCollection.cs | 2 +- .../RequestParsingBenchmark.cs | 2 +- .../ResponseHeaderCollectionBenchmark.cs | 2 +- .../ResponseHeadersWritingBenchmark.cs | 2 +- .../Internal/Http/Http1Connection.cs | 2 +- src/Kestrel.Core/Internal/ServiceContext.cs | 2 +- src/Kestrel.Core/KestrelServer.cs | 2 +- test/Kestrel.Core.Tests/KestrelServerTests.cs | 17 ++++++++++++++--- .../ListenerPrimaryTests.cs | 4 ++-- test/shared/TestServiceContext.cs | 2 +- 12 files changed, 26 insertions(+), 15 deletions(-) diff --git a/benchmarks/Kestrel.Performance/Http1ConnectionParsingOverheadBenchmark.cs b/benchmarks/Kestrel.Performance/Http1ConnectionParsingOverheadBenchmark.cs index 28413ab08b..35f40de854 100644 --- a/benchmarks/Kestrel.Performance/Http1ConnectionParsingOverheadBenchmark.cs +++ b/benchmarks/Kestrel.Performance/Http1ConnectionParsingOverheadBenchmark.cs @@ -29,7 +29,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance var serviceContext = new ServiceContext { ServerOptions = new KestrelServerOptions(), - HttpParserFactory = f => NullParser.Instance + HttpParser = NullParser.Instance }; var http1Connection = new Http1Connection(application: null, context: new Http1ConnectionContext diff --git a/benchmarks/Kestrel.Performance/Http1WritingBenchmark.cs b/benchmarks/Kestrel.Performance/Http1WritingBenchmark.cs index 91f42667ae..ea5e13646f 100644 --- a/benchmarks/Kestrel.Performance/Http1WritingBenchmark.cs +++ b/benchmarks/Kestrel.Performance/Http1WritingBenchmark.cs @@ -104,7 +104,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance DateHeaderValueManager = new DateHeaderValueManager(), ServerOptions = new KestrelServerOptions(), Log = new MockTrace(), - HttpParserFactory = f => new HttpParser() + HttpParser = new HttpParser() }; var http1Connection = new TestHttp1Connection( diff --git a/benchmarks/Kestrel.Performance/HttpProtocolFeatureCollection.cs b/benchmarks/Kestrel.Performance/HttpProtocolFeatureCollection.cs index d55644d976..1df078f0e5 100644 --- a/benchmarks/Kestrel.Performance/HttpProtocolFeatureCollection.cs +++ b/benchmarks/Kestrel.Performance/HttpProtocolFeatureCollection.cs @@ -86,7 +86,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance DateHeaderValueManager = new DateHeaderValueManager(), ServerOptions = new KestrelServerOptions(), Log = new MockTrace(), - HttpParserFactory = f => new HttpParser() + HttpParser = new HttpParser() }; var http1Connection = new Http1Connection(application: null, context: new Http1ConnectionContext diff --git a/benchmarks/Kestrel.Performance/RequestParsingBenchmark.cs b/benchmarks/Kestrel.Performance/RequestParsingBenchmark.cs index 5837b5f5a1..d8c002c543 100644 --- a/benchmarks/Kestrel.Performance/RequestParsingBenchmark.cs +++ b/benchmarks/Kestrel.Performance/RequestParsingBenchmark.cs @@ -30,7 +30,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance DateHeaderValueManager = new DateHeaderValueManager(), ServerOptions = new KestrelServerOptions(), Log = new MockTrace(), - HttpParserFactory = f => new HttpParser() + HttpParser = new HttpParser() }; var http1Connection = new Http1Connection(application: null, context: new Http1ConnectionContext diff --git a/benchmarks/Kestrel.Performance/ResponseHeaderCollectionBenchmark.cs b/benchmarks/Kestrel.Performance/ResponseHeaderCollectionBenchmark.cs index cbd2fd5e79..6f9ac3b78a 100644 --- a/benchmarks/Kestrel.Performance/ResponseHeaderCollectionBenchmark.cs +++ b/benchmarks/Kestrel.Performance/ResponseHeaderCollectionBenchmark.cs @@ -179,7 +179,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance DateHeaderValueManager = new DateHeaderValueManager(), ServerOptions = new KestrelServerOptions(), Log = new MockTrace(), - HttpParserFactory = f => new HttpParser() + HttpParser = new HttpParser() }; var http1Connection = new Http1Connection(application: null, context: new Http1ConnectionContext diff --git a/benchmarks/Kestrel.Performance/ResponseHeadersWritingBenchmark.cs b/benchmarks/Kestrel.Performance/ResponseHeadersWritingBenchmark.cs index 4940aa4326..c11e5ceff5 100644 --- a/benchmarks/Kestrel.Performance/ResponseHeadersWritingBenchmark.cs +++ b/benchmarks/Kestrel.Performance/ResponseHeadersWritingBenchmark.cs @@ -119,7 +119,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Performance DateHeaderValueManager = new DateHeaderValueManager(), ServerOptions = new KestrelServerOptions(), Log = new MockTrace(), - HttpParserFactory = f => new HttpParser() + HttpParser = new HttpParser() }; var http1Connection = new TestHttp1Connection( diff --git a/src/Kestrel.Core/Internal/Http/Http1Connection.cs b/src/Kestrel.Core/Internal/Http/Http1Connection.cs index cb073a0724..a9e74a4220 100644 --- a/src/Kestrel.Core/Internal/Http/Http1Connection.cs +++ b/src/Kestrel.Core/Internal/Http/Http1Connection.cs @@ -35,7 +35,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http : base(context) { _context = context; - _parser = ServiceContext.HttpParserFactory(new Http1ParsingHandler(this)); + _parser = ServiceContext.HttpParser; _keepAliveTicks = ServerOptions.Limits.KeepAliveTimeout.Ticks; _requestHeadersTimeoutTicks = ServerOptions.Limits.RequestHeadersTimeout.Ticks; diff --git a/src/Kestrel.Core/Internal/ServiceContext.cs b/src/Kestrel.Core/Internal/ServiceContext.cs index 19d22a1272..30f45b1f37 100644 --- a/src/Kestrel.Core/Internal/ServiceContext.cs +++ b/src/Kestrel.Core/Internal/ServiceContext.cs @@ -13,7 +13,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal public IThreadPool ThreadPool { get; set; } - public Func> HttpParserFactory { get; set; } + public IHttpParser HttpParser { get; set; } public ISystemClock SystemClock { get; set; } diff --git a/src/Kestrel.Core/KestrelServer.cs b/src/Kestrel.Core/KestrelServer.cs index 81536a44ab..1905ab7bc6 100644 --- a/src/Kestrel.Core/KestrelServer.cs +++ b/src/Kestrel.Core/KestrelServer.cs @@ -101,7 +101,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core return new ServiceContext { Log = trace, - HttpParserFactory = handler => new HttpParser(handler.Connection.ServiceContext.Log.IsEnabled(LogLevel.Information)), + HttpParser = new HttpParser(trace.IsEnabled(LogLevel.Information)), ThreadPool = threadPool, SystemClock = systemClock, DateHeaderValueManager = dateHeaderValueManager, diff --git a/test/Kestrel.Core.Tests/KestrelServerTests.cs b/test/Kestrel.Core.Tests/KestrelServerTests.cs index 51e9de0221..da23da4f19 100644 --- a/test/Kestrel.Core.Tests/KestrelServerTests.cs +++ b/test/Kestrel.Core.Tests/KestrelServerTests.cs @@ -188,6 +188,8 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests public void LoggerCategoryNameIsKestrelServerNamespace() { var mockLoggerFactory = new Mock(); + var mockLogger = new Mock(); + mockLoggerFactory.Setup(m => m.CreateLogger(It.IsAny())).Returns(mockLogger.Object); new KestrelServer(Options.Create(null), Mock.Of(), mockLoggerFactory.Object); mockLoggerFactory.Verify(factory => factory.CreateLogger("Microsoft.AspNetCore.Server.Kestrel")); } @@ -195,8 +197,11 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests [Fact] public void StartWithNoTransportFactoryThrows() { + var mockLoggerFactory = new Mock(); + var mockLogger = new Mock(); + mockLoggerFactory.Setup(m => m.CreateLogger(It.IsAny())).Returns(mockLogger.Object); var exception = Assert.Throws(() => - new KestrelServer(Options.Create(null), null, Mock.Of())); + new KestrelServer(Options.Create(null), null, mockLoggerFactory.Object)); Assert.Equal("transportFactory", exception.ParamName); } @@ -231,7 +236,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests .Setup(transportFactory => transportFactory.Create(It.IsAny(), It.IsAny())) .Returns(mockTransport.Object); - var server = new KestrelServer(Options.Create(options), mockTransportFactory.Object, Mock.Of()); + var mockLoggerFactory = new Mock(); + var mockLogger = new Mock(); + mockLoggerFactory.Setup(m => m.CreateLogger(It.IsAny())).Returns(mockLogger.Object); + var server = new KestrelServer(Options.Create(options), mockTransportFactory.Object, mockLoggerFactory.Object); await server.StartAsync(new DummyApplication(), CancellationToken.None); var stopTask1 = server.StopAsync(default(CancellationToken)); @@ -285,7 +293,10 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests .Setup(transportFactory => transportFactory.Create(It.IsAny(), It.IsAny())) .Returns(mockTransport.Object); - var server = new KestrelServer(Options.Create(options), mockTransportFactory.Object, Mock.Of()); + var mockLoggerFactory = new Mock(); + var mockLogger = new Mock(); + mockLoggerFactory.Setup(m => m.CreateLogger(It.IsAny())).Returns(mockLogger.Object); + var server = new KestrelServer(Options.Create(options), mockTransportFactory.Object, mockLoggerFactory.Object); await server.StartAsync(new DummyApplication(), CancellationToken.None); var stopTask1 = server.StopAsync(default(CancellationToken)); diff --git a/test/Kestrel.Transport.Libuv.Tests/ListenerPrimaryTests.cs b/test/Kestrel.Transport.Libuv.Tests/ListenerPrimaryTests.cs index 41ee4a7f19..3defb90297 100644 --- a/test/Kestrel.Transport.Libuv.Tests/ListenerPrimaryTests.cs +++ b/test/Kestrel.Transport.Libuv.Tests/ListenerPrimaryTests.cs @@ -110,7 +110,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Tests DateHeaderValueManager = serviceContextPrimary.DateHeaderValueManager, ServerOptions = serviceContextPrimary.ServerOptions, ThreadPool = serviceContextPrimary.ThreadPool, - HttpParserFactory = serviceContextPrimary.HttpParserFactory, + HttpParser = serviceContextPrimary.HttpParser, }; var builderSecondary = new ConnectionBuilder(); builderSecondary.UseHttpServer(serviceContextSecondary, new DummyApplication(c => c.Response.WriteAsync("Secondary")), HttpProtocols.Http1); @@ -221,7 +221,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Transport.Libuv.Tests DateHeaderValueManager = serviceContextPrimary.DateHeaderValueManager, ServerOptions = serviceContextPrimary.ServerOptions, ThreadPool = serviceContextPrimary.ThreadPool, - HttpParserFactory = serviceContextPrimary.HttpParserFactory, + HttpParser = serviceContextPrimary.HttpParser, }; var builderSecondary = new ConnectionBuilder(); builderSecondary.UseHttpServer(serviceContextSecondary, new DummyApplication(c => c.Response.WriteAsync("Secondary")), HttpProtocols.Http1); diff --git a/test/shared/TestServiceContext.cs b/test/shared/TestServiceContext.cs index 556e36be58..1109cf5f05 100644 --- a/test/shared/TestServiceContext.cs +++ b/test/shared/TestServiceContext.cs @@ -39,7 +39,7 @@ namespace Microsoft.AspNetCore.Testing SystemClock = new MockSystemClock(); DateHeaderValueManager = new DateHeaderValueManager(SystemClock); ConnectionManager = new HttpConnectionManager(Log, ResourceCounter.Unlimited); - HttpParserFactory = handler => new HttpParser(handler.Connection.ServiceContext.Log.IsEnabled(LogLevel.Information)); + HttpParser = new HttpParser(Log.IsEnabled(LogLevel.Information)); ServerOptions = new KestrelServerOptions { AddServerHeader = false