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

Latest nightly binary is broken on aarch64 Asahi ("unsupported system page size") #134563

Closed
eggyal opened this issue Dec 20, 2024 · 13 comments · Fixed by #135081
Closed

Latest nightly binary is broken on aarch64 Asahi ("unsupported system page size") #134563

eggyal opened this issue Dec 20, 2024 · 13 comments · Fixed by #135081
Labels
C-bug Category: This is a bug. O-AArch64 Armv8-A or later processors in AArch64 mode P-high High priority regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. S-has-bisection Status: a bisection has been found for this issue T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@eggyal
Copy link
Contributor

eggyal commented Dec 20, 2024

On aarch64 GNU/Linux:

$ rustc +nightly -vV
<jemalloc>: Unsupported system page size
<jemalloc>: Unsupported system page size
LLVM ERROR: out of memory
Allocation failed
Aborted (core dumped)

searched nightlies: from nightly-2024-12-19 to nightly-2024-12-20
regressed nightly: nightly-2024-12-20
searched commit range: 4ba4ac6...9e136a3
regressed commit: 023521e

bisected with cargo-bisect-rustc v0.6.7

Host triple: aarch64-unknown-linux-gnu
Reproduce with:

cargo bisect-rustc --without-cargo --start 2024-12-19 --script ./check.sh 

Due to #133809 cc @mrkajetanp @Kobzol
@rustbot label +I-compilemem +O-AArch64 +requires-nightly +S-has-bisection

@eggyal eggyal added the C-bug Category: This is a bug. label Dec 20, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Dec 20, 2024
@ionicmc-rs
Copy link
Contributor

Hey it didnt add the labels, let me try to add them

@rustbot label I-compilemem O-AArch64 requires-nightly S-has-bisection

@rustbot rustbot added I-compilemem Issue: Problems and improvements with respect to memory usage during compilation. O-AArch64 Armv8-A or later processors in AArch64 mode requires-nightly This issue requires a nightly compiler in some way. S-has-bisection Status: a bisection has been found for this issue labels Dec 20, 2024
@Kobzol
Copy link
Contributor

Kobzol commented Dec 20, 2024

Thanks for the report. We should probably first get the after-dist tests running, or make sure that we test and deploy aarch64 on the same hardware, because currently we don't really have any tests that check whether the built artifacts actually work..

Posted a revert in #134564.

@bjorn3
Copy link
Member

bjorn3 commented Dec 20, 2024

Are you on Asahi Linux by any chance? Asahi Linux uses 16k pages, but Jemalloc only supports a page size of at most the page size of the build system unless you explicitly override it with --with-lg-page at compile time. The tikv-jemalloc-sys crate we use supports setting this using JEMALLOC_SYS_WITH_LG_PAGE at build time.

@eggyal
Copy link
Contributor Author

eggyal commented Dec 20, 2024

Yes, I can confirm this was on Asahi.

@Kobzol
Copy link
Contributor

Kobzol commented Dec 20, 2024

Ok, so something like this wouldn't be caught on our CI anyway then. But it still seems like a regression, so we should probably restore the original behavior.

@eggyal eggyal changed the title Latest nightly binary for aarch64-unknown-linux-gnu is broken ("unsupported system page size") Latest nightly binary is broken on aarch64 Asahi ("unsupported system page size") Dec 20, 2024
@mrkajetanp
Copy link
Contributor

Should we maybe then just switch off jemalloc for this build since that's where the problem seems to be?

@Kobzol
Copy link
Contributor

Kobzol commented Dec 20, 2024

jemalloc should provide a non-trivial performance improvement, so it seems like a shame to disable it (since the goal of this effort was to improve rust'c performance on aarch64 :) ). So I'd probably first try restoring the current behavior, by allowing jemalloc to work with larger page sizes.

@ionicmc-rs

This comment has been minimized.

@ionicmc-rs
Copy link
Contributor

Are you on Asahi Linux by any chance? Asahi Linux uses 16k pages, but Jemalloc only supports a page size of at most the page size of the build system unless you explicitly override it with --with-lg-page at compile time. The tikv-jemalloc-sys crate we use supports setting this using JEMALLOC_SYS_WITH_LG_PAGE at build time.

I guess this would be triage (correct me if im wrong)

@rustbot -needs-triage

@Kobzol Kobzol removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Dec 20, 2024
@ionicmc-rs

This comment has been minimized.

@mrkajetanp
Copy link
Contributor

So I'd probably first try restoring the current behavior, by allowing jemalloc to work with larger page sizes.

For sure, that's the proper solution. FWIW this is a wider issue than just this PR, e.g.:

#133748

@mrkajetanp
Copy link
Contributor

So we can revert it for now as a stop gap but the underlying issue still needs to be fixed with the aforementioned JEMALLOC_SYS_WITH_LG_PAGE.

bors added a commit to rust-lang-ci/rust that referenced this issue Dec 20, 2024
Revert "Auto merge of rust-lang#133809 - mrkajetanp:ci-aarch64-dist, r=Kobzol"

This reverts rust-lang#133809, as it produced broken aarch64 artifacts (rust-lang#134563).

`@bors` p=1
github-actions bot pushed a commit to rust-lang/miri that referenced this issue Dec 21, 2024
Revert "Auto merge of #133809 - mrkajetanp:ci-aarch64-dist, r=Kobzol"

This reverts rust-lang/rust#133809, as it produced broken aarch64 artifacts (rust-lang/rust#134563).

`@bors` p=1
bors added a commit to rust-lang-ci/rust that referenced this issue Jan 3, 2025
bootstrap: Build jemalloc with support for 64K pages

By default, jemalloc is built to only support the same page size as the host machine. Set an env variable so that jemalloc is built with support for page sizes up to 64K regardless of the host machine.

r? `@Kobzol`

Resolves rust-lang#134563
Potentially resolves rust-lang#133748 (needs verification)

----

Results from local rustc-perf testing below, within 0.5% on every metric except max-rss.
AArch64:
![Screenshot 2025-01-03 at 5 53 13 pm](https://github.com/user-attachments/assets/71705c59-7d7b-4753-a184-8c784233e603)
x86_64:
![Screenshot 2025-01-03 at 5 54 16 pm](https://github.com/user-attachments/assets/ea28aded-3b90-43f4-a965-b081b07b95ab)
@jieyouxu jieyouxu added regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. and removed requires-nightly This issue requires a nightly compiler in some way. labels Jan 5, 2025
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Jan 5, 2025
@jieyouxu jieyouxu added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jan 5, 2025
@jieyouxu jieyouxu added T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) and removed I-compilemem Issue: Problems and improvements with respect to memory usage during compilation. labels Jan 5, 2025
@apiraino
Copy link
Contributor

apiraino commented Jan 6, 2025

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-critical

@rustbot rustbot added P-critical Critical priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Jan 6, 2025
@apiraino apiraino added P-high High priority and removed P-critical Critical priority labels Jan 6, 2025
bors added a commit to rust-lang-ci/rust that referenced this issue Jan 6, 2025
bootstrap: Build jemalloc with support for 64K pages

By default, jemalloc is built to only support the same page size as the host machine. Set an env variable so that jemalloc is built with support for page sizes up to 64K regardless of the host machine.

r? `@Kobzol`

Resolves rust-lang#134563
Potentially resolves rust-lang#133748 (needs verification)

----

Results from local rustc-perf testing below, within 0.5% on every metric except max-rss.
AArch64:
![Screenshot 2025-01-03 at 5 53 13 pm](https://github.com/user-attachments/assets/71705c59-7d7b-4753-a184-8c784233e603)
x86_64:
![Screenshot 2025-01-03 at 5 54 16 pm](https://github.com/user-attachments/assets/ea28aded-3b90-43f4-a965-b081b07b95ab)
@bors bors closed this as completed in 2b97db2 Jan 7, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jan 7, 2025
Rollup merge of rust-lang#135081 - mrkajetanp:jemalloc-64k, r=Kobzol

bootstrap: Build jemalloc with support for 64K pages

By default, jemalloc is built to only support the same page size as the host machine. Set an env variable so that jemalloc is built with support for page sizes up to 64K regardless of the host machine.

r? `@Kobzol`

Resolves rust-lang#134563
Potentially resolves rust-lang#133748 (needs verification)

----

Results from local rustc-perf testing below, within 0.5% on every metric except max-rss.
AArch64:
![Screenshot 2025-01-03 at 5 53 13 pm](https://github.com/user-attachments/assets/71705c59-7d7b-4753-a184-8c784233e603)
x86_64:
![Screenshot 2025-01-03 at 5 54 16 pm](https://github.com/user-attachments/assets/ea28aded-3b90-43f4-a965-b081b07b95ab)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. O-AArch64 Armv8-A or later processors in AArch64 mode P-high High priority regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. S-has-bisection Status: a bisection has been found for this issue T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants