Skip to content
This repository was archived by the owner on Nov 20, 2018. It is now read-only.

Use pooled StringBuilder to reduce allocations when adding response cookies #591

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions src/Microsoft.AspNetCore.Http.Features/IResponseCookiesFeature.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,33 @@

namespace Microsoft.AspNetCore.Http.Features
{
/// <summary>
/// Feature containing cookies which will be returned with the response.
/// </summary>
public interface IResponseCookiesFeature
{
/// <summary>
/// Initial capacity of instances used in the implementation of this feature, in <see cref="char"/>s.
/// </summary>
/// <remarks>
/// For example, the initial capacity of <see cref="System.Text.StringBuilder"/> instances obtained
/// from an object pool.
/// </remarks>
int InitialPooledInstanceCapacity { get; set; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👎 This is an implementation detail, not a generic feature property.


/// <summary>
/// Maximum retained capacity of instances used in the implementation of this feature, in <see cref="char"/>s.
/// Instances larger than this will be deleted rather than preserved in the pool.
/// </summary>
/// <remarks>
/// For example, the maximum retained capacity of <see cref="System.Text.StringBuilder"/> instances obtained
/// from an object pool.
/// </remarks>
int MaximumRetainedPooledInstanceCapacity { get; set; }

/// <summary>
/// Cookies which will be returned with the response.
/// </summary>
IResponseCookies Cookies { get; }
}
}
39 changes: 36 additions & 3 deletions src/Microsoft.AspNetCore.Http/Features/ResponseCookiesFeature.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,31 +2,64 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

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
{
private FeatureReferences<IHttpResponseFeature> _features;
private FeatureReferences<IServiceProvidersFeature> _services;
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)
{
_features = new FeatureReferences<IHttpResponseFeature>(features);
_services = new FeatureReferences<IServiceProvidersFeature>(features);
}

private IHttpResponseFeature HttpResponseFeature =>
_features.Fetch(ref _features.Cache, f => null);
/// <inheritdoc />
/// <value>Default is <c>100</c> <see cref="char"/>s.</value>
public int InitialPooledInstanceCapacity { get; set; } = new StringBuilderPooledObjectPolicy().InitialCapacity;

/// <inheritdoc />
/// <value>Default is <c>4048</c> <see cref="char"/>s.</value>
public int MaximumRetainedPooledInstanceCapacity { get; set; } =
new StringBuilderPooledObjectPolicy().MaximumRetainedCapacity;

private IHttpResponseFeature HttpResponseFeature => _features.Fetch(ref _features.Cache, f => null);

private IServiceProvidersFeature ServiceProvidersFeature => _services.Fetch(ref _services.Cache, f => null);

/// <inheritdoc />
public IResponseCookies Cookies
{
get
{
if (_cookiesCollection == null)
{
var headers = HttpResponseFeature.Headers;
_cookiesCollection = new ResponseCookies(headers);

var serviceProvider = ServiceProvidersFeature.RequestServices;
var provider = (ObjectPoolProvider)serviceProvider.GetService(typeof(ObjectPoolProvider)) ??
new DefaultObjectPoolProvider();
var pool = provider.CreateStringBuilderPool(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this actually create a new pool per request? Or is it a GetOrCreate? How about if you call DefaultObjectPoolProvider?

If so, how is this an improvement over creating a few StringBuilders per request?

InitialPooledInstanceCapacity,
MaximumRetainedPooledInstanceCapacity);

_cookiesCollection = new ResponseCookies(headers, pool);
}

return _cookiesCollection;
}
}
Expand Down
55 changes: 44 additions & 11 deletions src/Microsoft.AspNetCore.Http/ResponseCookies.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@
// 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;

Expand All @@ -14,18 +15,26 @@ namespace Microsoft.AspNetCore.Http.Internal
/// </summary>
public class ResponseCookies : IResponseCookies
{
private readonly ObjectPool<StringBuilder> _builderPool;

/// <summary>
/// Create a new wrapper
/// </summary>
/// <param name="headers"></param>
public ResponseCookies(IHeaderDictionary headers)
/// <param name="headers">The <see cref="IHeaderDictionary"/> for the response.</param>
/// <param name="builderPool">The <see cref="ObjectPool{T}"/> used.</param>
public ResponseCookies(IHeaderDictionary headers, ObjectPool<StringBuilder> builderPool)
{
if (headers == null)
{
throw new ArgumentNullException(nameof(headers));
}
if (builderPool == null)
{
throw new ArgumentNullException(nameof(builderPool));
}

Headers = headers;
_builderPool = builderPool;
}

private IHeaderDictionary Headers { get; set; }
Expand All @@ -38,13 +47,25 @@ public ResponseCookies(IHeaderDictionary headers)
public void Append(string key, string value)
{
var setCookieHeaderValue = new SetCookieHeaderValue(
Uri.EscapeDataString(key),
Uri.EscapeDataString(value))
Uri.EscapeDataString(key),
Uri.EscapeDataString(value))
{
Path = "/"
};

Headers[HeaderNames.SetCookie] = StringValues.Concat(Headers[HeaderNames.SetCookie], setCookieHeaderValue.ToString());
string cookieValue;
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>
Expand All @@ -61,8 +82,8 @@ public void Append(string key, string value, CookieOptions options)
}

var setCookieHeaderValue = new SetCookieHeaderValue(
Uri.EscapeDataString(key),
Uri.EscapeDataString(value))
Uri.EscapeDataString(key),
Uri.EscapeDataString(value))
{
Domain = options.Domain,
Path = options.Path,
Expand All @@ -71,7 +92,19 @@ public void Append(string key, string value, CookieOptions options)
HttpOnly = options.HttpOnly,
};

Headers[HeaderNames.SetCookie] = StringValues.Concat(Headers[HeaderNames.SetCookie], setCookieHeaderValue.ToString());
string cookieValue;
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>
Expand All @@ -94,7 +127,7 @@ public void Delete(string key, CookieOptions options)
{
throw new ArgumentNullException(nameof(options));
}

var encodedKeyPlusEquals = Uri.EscapeDataString(key) + "=";
bool domainHasValue = !string.IsNullOrEmpty(options.Domain);
bool pathHasValue = !string.IsNullOrEmpty(options.Path);
Expand Down Expand Up @@ -130,7 +163,7 @@ public void Delete(string key, CookieOptions options)
newValues.Add(values[i]);
}
}

Headers[HeaderNames.SetCookie] = new StringValues(newValues.ToArray());
}

Expand Down
1 change: 1 addition & 0 deletions src/Microsoft.AspNetCore.Http/project.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down
34 changes: 23 additions & 11 deletions src/Microsoft.Net.Http.Headers/SetCookieHeaderValue.cs
Original file line number Diff line number Diff line change
Expand Up @@ -90,42 +90,54 @@ public string Value
public override string ToString()
{
StringBuilder header = new StringBuilder();
AppendToStringBuilder(header);

header.Append(_name);
header.Append("=");
header.Append(_value);
return header.ToString();
}

/// <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)
Expand Down
15 changes: 10 additions & 5 deletions test/Microsoft.AspNetCore.Http.Tests/ResponseCookiesTest.cs
Original file line number Diff line number Diff line change
@@ -1,19 +1,24 @@
// 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 Microsoft.Net.Http.Headers;
using System.Text;
using Microsoft.AspNetCore.Http.Internal;
using Microsoft.Extensions.ObjectPool;
using Microsoft.Net.Http.Headers;
using Xunit;

namespace Microsoft.AspNetCore.Http.Tests
{
public class ResponseCookiesTest
{
private static readonly ObjectPool<StringBuilder> _builderPool =
new DefaultObjectPoolProvider().Create<StringBuilder>(new StringBuilderPooledObjectPolicy());

[Fact]
public void DeleteCookieShouldSetDefaultPath()
{
var headers = new HeaderDictionary();
var cookies = new ResponseCookies(headers);
var cookies = new ResponseCookies(headers, _builderPool);
var testcookie = "TestCookie";

cookies.Delete(testcookie);
Expand All @@ -29,7 +34,7 @@ public void DeleteCookieShouldSetDefaultPath()
public void NoParamsDeleteRemovesCookieCreatedByAdd()
{
var headers = new HeaderDictionary();
var cookies = new ResponseCookies(headers);
var cookies = new ResponseCookies(headers, _builderPool);
var testcookie = "TestCookie";

cookies.Append(testcookie, testcookie);
Expand All @@ -49,7 +54,7 @@ public void NoParamsDeleteRemovesCookieCreatedByAdd()
public void EscapesKeyValuesBeforeSettingCookie(string key, string value, string expected)
{
var headers = new HeaderDictionary();
var cookies = new ResponseCookies(headers);
var cookies = new ResponseCookies(headers, _builderPool);

cookies.Append(key, value);

Expand Down
12 changes: 12 additions & 0 deletions test/Microsoft.Net.Http.Headers.Tests/SetCookieHeaderValueTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using Xunit;

namespace Microsoft.Net.Http.Headers
Expand Down Expand Up @@ -264,6 +265,17 @@ public void SetCookieHeaderValue_ToString(SetCookieHeaderValue input, string exp
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)
Expand Down