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

default binary size increase due to ice dumps logic #115610

Closed
oli-obk opened this issue Sep 6, 2023 · 3 comments · Fixed by #115627
Closed

default binary size increase due to ice dumps logic #115610

oli-obk opened this issue Sep 6, 2023 · 3 comments · Fixed by #115627
Labels
A-panic Area: Panicking machinery I-heavy Issue: Problems and improvements with respect to binary size of generated code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue.

Comments

@oli-obk
Copy link
Contributor

oli-obk commented Sep 6, 2023

#108714 slightly increased default binary size (checked on x86_64-pc-windows-msvc), the reason is now code

if let Some(path) = path
&& let Ok(mut out) = crate::fs::File::options().create(true).append(true).open(&path)
{
write(&mut out, BacktraceStyle::full());
}
unconditionally includes File::open (~ 2kb out of 130) and friends. can be tracked back by new imports of CreateFileW, GetFullPathNameW for rustc empty.rs -Copt-level=3 -Cdebuginfo=0 -Clto=yes -Ccodegen-units=1 -Cpanic=abort --edition=2018 where empty.rs is fn main(){}

Originally posted by @klensy in #108714 (comment)

@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Sep 6, 2023
@oli-obk
Copy link
Contributor Author

oli-obk commented Sep 6, 2023

cc @estebank

@cuviper
Copy link
Member

cuviper commented Sep 6, 2023

cc @rust-lang/libs -- I'm surprised that #108714 didn't include any libs review in the first place, having modified the default panic hook. Maybe rustc ought to be registering its own panic hook instead?

@fmease fmease added I-heavy Issue: Problems and improvements with respect to binary size of generated code. T-libs Relevant to the library team, which will review and decide on the PR/issue. A-panic Area: Panicking machinery and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Sep 6, 2023
@fmease fmease added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Sep 7, 2023
@bors bors closed this as completed in 42f5828 Sep 19, 2023
@klensy
Copy link
Contributor

klensy commented Sep 20, 2023

Size changed back to 123kb.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-panic Area: Panicking machinery I-heavy Issue: Problems and improvements with respect to binary size of generated code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants