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

Use split-debuginfo = "unpacked" for debug builds #10516

Merged
merged 1 commit into from
Mar 24, 2023

Conversation

Alexendoo
Copy link
Member

On Windows this has no effect as it's unsupported. On macOS the default set by cargo is already unpacked so no effect there either

For Linux it shaves a bit off the rebuild time, for me in the case of a simple touch + cargo build it goes from 12s to 10s

It saves a good amount of disk space too, on aarch64-unknown-linux-gnu it saves 1.2GB for a plain cargo build, 3GB when also running cargo dev and cargo test --no-run -F internal

r? @flip1995

changelog: none

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Mar 17, 2023
@xFrednet
Copy link
Member

Out of interest, do you know why unpacked is the default on Mac but not on linux? 🤔

@Alexendoo
Copy link
Member Author

I'm not sure, I think it would be up to cargo to make that change but I can't see anything about unpacked on linux in their issue list

@flip1995
Copy link
Member

@bors r+

That sounds like a really nice improvement. The Clippy target dir is responsible for 9.5% of the file sizes in my $HOME dir. This will improve things :)

@bors
Copy link
Collaborator

bors commented Mar 22, 2023

📌 Commit 43f8859 has been approved by flip1995

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Mar 22, 2023

⌛ Testing commit 43f8859 with merge 4f1ecfe...

bors added a commit that referenced this pull request Mar 22, 2023
Use `split-debuginfo = "unpacked"` for debug builds

On Windows this has no effect as it's unsupported. On macOS the default set by cargo is already unpacked so no effect there either

For Linux it shaves a bit off the rebuild time, for me in the case of a simple `touch` + `cargo build` it goes from 12s to 10s

It saves a good amount of disk space too, on `aarch64-unknown-linux-gnu` it saves 1.2GB for a plain `cargo build`, 3GB when also running `cargo dev` and `cargo test --no-run -F internal`

r? `@flip1995`

changelog: none
@bors
Copy link
Collaborator

bors commented Mar 22, 2023

💔 Test failed - checks-action_test

@Alexendoo
Copy link
Member Author

The extra files in deps confused the mv command, but I've set the integration build to not use split debuginfo since it's easier to just transfer the binaries

@flip1995
Copy link
Member

Thanks!

@bors r+

@bors
Copy link
Collaborator

bors commented Mar 24, 2023

📌 Commit a90e5cc has been approved by flip1995

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Mar 24, 2023

⌛ Testing commit a90e5cc with merge d9e42b0...

bors added a commit that referenced this pull request Mar 24, 2023
Use `split-debuginfo = "unpacked"` for debug builds

On Windows this has no effect as it's unsupported. On macOS the default set by cargo is already unpacked so no effect there either

For Linux it shaves a bit off the rebuild time, for me in the case of a simple `touch` + `cargo build` it goes from 12s to 10s

It saves a good amount of disk space too, on `aarch64-unknown-linux-gnu` it saves 1.2GB for a plain `cargo build`, 3GB when also running `cargo dev` and `cargo test --no-run -F internal`

r? `@flip1995`

changelog: none
@flip1995
Copy link
Member

@bors retry (yeeting to rustup PR)

@bors
Copy link
Collaborator

bors commented Mar 24, 2023

⌛ Testing commit a90e5cc with merge 00e9372...

@bors
Copy link
Collaborator

bors commented Mar 24, 2023

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: flip1995
Pushing 00e9372 to master...

@bors bors merged commit 00e9372 into rust-lang:master Mar 24, 2023
@Alexendoo Alexendoo deleted the split-debuginfo branch March 24, 2023 13:58
@klensy
Copy link
Contributor

klensy commented May 25, 2023

Funny thing: on my windows machine with this change rustc(or cargo?) forgets to clean 2.2GB of *.o files in deps folder.

@klensy
Copy link
Contributor

klensy commented May 25, 2023

Looks like it preserved here, but should this work for x86_64-pc-windows-msvc? Idk:
https://github.com/rust-lang/rust/blob/0b011b7b7e5d1a1737aa3337f01b79fd5f56cf04/compiler/rustc_codegen_ssa/src/back/link.rs#L1368-L1389

@Alexendoo
Copy link
Member Author

hmm I see what's happening I think, we use -Zunstable-options to enable internal rustc lints, but that also enables unsupported split-debuginfo profiles

I'll see if upstream will accept another -Z flag for that instead of using -Zunstable-options

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 14, 2023
…nfo, r=b-naber

Don't print unsupported split-debuginfo modes with `-Zunstable-options`

Currently unsupported `split-debuginfo` options are enabled by `-Zunstable-options`, for projects that have `-Zunstable-options` for other reasons this can be [an unexpected interaction](rust-lang/rust-clippy#10516 (comment))

This PR makes it so that `--print split-debuginfo -Zunstable-options` doesn't print unsupported modes, so that a cargo config of e.g.

```toml
[profile.dev]
split-debuginfo = "unpacked"
```

Would not cause an unsupported mode to be enabled on `x86_64-pc-windows-msvc`
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.

6 participants