-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Speedup startup of Python by importing less #3009
Conversation
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.
Thanks Christian!
I've merged #3014. I'd like to rebase these changes on those in order to measure the changes. I'll work on that. |
e847559
to
c3ff362
Compare
I've rebased the commits and re-ordered the last two because I'm interested in seeing the performance difference due to cddc4e8. |
Annoyingly, I'm not seeing the performance tests run. I'll need to investigate why. |
``_distutils_hack`` is imported by a ``.pth`` file at every start of a Python interpreter. The import of costly modules like ``re`` and ``contextlib`` almost doubles the initial startup time of an interpreter. - replace ``contextlib`` with simple context manager and try/except - replace ``re`` with simple string match - move import of ``importlib`` into function body - remove ``warnings.filterwarnings()``, which imports ``re``, too. Fixes: pypa#3006 Signed-off-by: Christian Heimes <christian@python.org>
cddc4e8
to
7619852
Compare
Seems I hadn't done the rebase correctly. Oh, I see what I did. I rebased, then when re-ordering the commits, undid the rebase. Re-doing the rebase, I can confirm that |
For some reason, I'm still not seeing the exercises run. I see now the issue is that More importantly, though, I'm not confident the performance tests are reliable enough, as they're only showing performance gains in the 1ms range:
|
I'm not sure what performance you're trying to measure or in what environment. If you look at #3006 you'll see that we measured the overall time of |
The performance test as implemented attempts to measure the startup time of the interpreter, and to do it in a way that runs in continuous integration and thus is available to be measured and audited for future changes. Unfortunately, that test also necessarily includes the overhead of setting up the subprocess, which is likely adding noise to the measurement. I was able to use tiran's technique to verify some numbers on my machine as well:
It's clear from the output that there's a lot of jitter even in these runs (between 506µs and 1050µs). Still, that's noticeably better than the 2541µs without the improvements. I guess that also corroborates the numbers I was seeing from pytest-perf (~1-2 ms improvement on my macbook pro). I still don't have any good evidence that the last commit (2b07b8e) is valuable or a premature optimization. I'll examine that next. |
Sorry, I have no idea what pip-run does. Could it import some of the modules that the hack depends on, so they aren’t counted? In #3006 I measured ~10 msec, and I doubt your hardware is 4x as fast as mine. Agreed that the perf impact of that commit should be minimal. Personally I find it easier to follow though. |
I recommend manual inspection of the full output of -X importtime to see what’s what. |
Looking at that last commit (2b07b8e), I notice that it's only going to affect the performance when a module is imported, so I want to devise a different test to measure its performance:
Looks like the string formatting and method lookup and lambda call add ~1µs to each import. I suspect that's an acceptable cost, so I'd like to retain the existing implementation, which follows more of a functional paradigm and avoids repetition.
See pip-run for details and maybe give it a try. It's a nice way to on a single command run Python with specific requirements installed (in this case, specific versions of setuptools). I personally keep pip-run installed in my system python environments to rapidly test things like above.
Good idea. Here are the full comparisons (released version versus this PR), I see only a few ms difference:
But that's probably because Even when I create a pristine virtualenv with no pip, Python still encounters the warnings module on startup, consuming 26ms:
So that begins explains why the performance benefit of this change isn't as great on my machine as on others. The biggest benefit was from not importing warnings/re, but that's happening anyway, presumably due to something with the homebrew install of Python I happen to be using. I'm going to proceed to merge this change without 2b07b8e, and we can revisit that change if needed. |
2b07b8e
to
7619852
Compare
Releasing as 60.4.0 |
Thanks! Tomorrow we should check speed.python.org and see what this did for the python startup benchmark in the timeline. |
Looks great! Thanks for confirming.
|
_distutils_hack
is imported by a.pth
file at every start of aPython interpreter. The import of costly modules like
re
andcontextlib
almost doubles the initial startup time of aninterpreter.
contextlib
with simple context manager and try/exceptre
with simple string matchimportlib
into function bodywarnings.filterwarnings()
, which importsre
, too.Fixes: #3006
Signed-off-by: Christian Heimes christian@python.org
Summary of changes
Closes
Pull Request Checklist
changelog.d/
.(See documentation for details)