Skip to content

GH-109653: Defer import of importlib.metadata._adapters - #109829

Merged
jaraco merged 3 commits into
python:mainfrom
AA-Turner:importtime/importlib.metadata
Mar 21, 2024
Merged

GH-109653: Defer import of importlib.metadata._adapters#109829
jaraco merged 3 commits into
python:mainfrom
AA-Turner:importtime/importlib.metadata

Conversation

@AA-Turner

@AA-Turner AA-Turner commented Sep 25, 2023

Copy link
Copy Markdown
Member

For a ~14% speed up. However, this introduces inline imports into .metadata, which might be undesirable.

A

@jaraco

jaraco commented Sep 25, 2023

Copy link
Copy Markdown
Member

I notice that the description here only mentions importlib.metadata, but this change also affects importlib.resources. Both packages have _adapters modules.

Can you confirm how much of the speedup is achieved by each of the two changes? How is timing benchmarked (is it as simple as py -m timeit 'import typing')?

@AA-Turner

Copy link
Copy Markdown
Member Author

How is timing benchmarked

$ python -X importtime -Sc 'import importlib.metadata' 2> importlib_metadata.log
$ cat importlib_metadata.log
$ tuna importlib_metadata.log

The tuna viewer is optional, often I just read the text log.

A

@AA-Turner

Copy link
Copy Markdown
Member Author

Can you confirm how much of the speedup is achieved by each of the two changes?

I'll try and come back with some timings

A

@hugovk

hugovk commented Feb 29, 2024

Copy link
Copy Markdown
Member

Here's some timings on macOS (non-debug build with --enable-optimizations --with-lto), running this three times, timings give from third run so everything's been imported at least once:

./python.exe -X importtime -Sc 'import importlib.metadata' 2> importlib_metadata.log && tuna importlib_metadata.log

Baseline with main (86e5e06)

importlib.metadata: 0.048 s

Details image

Deferring in Lib/importlib/metadata/__init__.py

importlib.metadata: 0.040 s

Details image

Deferring in Lib/importlib/resources/_common.py

importlib.metadata: 0.037 s

Details image

Deferring in both Lib/importlib/metadata/__init__.py and Lib/importlib/resources/_common.py

importlib.metadata: 0.030 s

Details image

@jaraco jaraco left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks hugovk for providing the benchmarks for reference.

I do think we still need comments for these deferred imports to indicate why they're deferred. Without comments or something else to protect them, the optimizations could get normalized away.

@bedevere-app

bedevere-app Bot commented Mar 21, 2024

Copy link
Copy Markdown

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

@jaraco
jaraco enabled auto-merge (squash) March 21, 2024 03:08
@jaraco
jaraco merged commit 6547330 into python:main Mar 21, 2024
@jaraco

jaraco commented Mar 21, 2024

Copy link
Copy Markdown
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants