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-108716: Turn off deep-freezing of modules.#108722
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
markshannon commented Aug 31, 2023 • edited by bedevere-bot
Loading Uh oh!
There was an error while loading. Please reload this page.
edited by bedevere-bot
Uh oh!
There was an error while loading. Please reload this page.
brandtbucher commented Aug 31, 2023
We may want to benchmark this on the other machines too, but in general I'm excited by this! |
markshannon commented Sep 1, 2023
vstinner commented Sep 6, 2023
FYI I merged my PR #108741 to fix a race condition in |
vstinner commented Sep 6, 2023
Oh right, for the comparison to the main branch, I see: Windows:
macOS:
To be honest, it's surprising that disabling an optimization supposed to make Python startup actually... has no effect (or might be faster, but that can be noise of the benchmark?). |
vstinner commented Sep 6, 2023
What I can say is that it has a nice impact on build performance! :-) Comparison main => this PR:
Python configured with |
brandtbucher 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.
Looks good to me!
vstinner commented Sep 8, 2023
It's unclear to me if |
markshannon commented Sep 8, 2023
|
vstinner commented Sep 11, 2023
It seems like the Windows build system was not updated. This rule in <TargetName="_RebuildDeepFrozen"AfterTargets="_RebuildFrozen"DependsOnTargets="FindPythonForBuild"Condition="$(Configuration) != 'PGUpdate'"> <!-- BEGIN deepfreeze rule --> <ExecCommand='$(PythonForBuild) "$(PySourcePath)Tools\build\deepfreeze.py" ^ "$(PySourcePath)Python\frozen_modules\importlib._bootstrap.h:importlib._bootstrap" ^ "$(PySourcePath)Python\frozen_modules\importlib._bootstrap_external.h:importlib._bootstrap_external" ^ "$(PySourcePath)Python\frozen_modules\zipimport.h:zipimport" ^ "$(PySourcePath)Python\frozen_modules\abc.h:abc" ^ "$(PySourcePath)Python\frozen_modules\codecs.h:codecs" ^ "$(PySourcePath)Python\frozen_modules\io.h:io" ^ "$(PySourcePath)Python\frozen_modules\_collections_abc.h:_collections_abc" ^ "$(PySourcePath)Python\frozen_modules\_sitebuiltins.h:_sitebuiltins" ^ "$(PySourcePath)Python\frozen_modules\genericpath.h:genericpath" ^ "$(PySourcePath)Python\frozen_modules\ntpath.h:ntpath" ^ "$(PySourcePath)Python\frozen_modules\posixpath.h:posixpath" ^ "$(PySourcePath)Python\frozen_modules\os.h:os" ^ "$(PySourcePath)Python\frozen_modules\site.h:site" ^ "$(PySourcePath)Python\frozen_modules\stat.h:stat" ^ "$(PySourcePath)Python\frozen_modules\importlib.util.h:importlib.util" ^ "$(PySourcePath)Python\frozen_modules\importlib.machinery.h:importlib.machinery" ^ "$(PySourcePath)Python\frozen_modules\runpy.h:runpy" ^ "$(PySourcePath)Python\frozen_modules\__hello__.h:__hello__" ^ "$(PySourcePath)Python\frozen_modules\__phello__.h:__phello__" ^ "$(PySourcePath)Python\frozen_modules\__phello__.ham.h:__phello__.ham" ^ "$(PySourcePath)Python\frozen_modules\__phello__.ham.eggs.h:__phello__.ham.eggs" ^ "$(PySourcePath)Python\frozen_modules\__phello__.spam.h:__phello__.spam" ^ "$(PySourcePath)Python\frozen_modules\frozen_only.h:frozen_only" ^ "-o" "$(PySourcePath)Python\deepfreeze\deepfreeze.c"'/> <!-- END deepfreeze rule --> </Target>If you want to keep the infra for now, maybe this rule should just be commented with a reference to |
gvanrossum commented Mar 16, 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.
This PR probably should also have updated Python/deepfreeze/README.txt to indicate that it is (currently) no longer used. It should also have updated various sections in Makefile.pre.in (including some comments) that still reference deepfreeze -- those are just distractions at this point (until we decide to actually deep-freeze some other objects, which doesn't look like it's going to happen for 3.13 at least). EDIT: See gh-116919 |
Performance impact is neutral, maybe a little positive, but noisy
Startup might be a bit slower: no-site is 2% faster, but normal startup 5% slower.
However, without deepfreeze it is much easier to implement things like: faster-cpython/ideas#566 or #99555 which should more than compensate.