dotnet / dotnet/aspnetcore

ResponseCachingKeyProvider Performance Optimisations

Open
#10,290 13 comments 4 reactions 0 assignees View on GitHub
affected-very-few area-middleware enhancement feature-response-caching severity-nice-to-have
Dominant language
C#
Stars
38.4k
Forks
10.9k
Avg merge
2d 10h
Merged PRs (30d)
281

Description

> tl;dr; This is a query about whether the team would accept performance optimisations for the `ResponseCachingKeyProvider` & related components?

I've recently been playing around with String.Create for an upcoming blog post and I remembered a discussion with @rynowak about ObjectPool being used to pool StringBuilder instances for some of the ASP.NET Core middleware.

One example I found is in [ResponseCachingKeyProvider](https://github.com/aspnet/AspNetCore/blob/master/src/Middleware/ResponseCaching/src/Internal/ResponseCachingKeyProvider.cs)

Just for my own curiosity, I had a play with a new version of `CreateBaseKey` which doesn't require a StringBuilder and uses String.Create instead. My theory is that with the other methods updated also (if possible) then maybe the need to depend on an ObjectPoolProvider and StringBuilderPool could be removed?

I did a very quick and basic version using String.Create and benchmarked it with the following results...

```
| Method | Mean | Error | StdDev | Gen 0 | Gen 1 | Gen 2 | Allocated |
|--------------- |---------:|----------:|----------:|-------:|------:|------:|----------:|
| GetKeyOriginal | 435.5 ns | 8.2836 ns | 8.1356 ns | 0.0191 | - | - | 120 B |
| GetKeyNew | 341.1 ns | 0.6806 ns | 0.5313 ns | 0.0191 | - | - | 120 B |
```

Is there any value in pursuing this for this provider and perhaps others as part of the on-going performance work? If so, I'd be keen to try and spend a little time looking deeper on this particular feature. Note that the code is untested and just a proof of concept at this point.

Perhaps @davidfowl and @benaadams also have some thoughts on the value of this?

For completeness, my rough code used for the benchmark:

```csharp
public string CreateBaseKeyStringCreate(ResponseCachingContext context)
{
if (context == null)
{
throw new ArgumentNullException(nameof(context));
}

var request = context.HttpContext.Request;

var length = request.Method.Length +
1 +
request.Scheme.Length +
1 +
request.Host.Value.Length +
request.PathBase.Value.Length +
request.Path.Value.Length;

var key = string.Create(length, (request, _options), (chars, state) =>
{
var (request, options) = state;
var position = 0;

request.Method.AsSpan().ToUpperInvariant(chars);
position += request.Method.Length;

chars[position++] = KeyDelimiter;

request.Scheme.AsSpan().ToUpperInvariant(chars.Slice(position));
position += request.Scheme.Length;

chars[position++] = KeyDelimiter;

request.Host.Value.AsSpan().ToUpperInvariant(chars.Slice(position));
position += request.Host.Value.Length;

var pathBaseSpan = request.PathBase.Value.AsSpan();
var pathSpan = request.Path.Value.AsSpan();

if (options.UseCaseSensitivePaths)
{
pathBaseSpan.ToUpperInvariant(chars.Slice(position));
position += pathBaseSpan.Length;

pathSpan.ToUpperInvariant(chars.Slice(position));
}
else
{
pathBaseSpan.CopyTo(chars.Slice(position));
position += pathBaseSpan.Length;

pathSpan.CopyTo(chars.Slice(position));
}
});

return key;
}
```

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.