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 for wheels. #3805

Merged

Conversation

mauritsvanrees
Copy link
Contributor

Summary of changes

In wheel.py:Wheel.is_compatible we called sys_tags each time to get the list of supported tags. Now we call this once, and cache the result in a simple variable. This drastically speeds up checking the compatibility of hundreds of wheels.

Closes #3804

Pull Request Checklist

@abravalheri
Copy link
Contributor

Hi @mauritsvanrees, thank you for the contribution and sorry for delay in reviewing...

I noticed that you used a global var for the implementation and some patching/mocking/stubbing capabilities for the tests.

What do you think about functools.lru_cache(maxsize=None)? (I think functools.cache would be better, but it is not available before 3.9 - and by looking at the implementation they are equivalent).

It seems also more elegant for testing because you can call clear_cache and there seems to be some evidence that it is faster because it is written in C.


Small note:

I don't know if setuptools/wheel.py is going to be maintained for long after we completely remove install/easy_install, but I understand that this is going to be a different discussion, which should involve the Buildout team that heavily uses it.

Since I could not find any references in the docs about it being deprecated, my assumption is that it is OK to add non-disruptive improvements.

@mauritsvanrees mauritsvanrees force-pushed the maurits-wheel-cache-supported-tags branch from c82d58b to 8af6bd4 Compare March 7, 2023 21:49
@mauritsvanrees
Copy link
Contributor Author

Hi @abravalheri. Thanks for the review and sorry for the delay in acting on it.

What do you think about functools.lru_cache(maxsize=None)?

Good point. I should probably use that function more often.
I have added a commit to do this. (And rebased on main and force pushed.)

This change has no effect on the timing of my small test script in issue #3804: the significant performance increase remains.

@abravalheri abravalheri merged commit 8961851 into pypa:main Mar 8, 2023
abravalheri added a commit that referenced this pull request Mar 8, 2023
@mauritsvanrees mauritsvanrees deleted the maurits-wheel-cache-supported-tags branch March 8, 2023 10:49
@mauritsvanrees
Copy link
Contributor Author

Thanks for your support @abravalheri!

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.

[FR] Cache supported tags in Wheel.is_compatible
2 participants