From e568e4311ab441f2f97529aabbb41e67a48ae246 Mon Sep 17 00:00:00 2001 From: Doug Bunting Date: Tue, 15 Mar 2016 14:36:19 -0700 Subject: [PATCH 1/2] Add `ObjectPool` to default services - can now use pooled `StringBuilder`s to address aspnet/HttpAbstractions#561 and similar issues nit: Fix incomplete test noted when adding new one --- .../WebHostBuilder.cs | 22 ++++++++++++++-- src/Microsoft.AspNetCore.Hosting/project.json | 5 ++-- .../WebHostBuilderTests.cs | 26 ++++++++++++++++--- 3 files changed, 46 insertions(+), 7 deletions(-) diff --git a/src/Microsoft.AspNetCore.Hosting/WebHostBuilder.cs b/src/Microsoft.AspNetCore.Hosting/WebHostBuilder.cs index 90e89a96..4dc94553 100644 --- a/src/Microsoft.AspNetCore.Hosting/WebHostBuilder.cs +++ b/src/Microsoft.AspNetCore.Hosting/WebHostBuilder.cs @@ -6,6 +6,7 @@ using System.Diagnostics; using System.IO; using System.Reflection; +using System.Text; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Hosting.Builder; using Microsoft.AspNetCore.Hosting.Internal; @@ -17,6 +18,7 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.DependencyInjection.Extensions; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.ObjectPool; using Microsoft.Extensions.PlatformAbstractions; namespace Microsoft.AspNetCore.Hosting @@ -26,6 +28,11 @@ namespace Microsoft.AspNetCore.Hosting /// public class WebHostBuilder : IWebHostBuilder { + // StringBuilderPooledObjectPolicy.MaximumRetainedCapacity's default is too small for general use and will + // cause useful StringBuilders to be thrown away in common scenarios e.g. serializing an antiforgery cookie. + // Instead of that value (4 kB), allow the few StringBuilders to grow to 1 MB. + private const int MaximumBuilderSize = 0x100000; + private readonly IHostingEnvironment _hostingEnvironment; private readonly ILoggerFactory _loggerFactory; private readonly List> _configureServicesDelegates; @@ -135,7 +142,7 @@ public IWebHostBuilder Configure(Action configureApp) } /// - /// Configure the provided which will be available as a hosting service. + /// Configure the provided which will be available as a hosting service. /// /// The delegate that configures the . /// The . @@ -155,7 +162,7 @@ public IWebHost Build() var appEnvironment = hostingContainer.GetRequiredService(); var startupLoader = hostingContainer.GetRequiredService(); - + var contentRootPath = ResolveContentRootPath(_options.ContentRootPath, appEnvironment.ApplicationBasePath); var applicationName = ResolveApplicationName() ?? appEnvironment.ApplicationName; @@ -198,6 +205,17 @@ private IServiceCollection BuildHostingServices() services.AddSingleton(diagnosticSource); services.AddSingleton(diagnosticSource); + services.AddSingleton>(serviceProvider => + { + var provider = new DefaultObjectPoolProvider(); + var policy = new StringBuilderPooledObjectPolicy + { + MaximumRetainedCapacity = MaximumBuilderSize, + }; + + return provider.Create(policy); + }); + // Conjure up a RequestServices services.AddTransient(); diff --git a/src/Microsoft.AspNetCore.Hosting/project.json b/src/Microsoft.AspNetCore.Hosting/project.json index 264fc8d4..ef3469fa 100644 --- a/src/Microsoft.AspNetCore.Hosting/project.json +++ b/src/Microsoft.AspNetCore.Hosting/project.json @@ -18,14 +18,15 @@ "Microsoft.AspNetCore.Hosting.Server.Abstractions": "1.0.0-*", "Microsoft.AspNetCore.Http": "1.0.0-*", "Microsoft.AspNetCore.Http.Extensions": "1.0.0-*", - "Microsoft.Extensions.FileProviders.Physical": "1.0.0-*", - "Microsoft.Extensions.Options": "1.0.0-*", "Microsoft.Extensions.Configuration": "1.0.0-*", "Microsoft.Extensions.Configuration.CommandLine": "1.0.0-*", "Microsoft.Extensions.Configuration.EnvironmentVariables": "1.0.0-*", "Microsoft.Extensions.Configuration.Json": "1.0.0-*", "Microsoft.Extensions.DependencyInjection": "1.0.0-*", + "Microsoft.Extensions.FileProviders.Physical": "1.0.0-*", "Microsoft.Extensions.Logging": "1.0.0-*", + "Microsoft.Extensions.ObjectPool": "1.0.0-*", + "Microsoft.Extensions.Options": "1.0.0-*", "Microsoft.Extensions.PlatformAbstractions": "1.0.0-*", "Microsoft.Extensions.TypeNameHelper.Sources": { "version": "1.0.0-*", diff --git a/test/Microsoft.AspNetCore.Hosting.Tests/WebHostBuilderTests.cs b/test/Microsoft.AspNetCore.Hosting.Tests/WebHostBuilderTests.cs index c3ac1b67..3148040d 100644 --- a/test/Microsoft.AspNetCore.Hosting.Tests/WebHostBuilderTests.cs +++ b/test/Microsoft.AspNetCore.Hosting.Tests/WebHostBuilderTests.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.IO; +using System.Text; using System.Threading.Tasks; using Microsoft.AspNetCore.Hosting.Fakes; using Microsoft.AspNetCore.Hosting.Internal; @@ -13,8 +14,8 @@ using Microsoft.AspNetCore.Http.Internal; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.ObjectPool; using Microsoft.Extensions.PlatformAbstractions; -using System.Reflection; using Xunit; namespace Microsoft.AspNetCore.Hosting @@ -92,8 +93,27 @@ public async Task IApplicationLifetimeRegisteredEvenWhenStartupCtorThrows_Fallba using (host) { host.Start(); - var service = host.Services.GetServices(); - Assert.NotNull(service); + var services = host.Services.GetServices(); + Assert.NotNull(services); + Assert.NotEmpty(services); + + await AssertResponseContains(server.RequestDelegate, "Exception from constructor"); + } + } + + [Fact] + public async Task ObjectPoolOfStringBuilderRegistered_EvenWhenStartupCtorThrows() + { + var builder = CreateWebHostBuilder(); + var server = new TestServer(); + var host = builder.UseServer(server).UseStartup().Build(); + using (host) + { + host.Start(); + var services = host.Services.GetServices>(); + Assert.NotNull(services); + Assert.NotEmpty(services); + await AssertResponseContains(server.RequestDelegate, "Exception from constructor"); } } From 5defb7da108d7ccf3514f6b88a01276f2e222bb2 Mon Sep 17 00:00:00 2001 From: Doug Bunting Date: Tue, 15 Mar 2016 15:50:29 -0700 Subject: [PATCH 2/2] PR comments: simplify test --- .../WebHostBuilderTests.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/Microsoft.AspNetCore.Hosting.Tests/WebHostBuilderTests.cs b/test/Microsoft.AspNetCore.Hosting.Tests/WebHostBuilderTests.cs index 3148040d..65fd058e 100644 --- a/test/Microsoft.AspNetCore.Hosting.Tests/WebHostBuilderTests.cs +++ b/test/Microsoft.AspNetCore.Hosting.Tests/WebHostBuilderTests.cs @@ -102,19 +102,19 @@ public async Task IApplicationLifetimeRegisteredEvenWhenStartupCtorThrows_Fallba } [Fact] - public async Task ObjectPoolOfStringBuilderRegistered_EvenWhenStartupCtorThrows() + public void ObjectPoolOfStringBuilderRegistered() { - var builder = CreateWebHostBuilder(); var server = new TestServer(); - var host = builder.UseServer(server).UseStartup().Build(); + var host = CreateWebHostBuilder() + .UseServer(server) + .Configure(app => { }) + .Build(); using (host) { host.Start(); var services = host.Services.GetServices>(); Assert.NotNull(services); Assert.NotEmpty(services); - - await AssertResponseContains(server.RequestDelegate, "Exception from constructor"); } }