-
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
Use singleton SwapGate
in Sabre reconstruction
#10865
Conversation
One or more of the the following people are requested to review this:
|
With #10314 merged, both |
In my local testing the overhead of calling
with #10314. This is actually a significant regression because before #10314 it took ~666ns:
so doing this can save a noticeable amount of time depending on how many swaps we need to create. |
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.
This LGTM, it's a straightforward optimization. We should look at the runtime overhead of SingletonGate.__new__
in parallel to try and minimize the overhead there.
Fwiw, there's a bunch of "hidden" overhead of
One potential way around this is to have the singleton class be a metaclass that overrides |
Since `SwapGate` is now a singleton instance in most cases, we can directly reuse the same instance during Sabre reconstruction rather than wasting cycles re-retrieving the singleton instance.
6270720
to
32a8842
Compare
Now rebased over #10782. |
Pull Request Test Coverage Report for Build 6250595529
💛 - Coveralls |
Summary
Since
SwapGate
is now a singleton instance in most cases, we can directly reuse the same instance during Sabre reconstruction rather than wasting cycles re-retrieving the singleton instance.Details and comments
See #10314 for the root of this.