Skip to content

Defer tempfile import to sites of use#328

Open
Sachaa-Thanasius wants to merge 2 commits intopython:mainfrom
Sachaa-Thanasius:defer-tempfile-usage
Open

Defer tempfile import to sites of use#328
Sachaa-Thanasius wants to merge 2 commits intopython:mainfrom
Sachaa-Thanasius:defer-tempfile-usage

Conversation

@Sachaa-Thanasius
Copy link
Copy Markdown

@Sachaa-Thanasius Sachaa-Thanasius commented Apr 7, 2025 •
edited
Loading

Part of #326.

tempfile is a relatively heavy import that's only used in a few routines, and they aren't always called when using importlib_resources, especially in the common case of as_file() being passed a pathlib.Path object. Thus, I think it's worthwhile to defer importing it until it's actually used.

This PR effectively inlines the one function using tempfile at module-level scope to avoid an import-time dependency on tempfile. It then "inlines" the necessary import in the two functions that use it.

Based on local testing, doing so improves the upfront import time of importlib_resourcesby about ~30% on supported CPython versions and ~50% on PyPy3.10.

@Sachaa-Thanasius Sachaa-Thanasius changed the title Defer tempfile import to sites of use. Defer tempfile import to sites of use Apr 7, 2025
jaraco
jaraco reviewed Apr 12, 2026
View reviewed changes
importlib_resources/_common.py
Comment on lines +195 to +196
with tempfile.TemporaryDirectory() as temp_dir:
yield _write_contents(pathlib.Path(temp_dir), path)
Copy link
Copy Markdown
Member

@jaraco jaraco Apr 12, 2026

Choose a reason for hiding this comment

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

I don't care for this refactor. The reason _temp_path exists is to decouple the pathlib.Path construction from the invocation of the write function. Yes, it's logically the same, but without the presence of _temp_path the defect that tempfile doesn't provide pathlib objects is hidden. Let's keep _temp_path, especially as its presence isn't implicated in the purpose of this change.

jaraco
jaraco reviewed Apr 12, 2026
View reviewed changes
importlib_resources/_common.py
Comment on lines +191 to +192
# Deferred for performance.
import tempfile
Copy link
Copy Markdown
Member

@jaraco jaraco Apr 12, 2026

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 repeating ourselves. Now we have 6 lines of code where previous 1 was sufficient... and moreover, these lines of code are entangled (their comments both refer to the same motivation and should be kept in sync. Better would be to do:

Suggested change
# Deferred for performance.
import tempfile
tempfile = _import_tempfile()

And then in _import_tempfile, document the motivation, which should not only include the basic motivation ("deferred for performance"), but also details on what performance is saved. For example, it should link to the PR ("#328") and that PR should have examples of how much performance is saved.

@jaraco
Copy link
Copy Markdown
Member

jaraco commented Apr 12, 2026

Based on local testing, doing so improves the upfront import time of importlib_resourcesby about ~30% on supported CPython versions and ~50% on PyPy3.10.

Use python -X importtime for both before and after to demonstrate quantitative results. Please also provide basics about the machine on which it's run, as that could affect the outcome.

@jaraco
Copy link
Copy Markdown
Member

jaraco commented Apr 12, 2026 •
edited
Loading

When I run the tests on my Macbook Pro M3 and Python 3.14, I see a negligible benefit.

 importlib_resources main 🐚 .tox/py/bin/python -X importtime -c 'import importlib_resources' 2>1 | grep importlib_resources$
import time:       103 |      10231 | importlib_resources
 importlib_resources main 🐚 gh pr checkout 328
remote: Enumerating objects: 8, done.
remote: Counting objects: 100% (8/8), done.
remote: Compressing objects: 100% (2/2), done.
remote: Total 8 (delta 6), reused 8 (delta 6), pack-reused 0 (from 0)
Unpacking objects: 100% (8/8), 870 bytes | 124.00 KiB/s, done.
From https://github.com/python/importlib_resources
 * [new ref]               refs/pull/328/head -> defer-tempfile-usage
Switched to branch 'defer-tempfile-usage'
 importlib_resources defer-tempfile-usage 🐚 .tox/py/bin/python -X importtime -c 'import importlib_resources' 2>1 | grep importlib_resources$
import time:       109 |      10760 | importlib_resources

Actually, that looks like a detriment.

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

Reviewers

@jaraco jaraco jaraco left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Sachaa-Thanasius @jaraco