Skip to content
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 supported tags in resolver factory #12755

Merged
merged 1 commit into from
Jun 27, 2024

Conversation

ichard26
Copy link
Member

@ichard26 ichard26 commented Jun 8, 2024

I've observed get_supported() consume up to 10% of the installation step when installing many packages. Each call can take 1-5ms (presumably only on Linux due to the large number of supported tags) and there is one call for every lookup in the cache. As all of these calls are made with no arguments, the tags can be trivially queried in advance during factory initialization.

Example profile

python -m cProfile -o profile.pstats -m pip install --only-binary=:all: --no-index --ignore-installed --no-deps --find-links homeassistant/deps/ -r homeassistant/requirements.txt (from https://github.com/home-assistant/core/blob/2024.6.0/requirements.txt)

get_supported() is left of install_given_reqs() which is coloured green.

pstats

Towards #12712.

@ichard26 ichard26 added the type: performance Commands take too long to run label Jun 8, 2024
Copy link
Member

@uranusjr uranusjr left a comment

Choose a reason for hiding this comment

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

Seems like a quick win.

@morotti
Copy link
Contributor

morotti commented Jun 24, 2024

I really hope this patch get merged soon. ❤️ ❤️ ❤️

I am getting ~17% faster pip install ... on my machine and my venvs.
The tag calculations take as long as it takes to download and resolve all packages!

@ichard26 ichard26 added this to the 24.2 milestone Jun 24, 2024
I've observed get_supported() consume up to 10% of the installation step
when installing many packages. Each call can take 1-5ms (presumably only
on Linux due to the large number of supported tags) and there is one
call for every lookup in the cache. As all of these calls are made with
no arguments, the tags can be trivially queried in advance during
factory initialization.
@ichard26 ichard26 force-pushed the cache-supported-tags branch from 24e23bd to 47e82e2 Compare June 27, 2024 15:26
@ichard26
Copy link
Member Author

Rebased on main and updated NEWS entry to more accurately reflect that this improves dependency resolution performance, not exclusively install performance. If CI stays 🍏, I'd like to see this merged.

@pfmoore pfmoore merged commit 47ffcdd into pypa:main Jun 27, 2024
28 checks passed
@pfmoore
Copy link
Member

pfmoore commented Jun 27, 2024

Nice improvement, thanks!

@ichard26 ichard26 deleted the cache-supported-tags branch June 27, 2024 16:13
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants