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

bpo-45020: Freeze some of the modules imported during startup. #28335

Merged

Conversation

ericsnowcurrently
Copy link
Member

@ericsnowcurrently ericsnowcurrently commented Sep 14, 2021

Doing this provides significant performance gains for runtime startup (~15% with all the imported modules frozen). We don't yet freeze all the imported modules because there are a few hiccups in the build systems we need to sort out first. (See bpo-45186 and bpo-45188.)

Note that in PR GH-28320 we added a command-line flag (-X frozen_modules=[on|off]) that allows users to opt out of (or into) using frozen modules. The default is still "off" but we will change it to "on" as soon as we can do it in a way that does not cause contributors pain.

FYI, most of this PR was already reviewed in #28107. Also, almost all the changes in this PR are generated code (not generated: Tools/scripts/freeze_modules.py and Lib/test/*.py).

Here are things we'll be doing in follow-up PRs:

  • always default to "on" if it's a PGO build, even if under development
  • freeze the remaining stdlib modules imported during startup (os, site, codecs, encodings.*)
  • default to "on" (except if actually running out of the source tree)
  • stop tracking the frozen module .h files in the repo
  • possibly freeze other modules (e.g. those imported for "python -m ...", AKA runpy)
  • possibly make frozen stdlib modules more like source modules, with __file__ and __path__ (see bpo-21736)

https://bugs.python.org/issue45020

@terryjreedy
Copy link
Member

Eric, I am going to do the test_query change separately, right now, so I can backport it and keep it in sync across versions.

Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

However, wait for Terry and his separate fix for idle_test/test_query.py (which currently has a failing test, I'm not sure why).

Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

I wonder if this would ease the pain of generating all those modules a bit?
It reduces the number of scrollback lines by 2/3rd.

diff --git a/Tools/scripts/freeze_modules.py b/Tools/scripts/freeze_modules.py
index 0c2b826280..f3d821529a 100644
--- a/Tools/scripts/freeze_modules.py
+++ b/Tools/scripts/freeze_modules.py
@@ -511,9 +511,8 @@ def regen_makefile(modules):
         # Note that we freeze the module to the target .h file
         # instead of going through an intermediate file like we used to.
         rules.append(f'{header}: Programs/_freeze_module {pyfile}')
-        rules.append(f'\t$(srcdir)/Programs/_freeze_module {src.frozenid} \\')
-        rules.append(f'\t\t$(srcdir)/{pyfile} \\')
-        rules.append(f'\t\t$(srcdir)/{header}')
+        rules.append(f'\t$(srcdir)/Programs/_freeze_module {src.frozenid} '
+                     f'$(srcdir)/{pyfile} $(srcdir)/{header}')
         rules.append('')
 
     frozenfiles[-1] = frozenfiles[-1].rstrip(" \\")

@terryjreedy
Copy link
Member

The reason there are failing test is that a) certain test files, such as asyncio and multiprocessing, are allowed to contain required randomly failing tests and b) when one of the github actions fail, all are rerun, giving bogus tests that passed on one run a chance to fail on the rerun. This is what happened to this time. I have now spent more time on these bogus failures than I did making the PR and I think this policy is rather rude. If there is a failure on a required test on the third run, either of you are welcome to hit the rerun button on the details page. Or merge if you can. I am off to sleep.

@gvanrossum
Copy link
Member

gvanrossum commented Sep 15, 2021 via email

@ericsnowcurrently
Copy link
Member Author

@terryjreedy, thanks for doing that.

(FTR, PR gh-28344)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants