Skip to content

Conversation

Kobzol
Copy link
Member

@Kobzol Kobzol commented Apr 14, 2025

Use a Docker container with glibc 2.28 instead of cargo zigbuild, because Zig doesn't support PGO.

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 14, 2025
@Kobzol Kobzol force-pushed the manylinux-instead-of-zigbuild branch 9 times, most recently from 401559a to 59cd04f Compare April 15, 2025 12:15
@Kobzol Kobzol force-pushed the manylinux-instead-of-zigbuild branch from 59cd04f to 292ff49 Compare April 15, 2025 12:23
@lnicola
Copy link
Member

lnicola commented Apr 15, 2025

Does pypa/manylinux_2_28_x86_64 even bring us anything besides almalinux:8? EDIT: I see a newer GCC.

@Kobzol
Copy link
Member Author

Kobzol commented Apr 15, 2025

It's a just a placeholder image that I use on some other crate to get glibc 2.28. We could use a different image for sure :)

@Kobzol Kobzol force-pushed the manylinux-instead-of-zigbuild branch 6 times, most recently from 21d8ca6 to 79338d0 Compare April 15, 2025 12:40
@Kobzol Kobzol changed the title [WIP] Use a Docker container instead of Zig for building with old(er) glibc on Linux Use a Docker container instead of Zig for building with old(er) glibc on x64 Linux Apr 15, 2025
@Kobzol Kobzol force-pushed the manylinux-instead-of-zigbuild branch from 79338d0 to bfe01bd Compare April 15, 2025 13:08
@Kobzol
Copy link
Member Author

Kobzol commented Apr 15, 2025

Great, PGO works (with 15% perf. win), and I checked that at most 2.28 GLIBC symbols are being used (https://github.com/rust-lang/rust-analyzer/actions/runs/14469656707?pr=19586):

objdump -T rust-analyzer-pgo/rust-analyzer-x86_64-unknown-linux-gnu | grep GLIBC | sed 's/.*GLIBC_\([.0-9]*\).*/\1/g' | sort -Vu

I didn't yet enable PGO in this PR, I want to do that separately.

r? @lnicola

@lnicola
Copy link
Member

lnicola commented Apr 15, 2025

We could also try quay.io/pypa/manylinux_2_34_aarch64? But it doesn't have to be the same PR. These images are really confusing anyway:

$ podman run --rm -it quay.io/pypa/manylinux_2_34_aarch64
[snip]
WARNING: image platform (linux/arm64) does not match the expected platform (linux/amd64)
[root@fa86d82151b1 /]#

And it's Alma 9, not 8 like the x86-64 one.

This gets us glibc 2.28 without using `cargo zigbuild`, which is not compatible with PGO.
@Kobzol Kobzol force-pushed the manylinux-instead-of-zigbuild branch from bfe01bd to 46d282c Compare April 15, 2025 13:43
@Kobzol
Copy link
Member Author

Kobzol commented Apr 15, 2025

Ok, let's try!

@Kobzol Kobzol force-pushed the manylinux-instead-of-zigbuild branch from 46d282c to 4633fbf Compare April 15, 2025 13:45
@Kobzol
Copy link
Member Author

Kobzol commented Apr 15, 2025

Hmm, looks like with cross-compilation it's not so easy. Let's just do x64 for now then.

@lnicola
Copy link
Member

lnicola commented Apr 15, 2025

Error response from daemon: container 6d9810bb76206a2890d94b6fe1d59f91ce996906f62b50fddb2f024051f5671d is not running

What?

@Kobzol
Copy link
Member Author

Kobzol commented Apr 15, 2025

I think the real error was in the previous step ( WARNING: The requested image's platform (linux/arm64) does not match the detected host platform (linux/amd64/v3) and no specific platform was requested).

@lnicola
Copy link
Member

lnicola commented Apr 15, 2025

It's really weird, I get the same error (container is not running) locally even with the x86-64 image:

$ docker create --entrypoint tail quay.io/pypa/manylinux_2_34_x86_64 -f /dev/null
7f757bf3eb98a92723b0435457b9768ce5ca9ba92f950333f5b0f1d9aa2c5a85
$ docker exec 7f757bf3eb98a92723b0435457b9768ce5ca9ba92f950333f5b0f1d9aa2c5a85 sh -c "cat /etc/*release | grep ^ID"
Error response from daemon: container 7f757bf3eb98a92723b0435457b9768ce5ca9ba92f950333f5b0f1d9aa2c5a85 is not running

@Kobzol
Copy link
Member Author

Kobzol commented Apr 15, 2025

I can run the container locally just fine. I wouldn't worry about it, tbh, the cross-compilation part is clearly what's the issue, and I don't want to deal with that right now, x64 is much more important anyway for the optimization, as it's used by many more people.

@lnicola
Copy link
Member

lnicola commented Apr 15, 2025

I can run it too, but not like they do on GHA. Anyway, I was just hoping we could drop the Zig stuff at one point.

@lnicola lnicola added this pull request to the merge queue Apr 15, 2025
Merged via the queue into rust-lang:master with commit 0a60ce6 Apr 15, 2025
14 checks passed
@Kobzol Kobzol deleted the manylinux-instead-of-zigbuild branch April 15, 2025 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants