Improve the V7 -> V8 bridge implementation (#1257)

BenchmarkDotNet.Artifacts/results/Polly.Core.Benchmarks.BridgeBenchmark-report-github.md

@@ -0,0 +1,15 @@
+``` ini
+
+BenchmarkDotNet=v0.13.5, OS=Windows 11 (10.0.22621.1702/22H2/2022Update/SunValley2), VM=Hyper-V
+Intel Xeon Platinum 8370C CPU 2.80GHz, 1 CPU, 16 logical and 8 physical cores
+.NET SDK=7.0.302
+  [Host] : .NET 7.0.5 (7.0.523.17405), X64 RyuJIT AVX2
+
+Job=MediumRun  Toolchain=InProcessEmitToolchain  IterationCount=15  
+LaunchCount=2  WarmupCount=10  
+
+```
+|                 Method |      Mean |    Error |   StdDev | Ratio | RatioSD |   Gen0 | Allocated | Alloc Ratio |
+|----------------------- |----------:|---------:|---------:|------:|--------:|-------:|----------:|------------:|
+|              NoOpAsync |  85.38 ns | 1.001 ns | 1.371 ns |  1.00 |    0.00 | 0.0120 |     304 B |        1.00 |
+| NullResilienceStrategy | 416.38 ns | 1.924 ns | 2.820 ns |  4.87 |    0.10 | 0.0148 |     376 B |        1.24 |

src/Polly.Core.Benchmarks/BridgeBenchmark.cs

@@ -0,0 +1,22 @@
+using BenchmarkDotNet.Attributes;
+
+namespace Polly.Core.Benchmarks;
+
+public class BridgeBenchmark
+{
+    private IAsyncPolicy<string>? _policy;
+    private IAsyncPolicy<string>? _policyWrapped;
+
+    [GlobalSetup]
+    public void Setup()
+    {
+        _policy = Policy.NoOpAsync<string>();
+        _policyWrapped = NullResilienceStrategy<string>.Instance.AsAsyncPolicy();
+    }
+
+    [Benchmark(Baseline = true)]
+    public Task NoOpAsync() => _policy!.ExecuteAsync(() => Task.FromResult("dummy"));
+
+    [Benchmark]
+    public Task NullResilienceStrategy() => _policyWrapped!.ExecuteAsync(() => Task.FromResult("dummy"));
+}

src/Polly.Core.Tests/Utils/LegacySupportTests.cs

@@ -0,0 +1,20 @@
+using System.Collections.Generic;
+using Polly.Utils;
+
+namespace Polly.Core.Tests.Utils;
+
+public class LegacySupportTests
+{
+    [Fact]
+    public void SetProperties_Ok()
+    {
+        var resilienceProperties = new ResilienceProperties();
+        var oldProps = resilienceProperties.Options;
+        var newProps = new Dictionary<string, object?>();
+
+        resilienceProperties.SetProperties(newProps, out var oldProperties2);
+
+        resilienceProperties.Options.Should().BeSameAs(newProps);
+        oldProperties2.Should().BeSameAs(oldProps);
+    }
+}

src/Polly.Core/ResilienceProperties.cs

@@ -9,7 +9,7 @@ namespace Polly;
 /// </summary>
 public sealed class ResilienceProperties : IDictionary<string, object?>
 {
-    private Dictionary<string, object?> Options { get; } = new();
+    internal IDictionary<string, object?> Options { get; set; } = new Dictionary<string, object?>();
 
     /// <summary>
     /// Gets the value of a given property.
@@ -87,26 +87,26 @@ internal void Replace(ResilienceProperties other)
     int ICollection<KeyValuePair<string, object?>>.Count => Options.Count;
 
     /// <inheritdoc/>
-    bool ICollection<KeyValuePair<string, object?>>.IsReadOnly => ((IDictionary<string, object?>)Options).IsReadOnly;
+    bool ICollection<KeyValuePair<string, object?>>.IsReadOnly => Options.IsReadOnly;
 
     /// <inheritdoc/>
     void IDictionary<string, object?>.Add(string key, object? value) => Options.Add(key, value);
 
     /// <inheritdoc/>
-    void ICollection<KeyValuePair<string, object?>>.Add(KeyValuePair<string, object?> item) => ((IDictionary<string, object?>)Options).Add(item);
+    void ICollection<KeyValuePair<string, object?>>.Add(KeyValuePair<string, object?> item) => Options.Add(item);
 
     /// <inheritdoc/>
     void ICollection<KeyValuePair<string, object?>>.Clear() => Options.Clear();
 
     /// <inheritdoc/>
-    bool ICollection<KeyValuePair<string, object?>>.Contains(KeyValuePair<string, object?> item) => ((IDictionary<string, object?>)Options).Contains(item);
+    bool ICollection<KeyValuePair<string, object?>>.Contains(KeyValuePair<string, object?> item) => Options.Contains(item);
 
     /// <inheritdoc/>
     bool IDictionary<string, object?>.ContainsKey(string key) => Options.ContainsKey(key);
 
     /// <inheritdoc/>
     void ICollection<KeyValuePair<string, object?>>.CopyTo(KeyValuePair<string, object?>[] array, int arrayIndex) =>
-        ((IDictionary<string, object?>)Options).CopyTo(array, arrayIndex);
+        Options.CopyTo(array, arrayIndex);
 
     /// <inheritdoc/>
     IEnumerator<KeyValuePair<string, object?>> IEnumerable<KeyValuePair<string, object?>>.GetEnumerator() => Options.GetEnumerator();
@@ -118,7 +118,7 @@ internal void Replace(ResilienceProperties other)
     bool IDictionary<string, object?>.Remove(string key) => Options.Remove(key);
 
     /// <inheritdoc/>
-    bool ICollection<KeyValuePair<string, object?>>.Remove(KeyValuePair<string, object?> item) => ((IDictionary<string, object?>)Options).Remove(item);
+    bool ICollection<KeyValuePair<string, object?>>.Remove(KeyValuePair<string, object?> item) => Options.Remove(item);
 
     /// <inheritdoc/>
     bool IDictionary<string, object?>.TryGetValue(string key, out object? value) => Options.TryGetValue(key, out value);

src/Polly.Core/Utils/LegacySupport.cs

@@ -0,0 +1,35 @@
+using System.ComponentModel;
+
+namespace Polly.Utils;
+
+/// <summary>
+/// Legacy support for older versions of Polly.
+/// </summary>
+/// <remarks>
+/// This class is used by the legacy Polly infrastructure and should not be used directly by user code.
+/// </remarks>
+[EditorBrowsable(EditorBrowsableState.Never)]
+public static class LegacySupport
+{
+    /// <summary>
+    /// Changes the underlying properties of a <see cref="ResilienceProperties"/> instance.
+    /// </summary>
+    /// <param name="resilienceProperties">The resilience properties.</param>
+    /// <param name="properties">The properties to use.</param>
+    /// <param name="oldProperties">The old properties used by <paramref name="resilienceProperties"/>.</param>
+    /// <remarks>
+    /// This method is used by the legacy Polly infrastructure and should not be used directly by user code.
+    /// </remarks>
+    [EditorBrowsable(EditorBrowsableState.Never)]
+    public static void SetProperties(
+        this ResilienceProperties resilienceProperties,
+        IDictionary<string, object?> properties,
+        out IDictionary<string, object?> oldProperties)
+    {
+        Guard.NotNull(resilienceProperties);
+        Guard.NotNull(properties);
+
+        oldProperties = resilienceProperties.Options;
+        resilienceProperties.Options = properties;
+    }
+}

src/Polly.Specs/ResilienceStrategyConversionExtensionsTests.cs

@@ -1,5 +1,6 @@
 using Polly.Strategy;
 using Polly.TestUtils;
+using Polly.Utilities.Wrappers;
 
 namespace Polly.Specs;
 
@@ -167,17 +168,20 @@ public void RetryStrategy_AsSyncPolicy_Ok()
             .Build()
             .AsSyncPolicy();
 
-        var tries = 0;
+        var context = new Context();
+        context["retry"] = 0;
+
         policy.Execute(
-            () =>
+            c =>
             {
-                tries++;
+                c["retry"] = (int)c["retry"] + 1;
                 return "dummy";
-            })
+            },
+            context)
             .Should()
             .Be("dummy");
 
-        tries.Should().Be(6);
+        context["retry"].Should().Be(6);
     }
 
     private static void AssertContext(Context context)

src/Polly/Utilities/Wrappers/ResilienceContextFactory.cs

@@ -1,42 +1,26 @@
+using Polly.Utils;
+
 namespace Polly.Utilities.Wrappers;
 
 internal static class ResilienceContextFactory
 {
-    public static readonly ResiliencePropertyKey<Context> ContextKey = new("Polly.Legacy.Context");
-
-    public static ResilienceContext Create(Context context, CancellationToken cancellationToken, bool continueOnCapturedContext)
+    public static ResilienceContext Create(
+        Context context,
+        CancellationToken cancellationToken,
+        bool continueOnCapturedContext,
+        out IDictionary<string, object> oldProperties)
     {
         var resilienceContext = ResilienceContext.Get();
         resilienceContext.CancellationToken = cancellationToken;
         resilienceContext.ContinueOnCapturedContext = continueOnCapturedContext;
-
-        foreach (var pair in context)
-        {
-            var props = (IDictionary<string, object>)resilienceContext.Properties;
-            props.Add(pair.Key, pair.Value);
-        }
-
-        resilienceContext.Properties.Set(ContextKey, context);
+        resilienceContext.Properties.SetProperties(context, out oldProperties);
 
         return resilienceContext;
     }
 
-    public static void Restore(ResilienceContext context)
+    public static void Cleanup(ResilienceContext resilienceContext, IDictionary<string, object> oldProperties)
     {
-        var originalContext = context.GetContext();
-
-        foreach (var pair in context.Properties)
-        {
-            if (pair.Key == ContextKey.Key)
-            {
-                continue;
-            }
-
-            originalContext[pair.Key] = pair.Value;
-        }
-
-        ResilienceContext.Return(context);
+        resilienceContext.Properties.SetProperties(oldProperties, out _);
+        ResilienceContext.Return(resilienceContext);
     }
-
-    public static Context GetContext(this ResilienceContext resilienceContext) => resilienceContext.Properties.GetValue(ContextKey, null!);
 }

src/Polly/Utilities/Wrappers/ResilienceStrategyAsyncPolicy.TResult.cs

@@ -8,21 +8,25 @@ internal sealed class ResilienceStrategyAsyncPolicy<TResult> : AsyncPolicy<TResu
 
     protected sealed override async Task<TResult> ImplementationAsync(Func<Context, CancellationToken, Task<TResult>> action, Context context, CancellationToken cancellationToken, bool continueOnCapturedContext)
     {
-        var resilienceContext = ResilienceContextFactory.Create(context, cancellationToken, continueOnCapturedContext);
+        var resilienceContext = ResilienceContextFactory.Create(
+            context,
+            cancellationToken,
+            continueOnCapturedContext,
+            out var oldProperties);
 
         try
         {
             return await _strategy.ExecuteAsync(
-                static async (context, state) =>
+                static async (resilienceContext, state) =>
                 {
-                    return await state(context.GetContext(), context.CancellationToken).ConfigureAwait(context.ContinueOnCapturedContext);
+                    return await state.action(state.context, resilienceContext.CancellationToken).ConfigureAwait(resilienceContext.ContinueOnCapturedContext);
                 },
                 resilienceContext,
-                action).ConfigureAwait(continueOnCapturedContext);
+                (action, context)).ConfigureAwait(continueOnCapturedContext);
         }
         finally
         {
-            ResilienceContextFactory.Restore(resilienceContext);
+            ResilienceContextFactory.Cleanup(resilienceContext, oldProperties);
         }
     }
 }

src/Polly/Utilities/Wrappers/ResilienceStrategyAsyncPolicy.cs

@@ -12,21 +12,25 @@ protected sealed override async Task ImplementationAsync(
         CancellationToken cancellationToken,
         bool continueOnCapturedContext)
     {
-        var resilienceContext = ResilienceContextFactory.Create(context, cancellationToken, continueOnCapturedContext);
+        var resilienceContext = ResilienceContextFactory.Create(
+            context,
+            cancellationToken,
+            continueOnCapturedContext,
+            out var oldProperties);
 
         try
         {
             await _strategy.ExecuteAsync(
-                static async (context, state) =>
+                static async (resilienceContext, state) =>
                 {
-                    await state(context.GetContext(), context.CancellationToken).ConfigureAwait(context.ContinueOnCapturedContext);
+                    await state.action(state.context, resilienceContext.CancellationToken).ConfigureAwait(resilienceContext.ContinueOnCapturedContext);
                 },
                 resilienceContext,
-                action).ConfigureAwait(continueOnCapturedContext);
+                (action, context)).ConfigureAwait(continueOnCapturedContext);
         }
         finally
         {
-            ResilienceContextFactory.Restore(resilienceContext);
+            ResilienceContextFactory.Cleanup(resilienceContext, oldProperties);
         }
     }
 
@@ -36,21 +40,25 @@ protected override async Task<TResult> ImplementationAsync<TResult>(
         CancellationToken cancellationToken,
         bool continueOnCapturedContext)
     {
-        var resilienceContext = ResilienceContextFactory.Create(context, cancellationToken, continueOnCapturedContext);
+        var resilienceContext = ResilienceContextFactory.Create(
+            context,
+            cancellationToken,
+            continueOnCapturedContext,
+            out var oldProperties);
 
         try
         {
             return await _strategy.ExecuteAsync(
-                static async (context, state) =>
+                static async (resilienceContext, state) =>
                 {
-                    return await state(context.GetContext(), context.CancellationToken).ConfigureAwait(context.ContinueOnCapturedContext);
+                    return await state.action(state.context, resilienceContext.CancellationToken).ConfigureAwait(resilienceContext.ContinueOnCapturedContext);
                 },
                 resilienceContext,
-                action).ConfigureAwait(continueOnCapturedContext);
+                (action, context)).ConfigureAwait(continueOnCapturedContext);
         }
         finally
         {
-            ResilienceContextFactory.Restore(resilienceContext);
+            ResilienceContextFactory.Cleanup(resilienceContext, oldProperties);
         }
     }
 }

src/Polly/Utilities/Wrappers/ResilienceStrategySyncPolicy.TResult.cs

@@ -8,21 +8,25 @@ internal sealed class ResilienceStrategySyncPolicy<TResult> : Policy<TResult>
 
     protected sealed override TResult Implementation(Func<Context, CancellationToken, TResult> action, Context context, CancellationToken cancellationToken)
     {
-        var resilienceContext = ResilienceContextFactory.Create(context, cancellationToken, true);
+        var resilienceContext = ResilienceContextFactory.Create(
+            context,
+            cancellationToken,
+            true,
+            out var oldProperties);
 
         try
         {
             return _strategy.Execute(
                 static (context, state) =>
                 {
-                    return state(context.GetContext(), context.CancellationToken);
+                    return state.action(state.context, context.CancellationToken);
                 },
                 resilienceContext,
-                action);
+                (action, context));
         }
         finally
         {
-            ResilienceContextFactory.Restore(resilienceContext);
+            ResilienceContextFactory.Cleanup(resilienceContext, oldProperties);
         }
     }
 }