Repository navigation
Conversation
|
As mentioned in #1215 before considering adding these targets I would like to see benchmarks which should that there is a benefit to building this microarchitecture specific distributions. Additional the change to bookworm likely raises the minimum glibc version required to consume these distributions from 2.24 (Debian 9 Stretch) to 2.36 (Debian 12 Bookworm). A better approach that is more maintainable in the long term to to use the LLVM toolchain to cross-compile these targets using a sysroot. This was mentioned in #1215. |
a61a7be to
cbcc593
Compare
Hardware validation + apples-to-apples benchmark — Python 3.13.15 on real P9/P10/P11 hardwareAll three binaries were cross-compiled from x86 (commit cbcc593) and tested on RHEL 9.8 machines inside Smoke tests — ALL PASSEDAll three binaries passed: Python version check, Performance benchmark — generic vs tuned, same machineTo isolate the tuning benefit from hardware differences, the same generic Workload: Pi to 10,000 digits via Python's POWER10
→ 38.7% faster on the same P10 machine with POWER11
→ 38.3% faster on the same P11 machine with glibc requirementPower9 requires glibc ≥ 2.17 (same as the generic baseline). Power10 and Power11 require glibc ≥ 2.36 due to the Bookworm build base. This is enforced in |
| # Stretch stopped publishing snapshots in April 2023. Last snapshot | ||
| # is 20230423T032533Z. But there are package authentication issues | ||
| # with this snapshot. | ||
| RUN for s in debian_stretch debian_stretch-updates debian-security_stretch/updates; do \ | ||
| echo "deb http://snapshot.debian.org/archive/${s%_*}/20221105T150728Z/ ${s#*_} main"; \ | ||
| done > /etc/apt/sources.list && \ |
There was a problem hiding this comment.
The snapshot release should not be removed. We want reproducible images when possible
|
A good amount of work is done to keep the x86-64 microarchitecture variants based on the same glibc version. The same should be done here. There is limited functionality in uv to select a build based on the host glibc version so supporting older versions is important. I would appreciate more details benchmarks, at minimum using fastmark but if possible pyperformance. These would give a more complete picture. The benchmarks should be based on the same compiler version as these difference can be substantial. If upgrading the compiler (and glibc version) give the bulk of the performance gain there is no reason to expand the build matrix. Is there a reason why cross-compiling using LLVM was not been tried discussed in #1215 and my earlier comment? |
| @@ -0,0 +1,54 @@ | |||
| # Debian Bookworm. | |||
| FROM docker.io/library/debian@sha256:6bc30d909583f38600edd6609e29eb3fb284ab8affce8d0389f332fc91c2dd91 | |||
| LABEL org.opencontainers.image.authors="Gregory Szorc <gregory.szorc@gmail.com>" | |||
There was a problem hiding this comment.
This would need to have a different author
|
@veenious sorry I missed that author fix when I sent over my initial suggestions. lmk if you want me to re-push anything or if your'e fine with fixing my commits. @jjhelmus thanks for the reviews again. for the sake of this PR -- we'd assumed that since the project hadn't gone that route yet we could add these in the way they're already built and continue to discuss in #1100 . No other reason. |
cbcc593 to
739a932
Compare
|
Hi @jjhelmus — updated the PR to address all review feedback. Summary below. Changes from previous revisionApproach: LLVM 23 + Debian Stretch sysroot (cross-compiled on x86) The LLVM cross-compilation approach in this PR follows #1259 (riscv64 LLVM PR) and #1241 (aarch64 Stretch sysroot PR) as the reference patterns.
Shell script changes — each one fixed a specific bug required for LLVM cross-compilation:
All changes are guarded to clang or ppc64le targets and do not affect any existing x86/aarch64/musl builds. glibcAll three new variants now match the existing
pyperformance benchmarksRan pyperformance 1.14.0 (
Selected results:
The gains are from POWER10/11 ISA features (MMA, new vector/crypto instructions) enabled by |
|
Thanks for the update. I'm out this week but will review this when I am back. |
|
Hi @jjhelmus — happy to switch to the astral-sh/llvm-build tarball. We checked the existing |
|
Hi @jjhelmus |
739a932 to
d4582c3
Compare
|
hi @jjhelmus ApproachAll four ppc64le targets (these three and the existing generic one) are cross-compiled from x86_64 with the packaged LLVM bundle (astral-sh/llvm-build, clang + lld) and a Debian Stretch ppc64el sysroot, following #1334. glibcAll variants keep the same floor as generic ppc64le and the x86_64 variants: glibc 2.17. validation.rs enforces it, and the highest symbol version in the built binaries is GLIBC_2.17. TestingPython 3.12.15, 3.13.17 and 3.14.9 (noopt) were built for power9, power10 and power11. Each of the 9 builds:
|
d4582c3 to
5ee6df0
Compare
|
Update: Upstream merged #1349, which adds LLVM cross-compilation support for ppc64le. Since this happened while the PR was under review, I rebased onto it (head 5ee6df0) and simplified the changes. Please ignore the earlier comments about a separate image and The three variants now reuse the upstream build.cross image and generic ppc64le settings, adding only -mcpu=pwr9/10/11. No new Dockerfile is needed. I also removed the libgcc_s.so.1 allowlist, since upstream's -static-libgcc handles it, and dropped the Verify-Peer=false workaround. The glibc minimum remains unchanged at 2.17. I've rebuilt and tested this commit with Python 3.12.15, 3.13.17, and 3.14.9 for POWER9, POWER10, and POWER11. All nine builds pass validate-distribution and successfully import decimal, ssl, sqlite3, zlib, bz2, lzma, and ctypes on the corresponding hardware. @jjhelmus Could you please re-review the updated PR when you get a chance? Thanks! |
|
Thanks for rebasing. I'm looking forward to the review. Thanks in advance Paul |
| # The cross-build helper reads sysconfig from the unmodified build tree. | ||
| # Use the cleaned installed values for PYTHON.json as well. | ||
| lib_suffix = "t" if os.environ.get("CPYTHON_FREETHREADED") else "" | ||
| sysconfig_data = os.path.join( | ||
| os.environ["ROOT"], "out", "python", "install", "lib", | ||
| "python%s%s" % (sysconfig.get_python_version(), lib_suffix), | ||
| sysconfig._get_sysconfigdata_name() + ".py", | ||
| ) | ||
| installed_config_vars = runpy.run_path(sysconfig_data)["build_time_vars"] |
There was a problem hiding this comment.
This is independent from adding new targets and should be done in a separate PR. This would improve all cross-builds.
| # Libtool's shared-library link filter drops these Clang cross-compilation | ||
| # flags, which would otherwise select host startup objects and runtimes. | ||
| sed -i 's/|--sysroot=\*|/|--sysroot=*|--target=*|--gcc-install-dir=*|/g' libtool | ||
|
|
There was a problem hiding this comment.
This is no longer needed given the embedding of these flags into the LIBTOOL_CC environment variable.
|
|
||
| # Libtool's shared-library link filter drops these Clang cross-compilation | ||
| # flags, which would otherwise select host startup objects and runtimes. | ||
| sed -i 's/|--sysroot=\*|/|--sysroot=*|--target=*|--gcc-install-dir=*|/g' libtool |
|
|
||
| # Libtool's shared-library link filter drops these Clang cross-compilation | ||
| # flags, which would otherwise select host startup objects and runtimes. | ||
| sed -i 's/|--sysroot=\*|/|--sysroot=*|--target=*|--gcc-install-dir=*|/g' libtool |
| ``ppc64le-unknown-linux-gnu`` | ||
| Linux on 64-bit little-endian PowerPC (ELFv2 ABI baseline). | ||
|
|
||
| ``ppc64le_power9-unknown-linux-gnu`` | ||
| Linux on 64-bit little-endian PowerPC optimized for POWER9. | ||
|
|
||
| Compiled with ``-mcpu=power9 -mtune=power9``. | ||
|
|
||
| ``ppc64le_power10-unknown-linux-gnu`` | ||
| Linux on 64-bit little-endian PowerPC optimized for POWER10. | ||
|
|
||
| Compiled with ``-mcpu=power10 -mtune=power10``. | ||
|
|
||
| ``ppc64le_power11-unknown-linux-gnu`` | ||
| Linux on 64-bit little-endian PowerPC optimized for POWER11. | ||
|
|
||
| Compiled with ``-mcpu=power11 -mtune=power11``. | ||
|
|
There was a problem hiding this comment.
These should be in the markdown file. The rst file only drive the older Sphinx documentation.
| }, | ||
| "libX11": { | ||
| "url": "https://www.x.org/releases/individual/lib/libX11-1.8.13.tar.gz", | ||
| "url": "https://xorg.freedesktop.org/archive/individual/lib/libX11-1.8.13.tar.gz", |
There was a problem hiding this comment.
These should not be changed.
|
I think adding these new CI targets will exceed the number of Linux builds in the second shard of the matrix. This will need to be expanded to support the additional jobs. |
Adds three ppc64le microarchitecture variant targets, mirroring the existing x86_64_v2/v3/v4 pattern: - ppc64le_power9-unknown-linux-gnu (-mcpu=pwr9 -mtune=pwr9) - ppc64le_power10-unknown-linux-gnu (-mcpu=pwr10 -mtune=pwr10) - ppc64le_power11-unknown-linux-gnu (-mcpu=pwr11 -mtune=pwr11) The variants reuse the generic ppc64le LLVM + Stretch sysroot build (build.cross image) and differ only in -mcpu/-mtune. glibc floor stays 2.17. Changes: - cpython-unix/targets.yml: add the three variant targets - ci-targets.yaml: add CI entries (3.11 is omitted for power10/power11) - cpython-unix/build.py: normalise variant triples for autoconf - cpython-unix/build-expat.sh: --with-dev-urandom for ppc64le - cpython-unix/extension-modules.yml: _decimal CONFIG_64 pattern matches the variant triples - docs/running.md: document the variant targets - src/release.rs, src/validation.rs: register the variant triples - .github/workflows/linux.yml: add a third build shard (build-2). The new targets add 84 jobs, taking the Linux push matrix from 490 to 574, which exceeds the 2 x 256 limit of the existing shards. Closes astral-sh#1215
5ee6df0 to
82a7e38
Compare
|
Thanks for the review, @jjhelmus. I've rebased and addressed review comments. Each point is addressed below:
|
Adds three ppc64le microarchitecture variant targets, mirroring the
existing x86_64_v2/v3/v4 pattern:
The variants reuse the generic ppc64le LLVM + Stretch sysroot build
(build.cross image) and differ only in -mcpu. glibc floor stays 2.17.
Changes:
PYTHON.json
Clang cross flags in libtool
the variant triples
Closes #1215