GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||||||||||||||||||||||||||
GeezerVison is showing github.com — gh-121423: Improve import time of `socket` by writing `socket.errorTab` as a constant and lazy import modules by Wulian233 · Pull Request #121424 · python/cpython Conversation
|
|
You are right, ↓ benchmark remove 1.14x faster |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
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 selectors
There was an error while loading. Please reload this page.
…nxrl4.rst Co-authored-by: Bénédikt Tran <[email protected]>
|
The benchmark only imports socket once. the other 999 imports are no-op as socket in in sys.modules. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
Am I right? I'm not familiar with benchmark, sorry I'm wrong |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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__" |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem 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.log
Before: 3ms
Current PR: 2ms
Also: 1ms
Moving the couple of import arrays into two functions that call it:
Sorry, something went wrong.
There was an error while loading. Please reload this page.
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 |
There was an error while loading. Please reload this page.
There was a problem 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:
global selectors if selectors is None: import selectors
You can define the selectors variable to None at the top of the file with a comment:
# module imported lazily selectors = None
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem 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?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem 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?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem 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.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem 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.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Can you also make the array module import lazy in this PR? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
I measured that the change saves 0.7 ms on timeit: hyperfine: |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Thank you! I just replaced the incorrect benchmark results in the PR with the correct ones |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
@Wulian233 Please could you do this as well? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
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 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem 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?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
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.
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.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
What do you think of this👀 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
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. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
socket by writing socket.errorTab as a constant and lazy import of selectorssocket by writing socket.errorTab as a constant and lazy import of selectors
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 :) |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
socket by writing socket.errorTab as a constant and lazy import of selectorssocket by writing socket.errorTab as a constant and lazy import modules
There was a problem 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 :)
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
#121423 is the right issue.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
socket by writing socket.errorTab as a constant and lazy import modulessocket by writing socket.errorTab as a constant and lazy import modules
|
Merged. Thanks for your nice enhancement @Wulian233. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.