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

Binary size creep on Fuchsia after commit 5ced3da #127039

Open
amanda-tait opened this issue Jun 27, 2024 · 6 comments
Open

Binary size creep on Fuchsia after commit 5ced3da #127039

amanda-tait opened this issue Jun 27, 2024 · 6 comments
Labels
E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example I-heavy Issue: Problems and improvements with respect to binary size of generated code. O-fuchsia Operating system: Fuchsia T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@amanda-tait
Copy link

amanda-tait commented Jun 27, 2024

After bors rolled commit 5ced3da, Fuchsia's canaries began failing their binary size creep checks. See attached screenshot. It's non-obvious why this change would impact codegen on Fuchsia, but I wanted to alert the rust-lang project and commit author that this has occurred.

Screenshot 2024-06-27 at 10 14 40 AM

cc: @tesuji @erickt

@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jun 27, 2024
@tesuji
Copy link
Contributor

tesuji commented Jun 27, 2024

The rustc-perf result for #125853 is neutral. Also the code there doesn't change any behaviors in mir tests.
So you might want to bisect again.

Edit: If you want, I could open a try PR to revert suspicious c036594 which I think could be the most responsible in #125853.

@rustbot label +E-needs-mcve

@amanda-tait
Copy link
Author

Interesting. I've double-checked the bisection and am confident this is the commit after which we observe the size creep. That said, I have no theory of the case for why this is occurring, so I may just be engaging in a post hoc ergo propter hoc fallacy. If the perf result is neutral, that's good evidence that something else may be responsible.

If it's not too much trouble, please open the try PR to see if that sheds more light on what's going on. Note that this is not a blocker for Fuchsia; we just wanted to alert you that we had observed the size creep.

@tesuji
Copy link
Contributor

tesuji commented Jun 27, 2024

Btw, how did you build rustc on fuchsia ? Did you enable tracing facility ?

#debug-logging = rust.debug-assertions (boolean)

I asked because in the mentioned PR, I changed some tracing level.

@lqd
Copy link
Member

lqd commented Jun 27, 2024

The tracing level could maybe impact rustc's size but not the produced binaries size 🤔 .

Similarly, a try build of the mentioned revert shouldn't provide much more information, unless the fuchsia team tries it to see if it fixes the issue.

What could be interesting though is the simplest way to execute the "binary size creep check", and then we could try it within the fuchsia docker image.

@lqd
Copy link
Member

lqd commented Jun 27, 2024

@amanda-tait @erickt @tmandry @djkoloski @P1n3appl3 #124032 landed a few PRs before #125853 and definitely had binary size increases, did your size checks also flag that one up?

@veera-sivarajan
Copy link
Contributor

@rustbot label -needs-triage +O-fuchisa +T-compiler +I-heavy

@saethlin saethlin added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. I-heavy Issue: Problems and improvements with respect to binary size of generated code. O-fuchsia Operating system: Fuchsia and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Jun 28, 2024
@rustbot rustbot added the E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example label Jun 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example I-heavy Issue: Problems and improvements with respect to binary size of generated code. O-fuchsia Operating system: Fuchsia T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

6 participants