-
-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
bpo-33587: inspect.getsource: reorder stat on file in linecache #6805
Conversation
This looks like a simple change to me. If I were to create a bug on bpo, what should it mention? Would something like "inspect.getsource() does unnecessary filesystem stat when file is already in linecache" suffice? |
Please open an issue on the bug tracker for discussion and provide your benchmark results. |
Thanks for your advise @matrixise and @serhiy-storchaka . I have added a bug https://bugs.python.org/issue33587 and posted a cProfile performance comparison there. Which is the canonical place to discuss the patch (performance numbers and other things), over here at github or at bpo? Here's the patch performance difference before and after the patch: test_code.py:
Before:
After the patch:
Total runtime reduced from 4.8 s to 2.8 s, and the major difference can be seen in the |
@pankajp, the place to discuss proposed changes and bugs is indeed the issue tracker. |
The check for os.path.exists() on source file is postponed in inspect.getsourcefile() until needed avoiding an expensive filesystem stat call and PEP 302 module loader check is moved last for performance since it is an uncommon case.
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.
LGTM, but the merge conflicts need to be resolved. |
@pankajp: While I understand it's been a long time, it looks still relevant. Are you interested in rebasing this and re-running the benchmarks to get this merged? |
The check for os.path.exists() on source file is postponed in
inspect.getsourcefile() until needed avoiding an expensive filesystem stat call.
https://bugs.python.org/issue33587