Skip to content

std: Don't emit debuginfo for backtrace deps - #74591

Closed
alexcrichton wants to merge 1 commit into
rust-lang:masterfrom
alexcrichton:backtrace-deps-no-debuginfo
Closed

std: Don't emit debuginfo for backtrace deps#74591
alexcrichton wants to merge 1 commit into
rust-lang:masterfrom
alexcrichton:backtrace-deps-no-debuginfo

Conversation

@alexcrichton

@alexcrichtonalexcrichton commented Jul 21, 2020

Copy link
Copy Markdown
Member

This was evaluated in #74560 and shown to have a positive effect on some
of the regressions originally reported from #73441. While it doesn't
recover all of the lost ground this is hopefully an easy enough knob to
tweak that we don't have worry too much about it later.

@rust-highfive

rust-highfive commented Jul 21, 2020

Copy link
Copy Markdown
Contributor

r? @Mark-Simulacrum

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfiverust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 21, 2020
@jonas-schievink

jonas-schievink commented Jul 21, 2020

Copy link
Copy Markdown
Contributor

@bors rollup=never

@Mark-Simulacrum

Mark-Simulacrum commented Jul 21, 2020

Copy link
Copy Markdown
Member

@bors r+

@bors

bors commented Jul 21, 2020

Copy link
Copy Markdown
Collaborator

📌 Commit 8a80a3eae7d0aa2f063d3effca4344edf9e73fdc has been approved by Mark-Simulacrum

@borsbors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 21, 2020
tesuji
tesuji reviewed Jul 21, 2020
View reviewed changes
Comment threadCargo.toml Outdated
Comment on lines 63 to 72

@tesujitesujiJul 21, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we make it more dense?

Suggested change
[profile.release.package.miniz_oxide]
debug = 0
[profile.release.package.gimli]
debug = 0
[profile.release.package.addr2line]
debug = 0
[profile.release.package.object]
debug = 0
[profile.release.package.adler]
debug = 0
[profile.release.package]
miniz_oxide = { debug = 0 }
gimli = { debug = 0 }
addr2line = { debug = 0 }
object = { debug = 0 }
adler = { debug = 0 }

@Mark-SimulacrumMark-SimulacrumJul 21, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems really unnecessary.

@eddybeddybJul 21, 2020•
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just out of curiosity, does TOML have the Nix thing of letting you write gimli = { debug = 0 } as gimli.debug = 0? IMO using the dot notation would feel more natural, but then again maybe that's personal bias.

@alexcrichtonalexcrichtonJul 21, 2020

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes TOML supports gimli.debug = 0, so I've gone ahead and updated to use that.

@alexcrichton
std: Don't emit debuginfo for backtrace deps
3075cc3
This was evaluated in rust-lang#74560 and shown to have a positive effect on some
of the regressions originally reported from rust-lang#73441. While it doesn't
recover all of the lost ground this is hopefully an easy enough knob to
tweak that we don't have worry too much about it later.
@alexcrichton
alexcrichtonforce-pushed the backtrace-deps-no-debuginfo branch from 8a80a3e to 3075cc3CompareJuly 21, 2020 22:41
@alexcrichton

alexcrichton commented Jul 21, 2020

Copy link
Copy Markdown
MemberAuthor

@bors: r=Mark-Simulacrum

@bors

bors commented Jul 21, 2020

Copy link
Copy Markdown
Collaborator

📌 Commit 3075cc3 has been approved by Mark-Simulacrum

@Mark-Simulacrum

Mark-Simulacrum commented Jul 22, 2020

Copy link
Copy Markdown
Member

@bors r-

We've discussed a bit and we want to revert the (original) backtrace PR and re-land it in the next week or so, but after getting a try build that fully evaluates it plus this patch (amongst others, perhaps) to better gauge the effects on perf.

@borsbors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jul 22, 2020
@Mark-SimulacrumMark-Simulacrum mentioned this pull request Jul 22, 2020
Merged
@Mark-SimulacrumMark-Simulacrum added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 22, 2020
@alexcrichton

alexcrichton commented Jul 22, 2020

Copy link
Copy Markdown
MemberAuthor

Ok, then let's close this.

@alexcrichton
alexcrichton deleted the backtrace-deps-no-debuginfo branch July 22, 2020 13:53
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Mark-SimulacrumMark-SimulacrumMark-Simulacrum left review comments
+2 more reviewers
@eddybeddybeddyb left review comments
@tesujitesujitesuji left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@Mark-SimulacrumMark-Simulacrum

Labels

S-blockedStatus: Blocked on something else such as an RFC or other implementation work.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@alexcrichton@rust-highfive@jonas-schievink@Mark-Simulacrum@bors@eddyb@tesuji