-
-
Notifications
You must be signed in to change notification settings - Fork 82
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
Speed up import time by deferring inspect #499
Conversation
importlib_metadata/__init__.py
Outdated
@@ -1071,6 +1070,9 @@ def _topmost(name: PackagePath) -> Optional[str]: | |||
return top if rest else None | |||
|
|||
|
|||
inspect = None |
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 is a trick that is used in stdlib. I am not sure it is needed here, but since get_toplevel_name
is called in a loop from _top_level_inferred
perhaps it is warranted to avoid the overhead of calling import inspect
repeatedly.
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'm not a fan of global variables or the additional complexity. This change introduces enough disruption to the essential logical flow that I'm -1. Do we know how much overhead there is in repeated import inspect
? My understanding (which may be incorrect) is that import inspect
is essentially a dict lookup if it's already been imported. I'm guessing the overhead is acceptable. Can we try a simple deferral for now?
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.
Thanks for the contrib!
importlib_metadata/__init__.py
Outdated
@@ -1071,6 +1070,9 @@ def _topmost(name: PackagePath) -> Optional[str]: | |||
return top if rest else None | |||
|
|||
|
|||
inspect = None |
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'm not a fan of global variables or the additional complexity. This change introduces enough disruption to the essential logical flow that I'm -1. Do we know how much overhead there is in repeated import inspect
? My understanding (which may be incorrect) is that import inspect
is essentially a dict lookup if it's already been imported. I'm guessing the overhead is acceptable. Can we try a simple deferral for now?
importlib_metadata/__init__.py
Outdated
|
||
global inspect | ||
if inspect is None: | ||
import inspect |
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.
Since we don't have a test protecting this behavior, I'd like to see a comment pointing to the issue, so a future someone doesn't refactor this optimization away.
- wallrusify - add a note about deffered import
I've minimized the change to address the specific issue. I'll deal with the typeshed ignore workaround separately. |
I should have added a news fragment before merging. I added it later in 71b4678. |
This change is released in v8.4.0. |
Deferring import of
inspect
cuts the import time by ~10% (4ms on my machine).CPython issue: python/cpython#118761
I haven't been able to run the tests locally, seeing these errors:
Benchmarks
These have been run with latest CPython main branch (as of Aug 6th 2024), these gains are likely representative for Python 3.13, but not 3.12.
this PR
main