-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
bootstrap: Include line numbers in debuginfo by default for library/compiler profile #104968
Conversation
(rustbot has picked a reviewer for you, use r? to override) |
My recollection is that this increases the disk size pretty considerably as well, could you measure that while you get the timing estimate? |
before:
after:
|
I wonder if it makes sense to only do this for the |
I think I have a system with a bit more parallelism. I see a I do not think this degree of slowdown is acceptable for a default, especially because evidence suggests it is mostly loaded into running the linker, which will always be done. If we could have
I think @thomcc previously commented that the benefit for |
I thought @thomcc was in favor of this change for library. |
It's really unfortunate to increase the library by ~5x, to half a gigabyte(!), even aside from the potential wall-time slowdown. I don't think we should do this by default at this time, it's too expensive a default. I do wonder whether the debuginfo really should be taking that much room -- naively, one might expect that a "line tables only" version should be able to be roughly on-par with what the source code (actually better!) itself takes up, whereas this is much bigger. Maybe we're including not just line table information but something more (e.g., types of some kind)? cc @rust-lang/wg-debugging -- I wonder if folks here have thoughts on what we can do to give a nicer experience at lower cost, or whether we think debuginfo is the wrong way to go, etc. |
The 5x size increase does seem fishy. |
☔ The latest upstream changes (presumably #107241) made this pull request unmergeable. Please resolve the merge conflicts. |
It turns out the vast majority of the size (over 350 MB!) comes from the |
#69074 also looks related. |
c66d4d3
to
98bec00
Compare
This PR changes src/bootstrap/defaults/config.codegen.toml. If appropriate, please also update This PR changes src/bootstrap/defaults/config.compiler.toml. If appropriate, please also update |
Ok, I added a new unstable flag that stops generating extra debuginfo when only line-level info is set, and enabled that and compression when building rustc itself. After that change, |
ah, I just realized the reduced size won't take effect for most contributors until the new flag hits beta - I can split it into a separate PR so it lands without having to change the defaults at the same time. |
Wall time before:
and after:
|
This comment has been minimized.
This comment has been minimized.
…oerister Extend -Cdebuginfo with new options and named aliases This is a rebase of rust-lang#83947, along with my best guess at what the new options mean. I tried to follow the LLVM source code to get a better idea but ran into quite a lot of trouble (https://rust-lang.zulipchat.com/#narrow/stream/187780-t-compiler.2Fwg-llvm/topic/go-to-definition.20in.20src.2Fllvm-project.3F). The description for the original PR follows below. Note that the changes in this PR have already been through FCP: rust-lang#83947 (comment) Closes rust-lang#109311. Helps with rust-lang#104968. r? `@michaelwoerister` cc `@cuviper` --- The -Cdebuginfo=1 option was never line tables only and can't be due to backwards compatibility issues. This was clarified and an option for emitting line tables only was added. Additionally an option for emitting line info directives only was added, which is needed for some targets, i.e. nvptx. The debug info options should now behave similarly to clang's debug info options. Fix rust-lang#60020 Fix rust-lang#64405
☔ The latest upstream changes (presumably #109808) made this pull request unmergeable. Please resolve the merge conflicts. |
Extend -Cdebuginfo with new options and named aliases This is a rebase of rust-lang/rust#83947, along with my best guess at what the new options mean. I tried to follow the LLVM source code to get a better idea but ran into quite a lot of trouble (https://rust-lang.zulipchat.com/#narrow/stream/187780-t-compiler.2Fwg-llvm/topic/go-to-definition.20in.20src.2Fllvm-project.3F). The description for the original PR follows below. Note that the changes in this PR have already been through FCP: rust-lang/rust#83947 (comment) Closes rust-lang/rust#109311. Helps with rust-lang/rust#104968. r? `@michaelwoerister` cc `@cuviper` --- The -Cdebuginfo=1 option was never line tables only and can't be due to backwards compatibility issues. This was clarified and an option for emitting line tables only was added. Additionally an option for emitting line info directives only was added, which is needed for some targets, i.e. nvptx. The debug info options should now behave similarly to clang's debug info options. Fix rust-lang/rust#60020 Fix rust-lang/rust#64405
waiting on me to test #110221 (comment) |
Without debuginfo:
With
For comparison, Both of these were taken with the default settings from |
f55c1db
to
abb371f
Compare
oh, i just realized we can cut down most of the added space in |
r=me, unless you want to do more here. @rustbot author |
abb371f
to
7651e2f
Compare
This had a semantic conflict with #112528 - I've rebased and pushed a commit that fixes the conflict as well as not building debuginfo for build scripts. |
err it's not actually correct, it changed the default for |
- Set the default to LineTablesOnly. This isn't enough debuginfo to run perf, but it's enough to get full backtraces. - Don't build debuginfo for build scripts and proc macros. This lowers the total disk space for stage0-rustc from 5.4 to 5.3 GB, or by 120 MB. - Fix a bug in deserialization where string values would never be deserialized For reasons I don't quite understand, using `&str` unconditionally failed: ``` thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Error { inner: Error { inner: TomlError { message: "data did not match any variant of untagged enum StringOrInt", original: None, keys: [], span: None } } }', src/main.rs:15:49 ```
7651e2f
to
12915ad
Compare
something here is broken and i don't have time to investigate it right now
this was significantly smaller last time i measured it and i don't know what changed. |
☔ The latest upstream changes (presumably #112756) made this pull request unmergeable. Please resolve the merge conflicts. |
This only includes debuginfo for the relevant component, so for library and tool developers the compile time impact should be negligible. For compiler authors I think the improved backtraces are worth the slight compile time hit (roughly 10% slower).
Set the default to LineTablesOnly. This isn't enough debuginfo to run perf, but it's enough to
get full backtraces.
Don't build debuginfo for build scripts and proc macros. This lowers the total disk space for stage0-rustc from 5.4 to 5.3 GB, or by 120 MB.
Fix a bug in deserialization where string values would never be deserialized
For reasons I don't quite understand, using
&str
unconditionally failed: