-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Small optimizations for the ExtensionManager #16382
Small optimizations for the ExtensionManager #16382
Conversation
…hey are not related to a specific feature. Also fixes LoadFeaturesAsync() to return the ordered features list.
This pull request has merge conflicts. Please resolve those before requesting a review. |
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.
Have you done any performance benchmark to understand what is the performance gain here?
I did not measure this, because I don't think it is worth the effort to test it. Same as e.g. making the classes |
I undid the initialization change and moved it back to the original So there are only a few optimizations left here, e.g. sealing the class and removing some fields that don't need to be kept alive over the lifetime of the application. |
Co-authored-by: Mike Alhayek <[email protected]>
Co-authored-by: Mike Alhayek <[email protected]>
The initialization of theExtensionManager
is synchronous and must always be run before any method can be called. It can therefore be moved into the constructor to avoid a check before any method call.This is a minor performance optimization, but
EnsureInitialized
was called at least once during every request and on some requests dozens of times.I based this optimization on PR #16379.
Note: After discussion, only some minor optimizations are kept here.