-
Notifications
You must be signed in to change notification settings - Fork 917
use Borrowed in extract_argument functions
#5695
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
Conversation
Tpt
left a comment
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.
I am not an expert on this code but it seems sensible to me
Icxolu
left a comment
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.
LGTM as well.
It looks like there are not many usages of BoundRef left. Any change that we can adjust them as well and remove it completely?
Co-authored-by: Thomas Tanon <thomas@pellissier-tanon.fr>
Potentially as a follow up, yes. It's not completely trivial as there are cases like |
Merging this PR will improve performance by 10.21%Summary
Performance Changes
Comparing |
In progress work to close #5392
I realised that using
Borrowedallows'ato outlive'pyin argument extraction. This is not always strictly necessary, but in the case of the async code I'm looking into in #5681 it turns out that it's valid for function arguments to have the'alifetime but not the'pylifetime.This might also be a performance help by removing the need to have references to pointers on stack frames for theEDIT: looks like it doesn't really affect performance, but I still think it's useful for async code.BoundRefstuff.