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

Windows: compile xz with CMake #6947

Merged
merged 8 commits into from
Mar 29, 2023
Merged

Conversation

nulano
Copy link
Contributor

@nulano nulano commented Feb 13, 2023

Changes proposed in this pull request:

  • Compile liblzma with CMake instead of VS project file, for Updated xz to 5.4.1 #6883 (comment):

    Updated windows/INSTALL-MSVC.txt to mention that CMake-based build is now the preferred method with Visual Studio. The project files will probably be removed after 5.4.x releases.

    This might also help with Failed to build Windows ARM64 dependencies #6679 (comment)

  • Use Ninja instead of NMake to build dependencies by default. Unlike NMake, Ninja can build several files in parallel. This makes very little difference on GHA which only provides 2 CPU cores, but is almost 2x faster on my computer.

  • Rewrite argument parsing in winbuild\build_prepare.py using argparse for better maintainability.

winbuild/build_prepare.py Outdated Show resolved Hide resolved
winbuild/build_prepare.py Outdated Show resolved Hide resolved
winbuild/build_prepare.py Outdated Show resolved Hide resolved
winbuild/build_prepare.py Outdated Show resolved Hide resolved
@hugovk
Copy link
Member

hugovk commented Feb 13, 2023

  • Use Ninja instead of NMake to build dependencies by default. Unlike NMake, Ninja can build several files in parallel. This makes very little difference on GHA which only provides 2 CPU cores, but is much faster on my computer.

Sounds good! What sort of speedup do you see?

@nulano
Copy link
Contributor Author

nulano commented Feb 13, 2023

Sounds good! What sort of speedup do you see?

Almost 2x on my 6 core 12 thread CPU.

With --nmake:

Days              : 0
Hours             : 0
Minutes           : 6
Seconds           : 44
Milliseconds      : 37
Ticks             : 4040372902
TotalDays         : 0.00467635752546296
TotalHours        : 0.112232580611111
TotalMinutes      : 6.73395483666667
TotalSeconds      : 404.0372902
TotalMilliseconds : 404037.2902

Without --nmake (i.e. with Ninja):

Days              : 0
Hours             : 0
Minutes           : 3
Seconds           : 39
Milliseconds      : 489
Ticks             : 2194899729
TotalDays         : 0.00254039320486111
TotalHours        : 0.0609694369166667
TotalMinutes      : 3.658166215
TotalSeconds      : 219.4899729
TotalMilliseconds : 219489.9729

nulano and others added 2 commits February 13, 2023 15:26
Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>
".",
]
),
f"{{cmake}} --build . --clean-first --parallel --target {target}",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I originally updated the documentation in this PR to require CMake 3.13+ as that is what xz's CMake files specify.

I missed that the cmake --build command requires CMake 3.15+ when there are multiple targets, I've now updated the PR.

The previous CMake requirement was 3.12, which is what is included with VS2017 and was the oldest I tested when writing the documentation in #4495.

VS2019 includes CMake 3.20, but I've found that 3.15 is sufficient for this PR.

@hugovk hugovk merged commit e945437 into python-pillow:main Mar 29, 2023
@nulano nulano deleted the winbuild-ninja branch March 29, 2023 16:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants