-
-
Notifications
You must be signed in to change notification settings - Fork 31.7k
gh-132952: Import _io instead of io at startup #132957
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
Conversation
Like #132956 this only helps |
I'm going to run this branch on our benchmarking hardware (as well as #132956), just so we have some numbers. |
Benchmarking of this PR: https://github.com/faster-cpython/benchmarking-public/blob/main/results/bm-20250425-3.14.0a7%2B-dac50cc/bm-20250425-linux-x86_64-JelleZijlstra-sunder_io-3.14.0a7%2B-dac50cc-vs-base.svg This is actually more effective than (19%) than #132956 (12%). I think it makes sense to merge this, and maybe the other just for good measure (though it should have no additional effect). |
This is now ready. One tricky aspect is that we need to check any side effects that come from importing |
@@ -0,0 +1,2 @@ | |||
Speed up startup with the ``-S`` argument by about 19% by importing the |
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.
it is worth mentioning "vs what" when stating an improvement %.
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.
It's 19% vs. before this commit, but it's only 1.5% faster than 3.13.0 (which is probably what most users care about, since they don't run main
). I would argue 1.5% is in the noise, so we probably shouldn't have a what's new entry at all -- this is just counteracting a regression in 3.14 development.
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.
It's only a NEWS entry, not text in the What's New, so I'll remove the 19% claim but leave the NEWS entry since this could affect behavior some users care about.
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, but I think we should remove the whatsnew.
When you're done making the requested changes, leave the comment: |
I have made the requested changes; please review again |
python_startup_no_site
benchmark #132952