-
Notifications
You must be signed in to change notification settings - Fork 577
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
cache extract_lambda_source #3966
cache extract_lambda_source #3966
Conversation
81852e2
to
cc1aee8
Compare
👍 Some general comments, for consideration.
[edit to clarify: By "here", I mean at the call site in |
I like the idea of using
The contrasting concern is that if the arguments are mutated, the deferred reprs end up actively misleading. Given the nature of stateful tests, I think we should probably continue to repr in advance until we get a complaint or see a profile which shows that repr performance is a problem here in practice. |
Coverage issue is tracked in #3968, I'll aim to get that over the weekend if nobody beats me to it. |
Good call with |
6ddf158
to
cebe6cb
Compare
cebe6cb
to
dfb720e
Compare
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.
🚀
Addresses the performance part of #3963. Have not profiled memory. It should also be significantly reduced as we're calling
ast.parse
less now, but this will not address any underlying memory leak, if one exists.