-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Performance tweaks #2667
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
Performance tweaks #2667
Conversation
- Avoid LINQ to create components. - Do not recreate `Outcome<T>` for chaos. - Avoid predicate allocation. - Use `ArgumentNullException.ThrowIfNull()` where possible. - Add throw helper for `ObjectDisposedException`. - Remove unused `Stopwatch`. - Use `Volatile.Read` instead of `Interlocked.CompareExchange()`. Cherry-picked from #2664. Co-Authored-By: Pent Ploompuu <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## main #2667 +/- ##
==========================================
- Coverage 96.23% 96.23% -0.01%
==========================================
Files 311 311
Lines 7329 7322 -7
Branches 1013 1012 -1
==========================================
- Hits 7053 7046 -7
Misses 222 222
Partials 54 54
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Update the benchmarks for latest run.
|
Ran benchmarks on main and then compared to the results pushed into this PR. Almost all benchmarks had improvements, though not necessarily large. Some benchmarks that didn't show an improvement were within the margin of error for measurement noise. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR applies a series of low-level performance optimizations across the pipeline utilities, reducing allocations and improving synchronization checks.
- Swaps
Interlocked.CompareExchangeforVolatile.Readto check pending executions. - Introduces a throw helper for
ObjectDisposedExceptionand usesArgumentNullException.ThrowIfNull()when available. - Replaces LINQ allocation patterns and streamlines default predicate instantiation.
Reviewed Changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| ExecutionTrackingComponent.cs | Uses Volatile.Read instead of Interlocked.CompareExchange |
| ComponentDisposeHelper.cs | Extracts throw logic into a dedicated helper method |
| Guard.cs | Calls ArgumentNullException.ThrowIfNull() under .NET 8+ |
| DefaultPredicates.cs | Replaces switch-expression allocation with direct ValueTask |
| ResiliencePipelineBuilderBase.cs | Uses List<T>.ConvertAll instead of LINQ Select().ToList() |
Comments suppressed due to low confidence (1)
src/Polly.Core/Utils/Guard.cs:12
- Add or update unit tests to cover the
ArgumentNullException.ThrowIfNullpath under .NET 8+ to ensure the guard behaves as expected.
#if NET8_0_OR_GREATER
Outcome<T>for chaos.ArgumentNullException.ThrowIfNull()where possible.ObjectDisposedException.Stopwatch.Volatile.Readinstead ofInterlocked.CompareExchange().Cherry-picked from #2664.