Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a FeaturesCollection constructor that supports initial capacity #31381

Merged
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
18 changes: 17 additions & 1 deletion src/Http/Http.Features/src/FeatureCollection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ public class FeatureCollection : IFeatureCollection
{
private static readonly KeyComparer FeatureKeyComparer = new KeyComparer();
private readonly IFeatureCollection? _defaults;
private readonly int _initialCapacity;
private IDictionary<Type, object>? _features;
private volatile int _containerRevision;

Expand All @@ -25,6 +26,21 @@ public FeatureCollection()
{
}

/// <summary>
/// Initializes a new instance of <see cref="FeatureCollection"/> with the specified initial capacity.
/// </summary>
/// <param name="initialCapacity">The initial number of elements that the collection can contain.</param>
/// <exception cref="System.ArgumentOutOfRangeException"><paramref name="initialCapacity"/> is less than 0</exception>
public FeatureCollection(int initialCapacity)
{
if (initialCapacity < 0)
{
throw new ArgumentOutOfRangeException(nameof(initialCapacity));
}

_initialCapacity = initialCapacity;
}

/// <summary>
/// Initializes a new instance of <see cref="FeatureCollection"/> with the specified defaults.
/// </summary>
Expand Down Expand Up @@ -73,7 +89,7 @@ public object? this[Type key]

if (_features == null)
{
_features = new Dictionary<Type, object>();
_features = new Dictionary<Type, object>(_initialCapacity);
Copy link
Member

Choose a reason for hiding this comment

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

This means that in the default case we're now passing in 0, even though we know we're about to add at least one item. How well does Dictionary handle that?

Choose a reason for hiding this comment

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

Last I checked dictionary starts at 0 either way and only after the first add allocates space to the next nearest prime from a predefined list, which is 3, then 7 and so on.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

@NinoFloris that's right.

}
_features[key] = value;
_containerRevision++;
Expand Down
3 changes: 2 additions & 1 deletion src/Http/Http.Features/src/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,5 @@ Microsoft.AspNetCore.Http.Features.FeatureCollection.Set<TFeature>(TFeature? ins
Microsoft.AspNetCore.Http.Features.IFeatureCollection.Get<TFeature>() -> TFeature?
Microsoft.AspNetCore.Http.Features.IFeatureCollection.Set<TFeature>(TFeature? instance) -> void
Microsoft.AspNetCore.Http.Features.IServerVariablesFeature.this[string! variableName].get -> string?
Microsoft.AspNetCore.Http.ISession.TryGetValue(string! key, out byte[]? value) -> bool
Microsoft.AspNetCore.Http.ISession.TryGetValue(string! key, out byte[]? value) -> bool
Microsoft.AspNetCore.Http.Features.FeatureCollection.FeatureCollection(int initialCapacity) -> void
6 changes: 5 additions & 1 deletion src/Http/Http/src/DefaultHttpContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ namespace Microsoft.AspNetCore.Http
/// </summary>
public sealed class DefaultHttpContext : HttpContext
{
// The initial size of the feature collection when using the default constructor; based on number of common features
// https://github.com/dotnet/aspnetcore/issues/31249
private const int DefaultFeatureCollectionSize = 10;

// Lambdas hoisted to static readonly fields to improve inlining https://github.com/dotnet/roslyn/issues/13624
private readonly static Func<IFeatureCollection, IItemsFeature> _newItemsFeature = f => new ItemsFeature();
private readonly static Func<DefaultHttpContext, IServiceProvidersFeature> _newServiceProvidersFeature = context => new RequestServicesFeature(context, context.ServiceScopeFactory);
Expand All @@ -44,7 +48,7 @@ public sealed class DefaultHttpContext : HttpContext
/// Initializes a new instance of the <see cref="DefaultHttpContext"/> class.
/// </summary>
public DefaultHttpContext()
: this(new FeatureCollection())
: this(new FeatureCollection(DefaultFeatureCollectionSize))
{
Features.Set<IHttpRequestFeature>(new HttpRequestFeature());
Features.Set<IHttpResponseFeature>(new HttpResponseFeature());
Expand Down