Benchmark for strategy creation

[martintmk]

Details on the issue fix or feature implementation

Benchmark that demonstrates the creation of strategies (V7 vs V8).

Confirm the following

• I started this PR by branching from the head of the default branch
• I have targeted the PR to merge into the default branch
• I have included unit tests for the issue/feature
• I have successfully run a local build

[martintmk]

The creation in V8 is indeed more expensive as it involves a lot more infra. For this reason the recreation of strategies on hot path should be discouraged.

[martincostello]

What would your generic recommendation be?

In the sandbox app the fallback behaviour is trivial and could be cached, but in the code on which it was based there's dynamic per-invocation behaviour going on to select the fallback.

It feels that to support the same behaviour the more performant option would be to drop Polly fallbacks entirely and deal with it at the call site like any "normal" code, which if true doesn't feel great that the performance for this scenario has actually gone backwards.

[martintmk]

I don't think I have seen a scenario where you are "forced" to recreate a strategy on a hot path.

At worst you need to fallback to some dynamic value, but that can be archived by passing such value to the fallback action using ResilienceContext.

[martintmk]

In you code the caching could look like:

[martincostello]

Could you paste the code rather than a picture of it? 😄

[martintmk]

Sure, I tried to create a draft PR to your repo and got rejected.

public ResilienceStrategy<TResult> GetStrategy<TResult>(ApiEndpointOption endpoint, bool handleExecutionFaults, string resource)
{
    return _registry.GetOrAddStrategy<TResult>($"fallback/{resource}/{handleExecutionFaults}", builder =>
    {
        var shouldHandle = new PredicateBuilder<TResult>()
            .Handle<ApiException>()
            .HandleHttpRequestFault()
            .Handle<TaskCanceledException>();

        if (handleExecutionFaults)
        {
            shouldHandle = shouldHandle
                .Handle<BrokenCircuitException>()
                .Handle<IsolatedCircuitException>()
                .Handle<TimeoutRejectedException>();
        }

        builder
            .AddStrategy(GetStrategy(endpoint, resource))
            .AddFallback(new FallbackStrategyOptions<TResult>()
            {
                FallbackAction = context =>
                {
                    if (context.Context.Properties.TryGetValue(FallbackKeys<TResult>.FallbackValue, out var value) && value != null)
                    {
                        return Outcome.FromResultAsTask(value());
                    }

                    return Outcome.FromResultAsTask<TResult>(default);
                },
                ShouldHandle = shouldHandle,
                StrategyName = $"{endpoint.Name} Fallback",
            });
    });
}

[martincostello]

Do you have the code for FallbackKeys?

Nevermind, I can see it in the screenshot.

[martincostello]

Wow that's a lot more 😅

[martintmk]

I think validation eats a lot of that as it involves reflection.

[codecov]

Codecov Report

Merging #1426 (7ff337a) into main (b9ce232) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #1426   +/-   ##
=======================================
  Coverage   83.92%   83.92%           
=======================================
  Files         275      275           
  Lines        6526     6526           
  Branches     1007     1007           
=======================================
  Hits         5477     5477           
  Misses        840      840           
  Partials      209      209           

Flag	Coverage Δ
macos	83.92% <ø> (ø)
windows	83.92% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.