Don't allocate for ResponseCookiesFeature

This commit is contained in:
Ben Adams 2016-10-07 04:22:33 +01:00 committed by Pavel Krymets
parent 063d6eca0f
commit 4569653504
7 changed files with 107 additions and 93 deletions

View File

@ -12,7 +12,6 @@ namespace SampleApp
{ {
public class PooledHttpContextFactory : IHttpContextFactory public class PooledHttpContextFactory : IHttpContextFactory
{ {
private readonly ObjectPool<StringBuilder> _builderPool;
private readonly IHttpContextAccessor _httpContextAccessor; private readonly IHttpContextAccessor _httpContextAccessor;
private readonly Stack<PooledHttpContext> _pool = new Stack<PooledHttpContext>(); private readonly Stack<PooledHttpContext> _pool = new Stack<PooledHttpContext>();
@ -28,7 +27,6 @@ namespace SampleApp
throw new ArgumentNullException(nameof(poolProvider)); throw new ArgumentNullException(nameof(poolProvider));
} }
_builderPool = poolProvider.CreateStringBuilderPool();
_httpContextAccessor = httpContextAccessor; _httpContextAccessor = httpContextAccessor;
} }
@ -39,9 +37,6 @@ namespace SampleApp
throw new ArgumentNullException(nameof(featureCollection)); throw new ArgumentNullException(nameof(featureCollection));
} }
var responseCookiesFeature = new ResponseCookiesFeature(featureCollection, _builderPool);
featureCollection.Set<IResponseCookiesFeature>(responseCookiesFeature);
PooledHttpContext httpContext = null; PooledHttpContext httpContext = null;
lock (_pool) lock (_pool)
{ {

View File

@ -29,7 +29,8 @@
"type": "build" "type": "build"
}, },
"NETStandard.Library": "1.6.1-*", "NETStandard.Library": "1.6.1-*",
"System.Text.Encodings.Web": "4.3.0-*" "System.Text.Encodings.Web": "4.3.0-*",
"Microsoft.Extensions.Primitives": "1.1.0-*"
}, },
"frameworks": { "frameworks": {
"net451": { "net451": {

View File

@ -16,9 +16,6 @@ namespace Microsoft.AspNetCore.Http.Features
// Lambda hoisted to static readonly field to improve inlining https://github.com/dotnet/roslyn/issues/13624 // Lambda hoisted to static readonly field to improve inlining https://github.com/dotnet/roslyn/issues/13624
private readonly static Func<IFeatureCollection, IHttpResponseFeature> _nullResponseFeature = f => null; private readonly static Func<IFeatureCollection, IHttpResponseFeature> _nullResponseFeature = f => null;
// 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 FeatureReferences<IHttpResponseFeature> _features;
private IResponseCookies _cookiesCollection; private IResponseCookies _cookiesCollection;
@ -50,7 +47,6 @@ namespace Microsoft.AspNetCore.Http.Features
} }
_features = new FeatureReferences<IHttpResponseFeature>(features); _features = new FeatureReferences<IHttpResponseFeature>(features);
_builderPool = builderPool;
} }
private IHttpResponseFeature HttpResponseFeature => _features.Fetch(ref _features.Cache, _nullResponseFeature); private IHttpResponseFeature HttpResponseFeature => _features.Fetch(ref _features.Cache, _nullResponseFeature);
@ -63,7 +59,7 @@ namespace Microsoft.AspNetCore.Http.Features
if (_cookiesCollection == null) if (_cookiesCollection == null)
{ {
var headers = HttpResponseFeature.Headers; var headers = HttpResponseFeature.Headers;
_cookiesCollection = new ResponseCookies(headers, _builderPool); _cookiesCollection = new ResponseCookies(headers, null);
} }
return _cookiesCollection; return _cookiesCollection;

View File

@ -2,7 +2,6 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
using System; using System;
using System.Text;
using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Http.Features;
using Microsoft.Extensions.ObjectPool; using Microsoft.Extensions.ObjectPool;
using Microsoft.Extensions.Options; using Microsoft.Extensions.Options;
@ -11,7 +10,6 @@ namespace Microsoft.AspNetCore.Http
{ {
public class HttpContextFactory : IHttpContextFactory public class HttpContextFactory : IHttpContextFactory
{ {
private readonly ObjectPool<StringBuilder> _builderPool;
private readonly IHttpContextAccessor _httpContextAccessor; private readonly IHttpContextAccessor _httpContextAccessor;
private readonly FormOptions _formOptions; private readonly FormOptions _formOptions;
@ -31,7 +29,6 @@ namespace Microsoft.AspNetCore.Http
throw new ArgumentNullException(nameof(formOptions)); throw new ArgumentNullException(nameof(formOptions));
} }
_builderPool = poolProvider.CreateStringBuilderPool();
_formOptions = formOptions.Value; _formOptions = formOptions.Value;
_httpContextAccessor = httpContextAccessor; _httpContextAccessor = httpContextAccessor;
} }
@ -43,9 +40,6 @@ namespace Microsoft.AspNetCore.Http
throw new ArgumentNullException(nameof(featureCollection)); throw new ArgumentNullException(nameof(featureCollection));
} }
var responseCookiesFeature = new ResponseCookiesFeature(featureCollection, _builderPool);
featureCollection.Set<IResponseCookiesFeature>(responseCookiesFeature);
var httpContext = new DefaultHttpContext(featureCollection); var httpContext = new DefaultHttpContext(featureCollection);
if (_httpContextAccessor != null) if (_httpContextAccessor != null)
{ {

View File

@ -15,8 +15,6 @@ namespace Microsoft.AspNetCore.Http.Internal
/// </summary> /// </summary>
public class ResponseCookies : IResponseCookies public class ResponseCookies : IResponseCookies
{ {
private readonly ObjectPool<StringBuilder> _builderPool;
/// <summary> /// <summary>
/// Create a new wrapper. /// Create a new wrapper.
/// </summary> /// </summary>
@ -30,7 +28,6 @@ namespace Microsoft.AspNetCore.Http.Internal
} }
Headers = headers; Headers = headers;
_builderPool = builderPool;
} }
private IHeaderDictionary Headers { get; set; } private IHeaderDictionary Headers { get; set; }
@ -44,25 +41,7 @@ namespace Microsoft.AspNetCore.Http.Internal
{ {
Path = "/" Path = "/"
}; };
var cookieValue = 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); Headers[HeaderNames.SetCookie] = StringValues.Concat(Headers[HeaderNames.SetCookie], cookieValue);
} }
@ -86,24 +65,7 @@ namespace Microsoft.AspNetCore.Http.Internal
HttpOnly = options.HttpOnly, HttpOnly = options.HttpOnly,
}; };
string cookieValue; var cookieValue = setCookieHeaderValue.ToString();
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); Headers[HeaderNames.SetCookie] = StringValues.Concat(Headers[HeaderNames.SetCookie], cookieValue);
} }

View File

@ -5,6 +5,7 @@ using System;
using System.Collections.Generic; using System.Collections.Generic;
using System.Diagnostics.Contracts; using System.Diagnostics.Contracts;
using System.Text; using System.Text;
using Microsoft.Extensions.Primitives;
namespace Microsoft.Net.Http.Headers namespace Microsoft.Net.Http.Headers
{ {
@ -17,6 +18,8 @@ namespace Microsoft.Net.Http.Headers
private const string PathToken = "path"; private const string PathToken = "path";
private const string SecureToken = "secure"; private const string SecureToken = "secure";
private const string HttpOnlyToken = "httponly"; private const string HttpOnlyToken = "httponly";
private const string SeparatorToken = "; ";
private const string EqualsToken = "=";
private const string DefaultPath = "/"; // TODO: Used? private const string DefaultPath = "/"; // TODO: Used?
private static readonly HttpHeaderParser<SetCookieHeaderValue> SingleValueParser private static readonly HttpHeaderParser<SetCookieHeaderValue> SingleValueParser
@ -89,10 +92,91 @@ namespace Microsoft.Net.Http.Headers
// name="val ue"; expires=Sun, 06 Nov 1994 08:49:37 GMT; max-age=86400; domain=domain1; path=path1; secure; httponly // name="val ue"; expires=Sun, 06 Nov 1994 08:49:37 GMT; max-age=86400; domain=domain1; path=path1; secure; httponly
public override string ToString() public override string ToString()
{ {
StringBuilder header = new StringBuilder(); var length = _name.Length + EqualsToken.Length + _value.Length;
AppendToStringBuilder(header);
return header.ToString(); string expires = null;
string maxAge = null;
if (Expires.HasValue)
{
expires = HeaderUtilities.FormatDate(Expires.Value);
length += SeparatorToken.Length + ExpiresToken.Length + EqualsToken.Length + expires.Length;
}
if (MaxAge.HasValue)
{
maxAge = HeaderUtilities.FormatInt64((long)MaxAge.Value.TotalSeconds);
length += SeparatorToken.Length + MaxAgeToken.Length + EqualsToken.Length + maxAge.Length;
}
if (Domain != null)
{
length += SeparatorToken.Length + DomainToken.Length + EqualsToken.Length + Domain.Length;
}
if (Path != null)
{
length += SeparatorToken.Length + PathToken.Length + EqualsToken.Length + Path.Length;
}
if (Secure)
{
length += SeparatorToken.Length + SecureToken.Length;
}
if (HttpOnly)
{
length += SeparatorToken.Length + HttpOnlyToken.Length;
}
var sb = new InplaceStringBuilder(length);
sb.Append(_name);
sb.Append(EqualsToken);
sb.Append(_value);
if (expires != null)
{
AppendSegment(ref sb, ExpiresToken, expires);
}
if (maxAge != null)
{
AppendSegment(ref sb, MaxAgeToken, maxAge);
}
if (Domain != null)
{
AppendSegment(ref sb, DomainToken, Domain);
}
if (Path != null)
{
AppendSegment(ref sb, PathToken, Path);
}
if (Secure)
{
AppendSegment(ref sb, SecureToken, null);
}
if (HttpOnly)
{
AppendSegment(ref sb, HttpOnlyToken, null);
}
return sb.ToString();
}
private static void AppendSegment(ref InplaceStringBuilder builder, string name, string value)
{
builder.Append(SeparatorToken);
builder.Append(name);
if (value != null)
{
builder.Append(EqualsToken);
builder.Append(value);
}
} }
/// <summary> /// <summary>

View File

@ -11,27 +11,11 @@ namespace Microsoft.AspNetCore.Http.Tests
{ {
public class ResponseCookiesTest public class ResponseCookiesTest
{ {
private static readonly ObjectPool<StringBuilder> _builderPool = [Fact]
new DefaultObjectPoolProvider().Create<StringBuilder>(new StringBuilderPooledObjectPolicy()); public void DeleteCookieShouldSetDefaultPath()
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 headers = new HeaderDictionary();
var cookies = new ResponseCookies(headers, builderPool); var cookies = new ResponseCookies(headers, null);
var testcookie = "TestCookie"; var testcookie = "TestCookie";
cookies.Delete(testcookie); cookies.Delete(testcookie);
@ -43,12 +27,11 @@ namespace Microsoft.AspNetCore.Http.Tests
Assert.Contains("expires=Thu, 01 Jan 1970 00:00:00 GMT", cookieHeaderValues[0]); Assert.Contains("expires=Thu, 01 Jan 1970 00:00:00 GMT", cookieHeaderValues[0]);
} }
[Theory] [Fact]
[MemberData(nameof(BuilderPoolData))] public void NoParamsDeleteRemovesCookieCreatedByAdd()
public void NoParamsDeleteRemovesCookieCreatedByAdd(ObjectPool<StringBuilder> builderPool)
{ {
var headers = new HeaderDictionary(); var headers = new HeaderDictionary();
var cookies = new ResponseCookies(headers, builderPool); var cookies = new ResponseCookies(headers, null);
var testcookie = "TestCookie"; var testcookie = "TestCookie";
cookies.Append(testcookie, testcookie); cookies.Append(testcookie, testcookie);
@ -66,15 +49,15 @@ namespace Microsoft.AspNetCore.Http.Tests
get get
{ {
// key, value, object pool, expected // key, value, object pool, expected
return new TheoryData<string, string, ObjectPool<StringBuilder>, string> return new TheoryData<string, string, string>
{ {
{ "key", "value", null, "key=value" }, { "key", "value", "key=value" },
{ "key,", "!value", null, "key%2C=%21value" }, { "key,", "!value", "key%2C=%21value" },
{ "ke#y,", "val^ue", null, "ke%23y%2C=val%5Eue" }, { "ke#y,", "val^ue", "ke%23y%2C=val%5Eue" },
{ "key", "value", _builderPool, "key=value" }, { "key", "value", "key=value" },
{ "key,", "!value", _builderPool, "key%2C=%21value" }, { "key,", "!value", "key%2C=%21value" },
{ "ke#y,", "val^ue", _builderPool, "ke%23y%2C=val%5Eue" }, { "ke#y,", "val^ue", "ke%23y%2C=val%5Eue" },
{ "base64", "QUI+REU/Rw==", _builderPool, "base64=QUI%2BREU%2FRw%3D%3D" }, { "base64", "QUI+REU/Rw==", "base64=QUI%2BREU%2FRw%3D%3D" },
}; };
} }
} }
@ -84,11 +67,10 @@ namespace Microsoft.AspNetCore.Http.Tests
public void EscapesKeyValuesBeforeSettingCookie( public void EscapesKeyValuesBeforeSettingCookie(
string key, string key,
string value, string value,
ObjectPool<StringBuilder> builderPool,
string expected) string expected)
{ {
var headers = new HeaderDictionary(); var headers = new HeaderDictionary();
var cookies = new ResponseCookies(headers, builderPool); var cookies = new ResponseCookies(headers, null);
cookies.Append(key, value); cookies.Append(key, value);