Use pooled `StringBuilder` to reduce allocations when adding response cookies

- #561
- new `SetCookieHeaderValue.AppendToStringBuilder()` method; avoids per-call `StringBuilder` allocation
- `ResponseCookies` uses `ObjectPool<StringBuilder>` that `ResponseCookiesFeature` provides
 - `ResponseCookies` works fine if no `ObjectPoolProvider` is available
- `IHttpContextFactory` instance is a singleton instantiated from CI
 - make `HttpContextFactory` `ObjectPoolProvider` and `ResponseCookiesFeature`-aware
 - apply same pattern to sample `PooledHttpContextFactory`
- pool is not currently configurable; defaults are fine for response cookies
 - if we need (policy) configuration, would add an `IOptions<HttpContextFactorySettings>`

nit: Add some doc comments

samples/SampleApp/PooledHttpContextFactory.cs

@@ -1,24 +1,48 @@
 // 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 System.Text;
 using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Http.Features;
+using Microsoft.AspNetCore.Http.Features.Internal;
+using Microsoft.Extensions.ObjectPool;
 
 namespace SampleApp
 {
     public class PooledHttpContextFactory : IHttpContextFactory
     {
+        private readonly ObjectPool<StringBuilder> _builderPool;
         private readonly IHttpContextAccessor _httpContextAccessor;
         private readonly Stack<PooledHttpContext> _pool = new Stack<PooledHttpContext>();
 
-        public PooledHttpContextFactory(IHttpContextAccessor httpContextAccessor)
+        public PooledHttpContextFactory(ObjectPoolProvider poolProvider)
+            : this(poolProvider, httpContextAccessor: null)
         {
+        }
+
+        public PooledHttpContextFactory(ObjectPoolProvider poolProvider, IHttpContextAccessor httpContextAccessor)
+        {
+            if (poolProvider == null)
+            {
+                throw new ArgumentNullException(nameof(poolProvider));
+            }
+
+            _builderPool = poolProvider.CreateStringBuilderPool();
             _httpContextAccessor = httpContextAccessor;
         }
 
         public HttpContext Create(IFeatureCollection featureCollection)
         {
+            if (featureCollection == null)
+            {
+                throw new ArgumentNullException(nameof(featureCollection));
+            }
+
+            var responseCookiesFeature = new ResponseCookiesFeature(featureCollection, _builderPool);
+            featureCollection.Set<IResponseCookiesFeature>(responseCookiesFeature);
+
             PooledHttpContext httpContext = null;
             lock (_pool)
             {

src/Microsoft.AspNetCore.Http.Features/IResponseCookies.cs

@@ -4,36 +4,39 @@
 namespace Microsoft.AspNetCore.Http
 {
     /// <summary>
-    /// A wrapper for the response Set-Cookie header
+    /// A wrapper for the response Set-Cookie header.
     /// </summary>
     public interface IResponseCookies
     {
         /// <summary>
-        /// Add a new cookie and value
+        /// Add a new cookie and value.
         /// </summary>
-        /// <param name="key"></param>
+        /// <param name="key">Name of the new cookie.</param>
-        /// <param name="value"></param>
+        /// <param name="value">Value of the new cookie.</param>
         void Append(string key, string value);
 
         /// <summary>
-        /// Add a new cookie
+        /// Add a new cookie.
         /// </summary>
-        /// <param name="key"></param>
+        /// <param name="key">Name of the new cookie.</param>
-        /// <param name="value"></param>
+        /// <param name="value">Value of the new cookie.</param>
-        /// <param name="options"></param>
+        /// <param name="options"><see cref="CookieOptions"/> included in the new cookie setting.</param>
         void Append(string key, string value, CookieOptions options);
 
         /// <summary>
-        /// Sets an expired cookie
+        /// Sets an expired cookie.
         /// </summary>
-        /// <param name="key"></param>
+        /// <param name="key">Name of the cookie to expire.</param>
         void Delete(string key);
 
         /// <summary>
-        /// Sets an expired cookie
+        /// Sets an expired cookie.
         /// </summary>
-        /// <param name="key"></param>
+        /// <param name="key">Name of the cookie to expire.</param>
-        /// <param name="options"></param>
+        /// <param name="options">
+        /// <see cref="CookieOptions"/> used to discriminate the particular cookie to expire. The
+        /// <see cref="CookieOptions.Domain"/> and <see cref="CookieOptions.Path"/> values are especially important.
+        /// </param>
         void Delete(string key, CookieOptions options);
     }
 }

src/Microsoft.AspNetCore.Http.Features/IResponseCookiesFeature.cs

@@ -3,8 +3,14 @@
 
 namespace Microsoft.AspNetCore.Http.Features
 {
+    /// <summary>
+    /// A helper for creating the response Set-Cookie header.
+    /// </summary>
     public interface IResponseCookiesFeature
     {
+        /// <summary>
+        /// Gets the wrapper for the response Set-Cookie header.
+        /// </summary>
         IResponseCookies Cookies { get; }
     }
 }

src/Microsoft.AspNetCore.Http/Features/ResponseCookiesFeature.cs

@@ -1,23 +1,58 @@
 // 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.Text;
 using Microsoft.AspNetCore.Http.Internal;
+using Microsoft.Extensions.ObjectPool;
 
 namespace Microsoft.AspNetCore.Http.Features.Internal
 {
+    /// <summary>
+    /// Default implementation of <see cref="IResponseCookiesFeature"/>.
+    /// </summary>
     public class ResponseCookiesFeature : IResponseCookiesFeature
     {
+        // Object pool will be null only in test scenarios e.g. if code news up a DefaultHttpContext.
+        private readonly ObjectPool<StringBuilder> _builderPool;
+
         private FeatureReferences<IHttpResponseFeature> _features;
         private IResponseCookies _cookiesCollection;
 
+        /// <summary>
+        /// Initializes a new <see cref="ResponseCookiesFeature"/> instance.
+        /// </summary>
+        /// <param name="features">
+        /// <see cref="IFeatureCollection"/> containing all defined features, including this
+        /// <see cref="IResponseCookiesFeature"/> and the <see cref="IHttpResponseFeature"/>.
+        /// </param>
         public ResponseCookiesFeature(IFeatureCollection features)
+            : this(features, builderPool: null)
         {
-            _features = new FeatureReferences<IHttpResponseFeature>(features);
         }
 
-        private IHttpResponseFeature HttpResponseFeature =>
+        /// <summary>
-            _features.Fetch(ref _features.Cache, f => null);
+        /// Initializes a new <see cref="ResponseCookiesFeature"/> instance.
+        /// </summary>
+        /// <param name="features">
+        /// <see cref="IFeatureCollection"/> containing all defined features, including this
+        /// <see cref="IResponseCookiesFeature"/> and the <see cref="IHttpResponseFeature"/>.
+        /// </param>
+        /// <param name="builderPool">The <see cref="ObjectPool{T}"/>, if available.</param>
+        public ResponseCookiesFeature(IFeatureCollection features, ObjectPool<StringBuilder> builderPool)
+        {
+            if (features == null)
+            {
+                throw new ArgumentNullException(nameof(features));
+            }
 
+            _features = new FeatureReferences<IHttpResponseFeature>(features);
+            _builderPool = builderPool;
+        }
+
+        private IHttpResponseFeature HttpResponseFeature => _features.Fetch(ref _features.Cache, f => null);
+
+        /// <inheritdoc />
         public IResponseCookies Cookies
         {
             get
@@ -25,8 +60,9 @@ namespace Microsoft.AspNetCore.Http.Features.Internal
                 if (_cookiesCollection == null)
                 {
                     var headers = HttpResponseFeature.Headers;
-                    _cookiesCollection = new ResponseCookies(headers);
+                    _cookiesCollection = new ResponseCookies(headers, _builderPool);
                 }
+
                 return _cookiesCollection;
             }
         }

src/Microsoft.AspNetCore.Http/HttpContextFactory.cs

@@ -1,30 +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.Text;
 using Microsoft.AspNetCore.Http.Features;
+using Microsoft.AspNetCore.Http.Features.Internal;
+using Microsoft.Extensions.ObjectPool;
 
 namespace Microsoft.AspNetCore.Http.Internal
 {
     public class HttpContextFactory : IHttpContextFactory
     {
-        private IHttpContextAccessor _httpContextAccessor;
+        private readonly ObjectPool<StringBuilder> _builderPool;
+        private readonly IHttpContextAccessor _httpContextAccessor;
 
-        public HttpContextFactory() : this(httpContextAccessor: null)
+        public HttpContextFactory(ObjectPoolProvider poolProvider)
+            : this(poolProvider, httpContextAccessor: null)
         {
         }
 
-        public HttpContextFactory(IHttpContextAccessor httpContextAccessor)
+        public HttpContextFactory(ObjectPoolProvider poolProvider, IHttpContextAccessor httpContextAccessor)
         {
+            if (poolProvider == null)
+            {
+                throw new ArgumentNullException(nameof(poolProvider));
+            }
+
+            _builderPool = poolProvider.CreateStringBuilderPool();
             _httpContextAccessor = httpContextAccessor;
         }
 
         public HttpContext Create(IFeatureCollection featureCollection)
         {
+            if (featureCollection == null)
+            {
+                throw new ArgumentNullException(nameof(featureCollection));
+            }
+
+            var responseCookiesFeature = new ResponseCookiesFeature(featureCollection, _builderPool);
+            featureCollection.Set<IResponseCookiesFeature>(responseCookiesFeature);
+
             var httpContext = new DefaultHttpContext(featureCollection);
             if (_httpContextAccessor != null)
             {
                 _httpContextAccessor.HttpContext = httpContext;
             }
+
             return httpContext;
         }
 

src/Microsoft.AspNetCore.Http/ResponseCookies.cs

@@ -2,23 +2,27 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System;
-using System.Text.Encodings.Web;
 using System.Collections.Generic;
+using System.Text;
+using Microsoft.Extensions.ObjectPool;
 using Microsoft.Extensions.Primitives;
 using Microsoft.Net.Http.Headers;
 
 namespace Microsoft.AspNetCore.Http.Internal
 {
     /// <summary>
-    /// A wrapper for the response Set-Cookie header
+    /// A wrapper for the response Set-Cookie header.
     /// </summary>
     public class ResponseCookies : IResponseCookies
     {
+        private readonly ObjectPool<StringBuilder> _builderPool;
+
         /// <summary>
-        /// Create a new wrapper
+        /// Create a new wrapper.
         /// </summary>
-        /// <param name="headers"></param>
+        /// <param name="headers">The <see cref="IHeaderDictionary"/> for the response.</param>
-        public ResponseCookies(IHeaderDictionary headers)
+        /// <param name="builderPool">The <see cref="ObjectPool{T}"/>, if available.</param>
+        public ResponseCookies(IHeaderDictionary headers, ObjectPool<StringBuilder> builderPool)
         {
             if (headers == null)
             {
@@ -26,33 +30,44 @@ namespace Microsoft.AspNetCore.Http.Internal
             }
 
             Headers = headers;
+            _builderPool = builderPool;
         }
 
         private IHeaderDictionary Headers { get; set; }
 
-        /// <summary>
+        /// <inheritdoc />
-        /// Add a new cookie and value
-        /// </summary>
-        /// <param name="key"></param>
-        /// <param name="value"></param>
         public void Append(string key, string value)
         {
             var setCookieHeaderValue = new SetCookieHeaderValue(
-                    Uri.EscapeDataString(key),
+                Uri.EscapeDataString(key),
-                    Uri.EscapeDataString(value))
+                Uri.EscapeDataString(value))
             {
                 Path = "/"
             };
 
-            Headers[HeaderNames.SetCookie] = StringValues.Concat(Headers[HeaderNames.SetCookie], setCookieHeaderValue.ToString());
+            string cookieValue;
+            if (_builderPool == null)
+            {
+                cookieValue = setCookieHeaderValue.ToString();
+            }
+            else
+            {
+                var stringBuilder = _builderPool.Get();
+                try
+                {
+                    setCookieHeaderValue.AppendToStringBuilder(stringBuilder);
+                    cookieValue = stringBuilder.ToString();
+                }
+                finally
+                {
+                    _builderPool.Return(stringBuilder);
+                }
+            }
+
+            Headers[HeaderNames.SetCookie] = StringValues.Concat(Headers[HeaderNames.SetCookie], cookieValue);
         }
 
-        /// <summary>
+        /// <inheritdoc />
-        /// Add a new cookie
-        /// </summary>
-        /// <param name="key"></param>
-        /// <param name="value"></param>
-        /// <param name="options"></param>
         public void Append(string key, string value, CookieOptions options)
         {
             if (options == null)
@@ -61,8 +76,8 @@ namespace Microsoft.AspNetCore.Http.Internal
             }
 
             var setCookieHeaderValue = new SetCookieHeaderValue(
-                    Uri.EscapeDataString(key),
+                Uri.EscapeDataString(key),
-                    Uri.EscapeDataString(value))
+                Uri.EscapeDataString(value))
             {
                 Domain = options.Domain,
                 Path = options.Path,
@@ -71,30 +86,42 @@ namespace Microsoft.AspNetCore.Http.Internal
                 HttpOnly = options.HttpOnly,
             };
 
-            Headers[HeaderNames.SetCookie] = StringValues.Concat(Headers[HeaderNames.SetCookie], setCookieHeaderValue.ToString());
+            string cookieValue;
+            if (_builderPool == null)
+            {
+                cookieValue = setCookieHeaderValue.ToString();
+            }
+            else
+            {
+                var stringBuilder = _builderPool.Get();
+                try
+                {
+                    setCookieHeaderValue.AppendToStringBuilder(stringBuilder);
+                    cookieValue = stringBuilder.ToString();
+                }
+                finally
+                {
+                    _builderPool.Return(stringBuilder);
+                }
+            }
+
+            Headers[HeaderNames.SetCookie] = StringValues.Concat(Headers[HeaderNames.SetCookie], cookieValue);
         }
 
-        /// <summary>
+        /// <inheritdoc />
-        /// Sets an expired cookie
-        /// </summary>
-        /// <param name="key"></param>
         public void Delete(string key)
         {
             Delete(key, new CookieOptions() { Path = "/" });
         }
 
-        /// <summary>
+        /// <inheritdoc />
-        /// Sets an expired cookie
-        /// </summary>
-        /// <param name="key"></param>
-        /// <param name="options"></param>
         public void Delete(string key, CookieOptions options)
         {
             if (options == null)
             {
                 throw new ArgumentNullException(nameof(options));
             }
-            
+
             var encodedKeyPlusEquals = Uri.EscapeDataString(key) + "=";
             bool domainHasValue = !string.IsNullOrEmpty(options.Domain);
             bool pathHasValue = !string.IsNullOrEmpty(options.Path);
@@ -130,7 +157,7 @@ namespace Microsoft.AspNetCore.Http.Internal
                         newValues.Add(values[i]);
                     }
                 }
-                
+
                 Headers[HeaderNames.SetCookie] = new StringValues(newValues.ToArray());
             }
 

src/Microsoft.AspNetCore.Http/project.json

@@ -17,6 +17,7 @@
   "dependencies": {
     "Microsoft.AspNetCore.Http.Abstractions": "1.0.0-*",
     "Microsoft.AspNetCore.WebUtilities": "1.0.0-*",
+    "Microsoft.Extensions.ObjectPool": "1.0.0-*",
     "Microsoft.Net.Http.Headers": "1.0.0-*"
   },
   "frameworks": {

src/Microsoft.Net.Http.Headers/SetCookieHeaderValue.cs

@@ -90,42 +90,54 @@ namespace Microsoft.Net.Http.Headers
         public override string ToString()
         {
             StringBuilder header = new StringBuilder();
+            AppendToStringBuilder(header);
 
-            header.Append(_name);
+            return header.ToString();
-            header.Append("=");
+        }
-            header.Append(_value);
+
+        /// <summary>
+        /// Append string representation of this <see cref="SetCookieHeaderValue"/> to given
+        /// <paramref name="builder"/>.
+        /// </summary>
+        /// <param name="builder">
+        /// The <see cref="StringBuilder"/> to receive the string representation of this
+        /// <see cref="SetCookieHeaderValue"/>.
+        /// </param>
+        public void AppendToStringBuilder(StringBuilder builder)
+        {
+            builder.Append(_name);
+            builder.Append("=");
+            builder.Append(_value);
 
             if (Expires.HasValue)
             {
-                AppendSegment(header, ExpiresToken, HeaderUtilities.FormatDate(Expires.Value));
+                AppendSegment(builder, ExpiresToken, HeaderUtilities.FormatDate(Expires.Value));
             }
 
             if (MaxAge.HasValue)
             {
-                AppendSegment(header, MaxAgeToken, HeaderUtilities.FormatInt64((long)MaxAge.Value.TotalSeconds));
+                AppendSegment(builder, MaxAgeToken, HeaderUtilities.FormatInt64((long)MaxAge.Value.TotalSeconds));
             }
 
             if (Domain != null)
             {
-                AppendSegment(header, DomainToken, Domain);
+                AppendSegment(builder, DomainToken, Domain);
             }
 
             if (Path != null)
             {
-                AppendSegment(header, PathToken, Path);
+                AppendSegment(builder, PathToken, Path);
             }
 
             if (Secure)
             {
-                AppendSegment(header, SecureToken, null);
+                AppendSegment(builder, SecureToken, null);
             }
 
             if (HttpOnly)
             {
-                AppendSegment(header, HttpOnlyToken, null);
+                AppendSegment(builder, HttpOnlyToken, null);
             }
-
-            return header.ToString();
         }
 
         private static void AppendSegment(StringBuilder builder, string name, string value)

test/Microsoft.AspNetCore.Http.Tests/HttpContextFactoryTests.cs

@@ -2,6 +2,7 @@
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using Microsoft.AspNetCore.Http.Features;
+using Microsoft.Extensions.ObjectPool;
 using Xunit;
 
 namespace Microsoft.AspNetCore.Http.Internal
@@ -13,7 +14,7 @@ namespace Microsoft.AspNetCore.Http.Internal
         {
             // Arrange
             var accessor = new HttpContextAccessor();
-            var contextFactory = new HttpContextFactory(accessor);
+            var contextFactory = new HttpContextFactory(new DefaultObjectPoolProvider(), accessor);
 
             // Act
             var context = contextFactory.Create(new FeatureCollection());
@@ -26,7 +27,7 @@ namespace Microsoft.AspNetCore.Http.Internal
         public void AllowsCreatingContextWithoutSettingAccessor()
         {
             // Arrange
-            var contextFactory = new HttpContextFactory();
+            var contextFactory = new HttpContextFactory(new DefaultObjectPoolProvider());
 
             // Act & Assert
             var context = contextFactory.Create(new FeatureCollection());

test/Microsoft.AspNetCore.Http.Tests/ResponseCookiesTest.cs

@@ -1,19 +1,37 @@
 // 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 Xunit;
+using System.Text;
-using Microsoft.Net.Http.Headers;
 using Microsoft.AspNetCore.Http.Internal;
+using Microsoft.Extensions.ObjectPool;
+using Microsoft.Net.Http.Headers;
+using Xunit;
 
 namespace Microsoft.AspNetCore.Http.Tests
 {
     public class ResponseCookiesTest
     {
-        [Fact]
+        private static readonly ObjectPool<StringBuilder> _builderPool =
-        public void DeleteCookieShouldSetDefaultPath()
+            new DefaultObjectPoolProvider().Create<StringBuilder>(new StringBuilderPooledObjectPolicy());
+
+        public static TheoryData BuilderPoolData
+        {
+            get
+            {
+                return new TheoryData<ObjectPool<StringBuilder>>
+                {
+                    null,
+                    _builderPool,
+                };
+            }
+        }
+
+        [Theory]
+        [MemberData(nameof(BuilderPoolData))]
+        public void DeleteCookieShouldSetDefaultPath(ObjectPool<StringBuilder> builderPool)
         {
             var headers = new HeaderDictionary();
-            var cookies = new ResponseCookies(headers);
+            var cookies = new ResponseCookies(headers, builderPool);
             var testcookie = "TestCookie";
 
             cookies.Delete(testcookie);
@@ -25,11 +43,12 @@ namespace Microsoft.AspNetCore.Http.Tests
             Assert.Contains("expires=Thu, 01 Jan 1970 00:00:00 GMT", cookieHeaderValues[0]);
         }
 
-        [Fact]
+        [Theory]
-        public void NoParamsDeleteRemovesCookieCreatedByAdd()
+        [MemberData(nameof(BuilderPoolData))]
+        public void NoParamsDeleteRemovesCookieCreatedByAdd(ObjectPool<StringBuilder> builderPool)
         {
             var headers = new HeaderDictionary();
-            var cookies = new ResponseCookies(headers);
+            var cookies = new ResponseCookies(headers, builderPool);
             var testcookie = "TestCookie";
 
             cookies.Append(testcookie, testcookie);
@@ -42,14 +61,33 @@ namespace Microsoft.AspNetCore.Http.Tests
             Assert.Contains("expires=Thu, 01 Jan 1970 00:00:00 GMT", cookieHeaderValues[0]);
         }
 
+        public static TheoryData EscapesKeyValuesBeforeSettingCookieData
+        {
+            get
+            {
+                // key, value, object pool, expected
+                return new TheoryData<string, string, ObjectPool<StringBuilder>, string>
+                {
+                    { "key", "value", null, "key=value" },
+                    { "key,", "!value", null, "key%2C=%21value" },
+                    { "ke#y,", "val^ue", null, "ke%23y%2C=val%5Eue" },
+                    { "key", "value", _builderPool, "key=value" },
+                    { "key,", "!value", _builderPool, "key%2C=%21value" },
+                    { "ke#y,", "val^ue", _builderPool, "ke%23y%2C=val%5Eue" },
+                };
+            }
+        }
+
         [Theory]
-        [InlineData("key", "value", "key=value")]
+        [MemberData(nameof(EscapesKeyValuesBeforeSettingCookieData))]
-        [InlineData("key,", "!value", "key%2C=%21value")]
+        public void EscapesKeyValuesBeforeSettingCookie(
-        [InlineData("ke#y,", "val^ue", "ke%23y%2C=val%5Eue")]
+            string key,
-        public void EscapesKeyValuesBeforeSettingCookie(string key, string value, string expected)
+            string value,
+            ObjectPool<StringBuilder> builderPool,
+            string expected)
         {
             var headers = new HeaderDictionary();
-            var cookies = new ResponseCookies(headers);
+            var cookies = new ResponseCookies(headers, builderPool);
 
             cookies.Append(key, value);
 

test/Microsoft.Net.Http.Headers.Tests/SetCookieHeaderValueTest.cs

@@ -4,6 +4,7 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
+using System.Text;
 using Xunit;
 
 namespace Microsoft.Net.Http.Headers
@@ -264,6 +265,17 @@ namespace Microsoft.Net.Http.Headers
             Assert.Equal(expectedValue, input.ToString());
         }
 
+        [Theory]
+        [MemberData(nameof(SetCookieHeaderDataSet))]
+        public void SetCookieHeaderValue_AppendToStringBuilder(SetCookieHeaderValue input, string expectedValue)
+        {
+            var builder = new StringBuilder();
+
+            input.AppendToStringBuilder(builder);
+
+            Assert.Equal(expectedValue, builder.ToString());
+        }
+
         [Theory]
         [MemberData(nameof(SetCookieHeaderDataSet))]
         public void SetCookieHeaderValue_Parse_AcceptsValidValues(SetCookieHeaderValue cookie, string expectedValue)