-
-
Notifications
You must be signed in to change notification settings - Fork 31.9k
bpo-45850: Implement deep-freeze on Windows #29648
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
(But needs work, to avoid recompiling each time.)
The recipe is copied from how unchanged frozen files are treated.
@@ -347,6 +367,18 @@ | |||
<Message Text="Updated files: @(_Updated->'%(Filename)%(Extension)',', ')" | |||
Condition="'@(_Updated)' != ''" Importance="high" /> | |||
</Target> | |||
<Target Name="_RebuildDeepFrozen" AfterTargets="_RebuildFrozen" Condition="$(Configuration) != 'PGUpdate'"> | |||
<Exec Command='$(PythonForBuild) "$(PySourcePath)Tools\scripts\deepfreeze.py" "%(None.OutFile)" "-m" "%(None.ModName)" -o "%(None.IntFile)"' /> |
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.
Do we need a separate DeepIntFile?
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.
Probably, yeah. It'll be used for dependency analysis, and I think it's more likely to decide that it's always out of date than to incorrectly assume it's up to date, but it's safest to only have one task creating each file during a build.
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
@@ -347,6 +367,18 @@ | |||
<Message Text="Updated files: @(_Updated->'%(Filename)%(Extension)',', ')" | |||
Condition="'@(_Updated)' != ''" Importance="high" /> | |||
</Target> | |||
<Target Name="_RebuildDeepFrozen" AfterTargets="_RebuildFrozen" Condition="$(Configuration) != 'PGUpdate'"> | |||
<Exec Command='$(PythonForBuild) "$(PySourcePath)Tools\scripts\deepfreeze.py" "%(None.OutFile)" "-m" "%(None.ModName)" -o "%(None.IntFile)"' /> |
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.
Probably, yeah. It'll be used for dependency analysis, and I think it's more likely to decide that it's always out of date than to incorrectly assume it's up to date, but it's safest to only have one task creating each file during a build.
Co-authored-by: Steve Dower <[email protected]>
57463cb
to
b52ad63
Compare
@zooba Approve all you want, but the x86 build is still broken. I'll look into that. |
This fixes the x86 build
Also use a separate intermediate file for deepfreeze.
🤖 New build scheduled with the buildbot fleet by @gvanrossum for commit a7415ed 🤖 If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again. |
When the buildbots pass I intend to land this. |
Speedup: amd64 with PGO, from 32 msec to 30 msec (average of 100 runs each), so about 6-7% faster. Not as much as on macOS, alas. |
46a40cc
to
a3f60fc
Compare
Re: Windows Arm64 failure. I've upgraded host python to 3.10 so hopefully, that should solve the win/arm64 specific issue. Could someone retrigger the build please? |
The ARM64 and AMD64 buildbots now pass. I'm going to ignore the failing Windows (x86) test because it's in an obscure corner of asyncio; I assume that test suite is flaky. I will land after breakfast. |
@gvanrossum: Please replace |
Implement changes to build with deep-frozen modules on Windows. Note that we now require Python 3.10 as the "bootstrap" or "host" Python. This causes a modest startup speed (around 7%) on Windows.
Loose ends:
When you run make_freeze.py on Windows it makes os.path an alias of ntpath instead of posixpath(bpo-45272, bpo-45273)https://bugs.python.org/issue45850