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

Speed-up lazy heapq import in collections #127538

Merged
merged 1 commit into from
Dec 3, 2024

Conversation

rhettinger
Copy link
Contributor

It is faster to test for None than to go through the import logic to check sys.modules.

Baseline:

$ ./python.exe -m timeit -s 'from collections import Counter' -s 'c=Counter("abc")' 'c.most_common(1)'
500000 loops, best of 5: 481 nsec per loop

Patched:

$ ./python.exe -m timeit -s 'from collections import Counter' -s 'c=Counter("abc")' 'c.most_common(1)'
500000 loops, best of 5: 400 nsec per loop

Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

LGTM.

I hope that in future such optimization will not be needed. The lookup in sys.modules has the same cost as the lookup in globals(), but there is currently additional cost of checking that the module is not partially imported -- this is two additional attribute lookups (__spec__ and other). I am planning to optimize them out.

@Wulian233
Copy link
Contributor

There have been some similar optimizations before, and it is suggested to start with gh-110761

#118761

@rhettinger rhettinger merged commit dffb909 into python:main Dec 3, 2024
48 checks passed
srinivasreddy pushed a commit to srinivasreddy/cpython that referenced this pull request Jan 8, 2025
ebonnal pushed a commit to ebonnal/cpython that referenced this pull request Jan 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Performance or resource usage skip issue skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants