Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34k
gh-121423: Improve import time of socket by writing socket.errorTab as a constant and lazy import modules#121424
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
Wulian233 commented Jul 6, 2024 • edited by bedevere-app bot
Loading Uh oh!
There was an error while loading. Please reload this page.
edited by bedevere-app bot
Uh oh!
There was an error while loading. Please reload this page.
socket.errotTab and lazy import selectorssocket.errorTab and lazy import selectors
sobolevn left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think that your benchmark is correct. Because you put import socket into your setup part. So, the import time itself is not counted.
Uh oh!
There was an error while loading. Please reload this page.
Wulian233 commented Jul 6, 2024 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
You are right, ↓ benchmark remove 1.14x faster |
Misc/NEWS.d/next/Library/2024-07-06-12-37-10.gh-issue-121423.vnxrl4.rst Outdated Show resolvedHide resolved
Uh oh!
There was an error while loading. Please reload this page.
…nxrl4.rst Co-authored-by: Pieter Eendebak <[email protected]>
socket.errorTab and lazy import selectorssocket by writing socket.errorTab as a constant and lazy import of selectorsMisc/NEWS.d/next/Library/2024-07-06-12-37-10.gh-issue-121423.vnxrl4.rst Outdated Show resolvedHide resolved
Uh oh!
There was an error while loading. Please reload this page.
…nxrl4.rst Co-authored-by: Bénédikt Tran <[email protected]>
barry-scott commented Aug 31, 2024
The benchmark only imports socket once. the other 999 imports are no-op as socket in in sys.modules. |
Wulian233 commented Aug 31, 2024 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
Am I right? I'm not familiar with benchmark, sorry I'm wrong |
effigies commented Aug 31, 2024
You can use hyperfine to measure the entire Python process, in which case using a ❯ hyperfine -w 16 -u microsecond 'python -c pass'Benchmark 1: python -c pass Time (mean ± σ): 10921.2 µs ± 844.5 µs [User: 8745.1 µs, System: 2091.4 µs] Range (min … max): 9720.3 µs … 15591.1 µs 242 runs ❯ hyperfine -w 16 -u microsecond 'python -c "import socket; socket.__all__"'Benchmark 1: python -c "import socket; socket.__all__" Time (mean ± σ): 14554.3 µs ± 1154.4 µs [User: 12061.3 µs, System: 2368.5 µs] Range (min … max): 12826.1 µs … 20450.0 µs 206 runs |
hugovk commented Aug 31, 2024
Using a PGO+LTO build on macOS, so this will just be measuring the ❯ hyperfine --warmup 16 \ --prepare "git checkout socket"'./python.exe -c "import socket; socket.__all__"' \ --prepare "git checkout 6239d41527d5977aa5d44e4b894d719bc045860e"'./python.exe -c "import socket; socket.__all__"' Benchmark 1: ./python.exe -c "import socket; socket.__all__" Time (mean ± σ): 16.2 ms ± 0.8 ms [User: 13.1 ms, System: 2.5 ms] Range (min … max): 15.5 ms … 21.7 ms 91 runs Warning: Statistical outliers were detected. Consider re-running this benchmark on a quiet system without any interferences from other programs. Benchmark 2: ./python.exe -c "import socket; socket.__all__" Time (mean ± σ): 17.3 ms ± 1.0 ms [User: 13.9 ms, System: 2.7 ms] Range (min … max): 16.6 ms … 25.1 ms 88 runs Warning: Statistical outliers were detected. Consider re-running this benchmark on a quiet system without any interferences from other programs. Summary ./python.exe -c "import socket; socket.__all__" ran 1.06 ± 0.08 times faster than ./python.exe -c "import socket; socket.__all__" |
hugovk left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By the way, we have a general purpose issue for import time improvements -- #118761 -- shall we use that as an umbrella issue for this PR as well?
Using tuna to visualise import times (again, on macOS):
./python.exe -X importtime -c "import socket"2> import.log && tuna import.logBefore: 3ms

Current PR: 2ms

Also: 1ms
Moving the couple of import arrays into two functions that call it:

Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Hugo van Kemenade <[email protected]>
| def _sendfile_use_sendfile(self, file, offset=0, count=None): | ||
| # Lazy import to improve module import time | ||
| import selectors |
vstinnerSep 2, 2024 • edited by hugovk
Loading Uh oh!
There was an error while loading. Please reload this page.
edited by hugovk
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest to use a global variable to avoid the import at each call:
globalselectorsifselectorsisNone: importselectorsYou can define the selectors variable to None at the top of the file with a comment:
# module imported lazilyselectors=NoneThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I checked several recent PRs making module loading lazy, but most of them do not use the pattern with a global variable (I suspect for readability reasons).
@vstinner Are there any other reasons besides the small performance improvement for using the global variable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@serhiy-storchaka: Do you think that it's still useful in 2024 to use a global variable to avoid import selectors at each function call?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd just do the import. It's a mere dict lookup without a conditional when the import has already happened.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just measure. It is more than a mere dict lookup (we also need to check that the module is not partially initialized, this adds 2 more dict lookups or like).
In this case, I think that the difference may be small even in comparison with a single os.fstat() call. The idiom proposed by @vstinner may be used when the whole function is very fast.
vstinner commented Sep 2, 2024
Can you also make the array module import lazy in this PR? |
vstinner commented Sep 2, 2024
I measured that the change saves 0.7 ms on timeit: hyperfine: |
Wulian233 commented Sep 2, 2024
Thank you! I just replaced the incorrect benchmark results in the PR with the correct ones |
hugovk commented Sep 2, 2024
@Wulian233 Please could you do this as well? |
Wulian233 commented Sep 3, 2024 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
Of course! I just lazy imported array and haven't done full benchmarking yet. I also mentioned this optimization in 3.14.rst, do you think it should be included? @hugovk |
vstinner left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
What's New entry:
which results in a 30% speed up in standard pyperformance benchmarks.
This sentence can be misunderstood as "everything is 30% faster".
Which pyperformance benchmark is now faster?
Uh oh!
There was an error while loading. Please reload this page.
| def _sendfile_use_sendfile(self, file, offset=0, count=None): | ||
| # Lazy import to improve module import time | ||
| import selectors |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just measure. It is more than a mere dict lookup (we also need to check that the module is not partially initialized, this adds 2 more dict lookups or like).
In this case, I think that the difference may be small even in comparison with a single os.fstat() call. The idiom proposed by @vstinner may be used when the whole function is very fast.
Wulian233 commented Sep 3, 2024
What do you think of this👀 |
hugovk commented Sep 3, 2024 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
For What's New, we can follow the example @AlexWaygood wrote for 3.13, which grouped a few import improvements together:
Let's do the same with #118761 in 3.14. We have two under that issue so far. I recommend we also group this PR under #118761 as well -> rename this PR title So we can follow something like that now, or leave it out for now and add a grouped summary later. |
serhiy-storchaka commented Sep 3, 2024
Omit details that are not interested to the end user. I would also remove any mention from Ehat's New -- this is an insignificant change. I am sure that if you measure import time of different modules, you will find larger changes between versions (maybe even between bugfix releases) without anybody noticing. |
socket by writing socket.errorTab as a constant and lazy import of selectorssocket by writing socket.errorTab as a constant and lazy import of selectorsWulian233 commented Sep 3, 2024 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
Okay, now this pr belongs to 118761. I revert the changes to 3.14 so we can write them together future when there are more modules optimizations :) |
socket by writing socket.errorTab as a constant and lazy import of selectorssocket by writing socket.errorTab as a constant and lazy import modulesThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please rename to Misc/NEWS.d/next/Library/2024-07-06-12-37-10.gh-issue-118761.vnxrl4.rst (containing gh-issue-118761) and we're ready to merge :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#121423 is the right issue.
serhiy-storchaka left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
socket by writing socket.errorTab as a constant and lazy import modulessocket by writing socket.errorTab as a constant and lazy import modulesvstinner commented Sep 4, 2024
Merged. Thanks for your nice enhancement @Wulian233. |
hyperfine:
≈ 30% faster
socket.errorTaband lazy importselectors#121423