`ResponseCookies.Append()` allocates lots, big lots #561

dougbu · 2016-02-16T17:35:37Z

Despite using StringValuess, ResponseCookies.Append() is very allocation-heavy. Even just reusing the StringBuilder created in SetCookieHeaderValue.ToString() would be a significant improvement.

For example, running a scenario that adds an Antiforgery token to every response (~156 characters in the token in this case) allocates almost 1 KB extra per response. That's more than 2% of the allocations in this scenario.

Data for 3000 requests:

The text was updated successfully, but these errors were encountered:

muratg · 2016-03-14T19:13:23Z

@dougbu Are you able to send a PR to reuse the StringBuilder?

dougbu · 2016-03-14T19:15:01Z

Sure. I'll do that today.

dougbu · 2016-03-14T23:33:48Z

Offline discussion w/ @Tratcher: Reusing a StringBuilder per ResponseCookies instance won't do much because few cookies are set per request. Will instead add an object pool of a small class containing (at least for now) just a StringBuilder instance.

muratg · 2016-03-15T04:43:53Z

@dougbu That makes sense.

- can now use pooled `StringBuilder`s to address aspnet/HttpAbstractions#561 and similar issues nit: Fix incomplete test noted when adding new one

… cookies - #561

…tures` package and namespace - related to #561 - move required interfaces and classes down from `Microsoft.AspNetCore.Http.Abstractions` - move everything in the `Microsoft.AspNetCore.Http.Features` into their own directory nit: remove transient dependencies listed in `Microsoft.AspNetCore.Http.Abstractions`'s `project.json`

- see issue aspnet/HttpAbstractions#561 and pull aspnet/HttpAbstractions#589

…tures` package and namespace - related to #561 - move required interfaces and classes down from `Microsoft.AspNetCore.Http.Abstractions` - move everything in the `Microsoft.AspNetCore.Http.Features` into their own directory nit: remove transient dependencies listed in `Microsoft.AspNetCore.Http.Abstractions`'s `project.json`

- see issue aspnet/HttpAbstractions#561 and pull aspnet/HttpAbstractions#589

… cookies - #561 - new `SetCookieHeaderValue.AppendToStringBuilder()` method; avoids per-call `StringBuilder` allocation - `ResponseCookies` uses `ObjectPool<StringBuilder>` that `ResponseCookiesFeature` provides - `ResponseCookiesFeature` creates an `ObjectPoolProvider` instance if none in DI - `IResponseCookiesFeature` exposes `int` properties supporting middleware overrides of the pool policy nit: Add some doc comments

…tures` package and namespace - #590, also related to #561 - move feature interfaces from `Microsoft.AspNetCore.Http` package - move required classes from `Microsoft.AspNetCore.Http.Abstractions` package - move `ISession` and `WebSocketAcceptContext` to `Microsoft.AspNetCore.Http` namespace (#590) nit: remove transient dependencies listed in `Microsoft.AspNetCore.Http.Abstractions`'s `project.json`

- see issue aspnet/HttpAbstractions#561 and pull aspnet/HttpAbstractions#589

dougbu · 2016-03-21T16:31:29Z

For exposing the feature interfaces (no longer in .Internal namespaces):

… cookies - #561 - new `SetCookieHeaderValue.AppendToStringBuilder()` method; avoids per-call `StringBuilder` allocation - `ResponseCookies` uses `ObjectPool<StringBuilder>` that `ResponseCookiesFeature` provides - `ResponseCookiesFeature` creates an `ObjectPoolProvider` instance if none in DI - `IResponseCookiesFeature` exposes `int` properties supporting middleware overrides of the pool policy nit: Add some doc comments

- take advantage of aspnet/HttpAbstractions#561 fix

… cookies - #561 - new `SetCookieHeaderValue.AppendToStringBuilder()` method; avoids per-call `StringBuilder` allocation - `ResponseCookies` uses `ObjectPool<StringBuilder>` that `ResponseCookiesFeature` provides - `ResponseCookiesFeature` creates an `ObjectPoolProvider` instance if none in DI - `IResponseCookiesFeature` exposes `int` properties supporting middleware overrides of the pool policy nit: Add some doc comments

- e.g. take advantage of aspnet/HttpAbstractions#561 fix wherever cookies are used

- see pull aspnet/HttpAbstractions#594 and issue aspnet/HttpAbstractions#561

… 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

- e.g. take advantage of aspnet/HttpAbstractions#561 fix wherever cookies are used

- remove unused field - see pull aspnet/HttpAbstractions#594 and issue aspnet/HttpAbstractions#561

dougbu · 2016-03-25T19:18:30Z

Part two for the actual fix:

80813f7
aspnet/Hosting@b06a844 (always register an ObjectPoolProvider)
aspnet/HttpSysServer@f0a5a01 (react to HttpContextFactory constructor change)
aspnet/Antiforgery@0c81df8 (no need to register an ObjectPoolProvider)
aspnet/Mvc@43836ae (no need to register an ObjectPoolProvider)
aspnet/Routing@39b4f6f (no need to register an ObjectPoolProvider)

muratg added the Perf label Mar 14, 2016

dougbu self-assigned this Mar 14, 2016

dougbu added this to the 1.0.0-rc2 milestone Mar 14, 2016

dougbu mentioned this issue Mar 14, 2016

Excessively large number of StringBuilder objects allocated in FormReader for big input data #583

Closed

dougbu added the 2 - Working label Mar 15, 2016

dougbu added a commit to aspnet/Hosting that referenced this issue Mar 15, 2016

Add ObjectPool<StringBuilder> to default services

e568e43

- can now use pooled `StringBuilder`s to address aspnet/HttpAbstractions#561 and similar issues nit: Fix incomplete test noted when adding new one

dougbu mentioned this issue Mar 15, 2016

Add ObjectPool<StringBuilder> to default services aspnet/Hosting#652

Closed

dougbu added a commit that referenced this issue Mar 15, 2016

Use pooled StringBuilder to reduce allocations when adding response…

4c3a3ea

… cookies - #561

dougbu mentioned this issue Mar 15, 2016

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

Closed

dougbu mentioned this issue Mar 17, 2016

Move remaining feature interfaces into Microsoft.AspNetCore.Http.Features package and namespace #589

Closed

dougbu added a commit to aspnet/Hosting that referenced this issue Mar 18, 2016

React to HttpAbstractions change: No features in .Internal namespace

af49efe

- see issue aspnet/HttpAbstractions#561 and pull aspnet/HttpAbstractions#589

dougbu added a commit to aspnet/KestrelHttpServer that referenced this issue Mar 18, 2016

React to HttpAbstractions change: No features in .Internal namespace

bf4c1fc

- see issue aspnet/HttpAbstractions#561 and pull aspnet/HttpAbstractions#589

dougbu added a commit to aspnet/Security that referenced this issue Mar 18, 2016

React to HttpAbstractions change: No features in .Internal namespace

91c4d47

- see issue aspnet/HttpAbstractions#561 and pull aspnet/HttpAbstractions#589

dougbu added a commit to aspnet/KestrelHttpServer that referenced this issue Mar 19, 2016

React to HttpAbstractions change: No features in .Internal namespace

f13c418

- see issue aspnet/HttpAbstractions#561 and pull aspnet/HttpAbstractions#589

dougbu added a commit to aspnet/Security that referenced this issue Mar 19, 2016

React to HttpAbstractions change: No features in .Internal namespace

9361960

- see issue aspnet/HttpAbstractions#561 and pull aspnet/HttpAbstractions#589

dougbu mentioned this issue Mar 20, 2016

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

Closed

dougbu added a commit to aspnet/Hosting that referenced this issue Mar 21, 2016

React to HttpAbstractions change: No features in .Internal namespace

bb3809d

- see issue aspnet/HttpAbstractions#561 and pull aspnet/HttpAbstractions#589

dougbu mentioned this issue Mar 21, 2016

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

Closed

dougbu added a commit to aspnet/Session that referenced this issue Mar 23, 2016

Ensure an ObjectPoolProvider is registered

289cd92

- take advantage of aspnet/HttpAbstractions#561 fix

dougbu added a commit to aspnet/Security that referenced this issue Mar 23, 2016

Ensure an ObjectPoolProvider is registered

e6791cc

- take advantage of aspnet/HttpAbstractions#561 fix

This was referenced Mar 23, 2016

Ensure an ObjectPoolProvider is registered aspnet/Security#752

Closed

Ensure an ObjectPoolProvider is registered aspnet/Session#102

Closed

dougbu added a commit to aspnet/Hosting that referenced this issue Mar 23, 2016

Ensure an ObjectPoolProvider is registered

bcd5500

- e.g. take advantage of aspnet/HttpAbstractions#561 fix wherever cookies are used

dougbu mentioned this issue Mar 23, 2016

Ensure an ObjectPoolProvider is registered aspnet/Hosting#673

Closed

dougbu added a commit to aspnet/HttpSysServer that referenced this issue Mar 24, 2016

React to HttpContextFactory constructor change

8d096cc

- see pull aspnet/HttpAbstractions#594 and issue aspnet/HttpAbstractions#561

dougbu mentioned this issue Mar 24, 2016

React to HttpContextFactory constructor change aspnet/HttpSysServer#184

Closed

dougbu added a commit to aspnet/Hosting that referenced this issue Mar 25, 2016

Ensure an ObjectPoolProvider is registered

b06a844

- e.g. take advantage of aspnet/HttpAbstractions#561 fix wherever cookies are used

dougbu added a commit to aspnet/HttpSysServer that referenced this issue Mar 25, 2016

React to HttpContextFactory constructor change

f0a5a01

- remove unused field - see pull aspnet/HttpAbstractions#594 and issue aspnet/HttpAbstractions#561

dougbu added 3 - Done and removed 2 - Working labels Mar 25, 2016

dougbu closed this as completed Mar 25, 2016

pranavkm mentioned this issue Jul 29, 2016

Add dotnet-razor-precompile tool aspnet/Mvc#5087

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

`ResponseCookies.Append()` allocates lots, big lots #561

`ResponseCookies.Append()` allocates lots, big lots #561

dougbu commented Feb 16, 2016

muratg commented Mar 14, 2016

Uh oh!

dougbu commented Mar 14, 2016

Uh oh!

dougbu commented Mar 14, 2016

Uh oh!

muratg commented Mar 15, 2016

Uh oh!

dougbu commented Mar 21, 2016

Uh oh!

dougbu commented Mar 25, 2016

Uh oh!

ResponseCookies.Append() allocates lots, big lots #561

ResponseCookies.Append() allocates lots, big lots #561

Comments

dougbu commented Feb 16, 2016

muratg commented Mar 14, 2016

Uh oh!

dougbu commented Mar 14, 2016

Uh oh!

dougbu commented Mar 14, 2016

Uh oh!

muratg commented Mar 15, 2016

Uh oh!

dougbu commented Mar 21, 2016

Uh oh!

dougbu commented Mar 25, 2016

Uh oh!

`ResponseCookies.Append()` allocates lots, big lots #561

`ResponseCookies.Append()` allocates lots, big lots #561