-
-
Notifications
You must be signed in to change notification settings - Fork 837
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 for v6.0 #1127
Comments
Addressed the |
Addressed the Circular Dependency item in #1148. |
Run the full benchmark set against Priority next is to figure out the concurrency problem, then I can look at the others. summary:
|
Latest benchmark state following the merge of the concurrency changes. Almost everything looks good across the board, particularly on concurrency now. Of the three slower ones, 1 is a baseline, so is just margin-of-error. The other 2 I'm not sure. I'm going to run another test set overnight to check for random slowness. summary:
|
Re-ran the benchmarks, got only 1 slower bench this time, and it was a completely different one, so I'm putting it down to small variations in processor behaviour between runs. Given the current results, I'll consider any v6-introduced perf issues resolved, so any additional fixes will be just nice-to-haves. |
Last compare I ran vs develop gives me:
This makes me a bit sad because at one point almost all of them were faster at one point. I'll take a look at what's going on, but it might just be the known overhead of a few of the paths. |
As of #1194, performance is now faster almost across-the-board, barring some margin-of-error variations. I'm going to close this now, and barring any urgent stuff, I'm going to suggest we freeze further Autofac changes, and focus on getting v6 out the door. |
There's a few things left to do from a performance perspective following the pipelines changes.
CircularDependencyMiddleware.Execute
stack sees PushWithResize a bit, might be worth allocating some memory up frontResolveOperationBase.ResetSuccessfulRequests
resets the list with new, but would it be better to Clear() or something else?The text was updated successfully, but these errors were encountered: