Skip to content

Allow profiles to set -Cforce-frame-pointers #15333

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.

Sign up for GitHub

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

Jump to bottom
Open
davidlattimore opened this issue Mar 19, 2025 · 8 comments · May be fixed by #15451
Open

Allow profiles to set -Cforce-frame-pointers #15333

davidlattimore opened this issue Mar 19, 2025 · 8 comments · May be fixed by #15451
Labels
A-profiles Area: profiles C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review

Comments

@davidlattimore
Copy link

davidlattimore commented Mar 19, 2025 •
edited by epage
Loading

Problem

When profiling Rust code on Linux using perf, I can either use perf record with --call-graph=dwarf or I can enable frame pointers during the build by passing -Cforce-frame-pointers to rustc. With frame pointers, a perf run is substantially faster to process. My understanding is that this is because stack traces can be captured in the kernel with eBPF, which can follow the frame pointers, whereas with debug info it's all done by calling addr2line after the fact.

For a profiling run I just did, it took more than 8 minutes to process the perf data when using debug info and less than a second when using frame pointers.

I'd like to be able to turn on frame pointers for a particular profile in my Cargo.toml. Currently, I instead turn it on globally in my ~/.cargo/config.toml.

Proposed Solution

Add force-frame-pointers as an option in cargo profiles. e.g.

[profile.profiling]
inherits = "release"
force-frame-pointers = true

Notes

If it's agreed that this is a reasonable feature to add, I'm happy to implement it.
@davidlattimore davidlattimore added C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` S-triage Status: This issue is waiting on initial triage. labels Mar 19, 2025
@epage epage added A-profiles Area: profiles S-needs-team-input Status: Needs input from team on whether/how to proceed. and removed S-triage Status: This issue is waiting on initial triage. labels Mar 20, 2025
@hanna-kruppe
Copy link

Currently, I instead turn it on globally in my ~/.cargo/config.toml.

Note that these config settings don’t have to be global, you can add a .cargo/config.toml file to your workspace root for example. Not quite a replacement for a profile setting for several reasons but FYI.

@ehuss
Copy link
Contributor

ehuss commented Apr 1, 2025 •
edited
Loading

The cargo team briefly discussed this, and agreed that it seemed like a reasonable thing to add.
We noted some minor details, such as what exact values it accepts. It would probably need some option that means to "use the default", since we've generally run into problems when we don't have that.

It was also asked whether or not this would need to be unstable or not, though a small change it probable isn't needed.

@ehuss ehuss added S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review and removed S-needs-team-input Status: Needs input from team on whether/how to proceed. labels Apr 1, 2025
brvtalcake added a commit to brvtalcake/cargo that referenced this issue Apr 23, 2025
@brvtalcake
feat(manifest): add a force-frame-pointers profile option
53e8b80 
resolves rust-lang#15333
@brvtalcake
Copy link
Contributor

It would probably need some option that means to "use the default", since we've generally run into problems when we don't have that.

Alright, so what should be the valid options ? Should it accept just booleans, or something else ?

I feel like the only value worth handling is true, since forwarding a false to the compiler would anyway be the same thing as keeping the defaults (at least it seems like, as per the documentation, but it's a bit unclear so I'm not sure).

And even if it's not the case, users wanting more control could as well use the unstable profile-rustflags, so we keep the force-frame-pointers option only to allow for cases where one may really want these frame pointers.

@brvtalcake brvtalcake linked a pull request Apr 23, 2025 that will close this issue
Draft
@epage
Copy link
Contributor

epage commented Apr 23, 2025

I feel like the only value worth handling is true, since forwarding a false to the compiler would anyway be the same thing as keeping the defaults (at least it seems like, as per the documentation, but it's a bit unclear so I'm not sure).

The fact that the compiler has a false state has me question this but I agree the docs are unclear.

Overall, I'd find it confusing that force-frame-pointers = false does not map to -Cforce-frame-pointers=false/off

If false is meaningful, then we'd need to also know when to not pass -Cforce-frame-pointers at all.

@brvtalcake
Copy link
Contributor

The fact that the compiler has a false state has me question this but I agree the docs are unclear.

Yes, exactly

Overall, I'd find it confusing that force-frame-pointers = false does not map to -Cforce-frame-pointers=false/off

I can understand that, and that's true. But I thought it would be a bit of a duplicate with the profile-rustflags feature. Tell me and I will change the code to properly forward any provided value to rustc

If false is meaningful, then we'd need to also know when to not pass -Cforce-frame-pointers at all.

You're right. I will quickly look at rustc's handling of the option. Otherwise something akin to the lto option could be OK ?

@epage
Copy link
Contributor

epage commented Apr 23, 2025

. Otherwise something akin to the lto option could be OK ?

lto looks to be a pretty complex profile setting, so saying to do something like it carries a lot of weight that I assume you don't intend.

@hanna-kruppe
Copy link

hanna-kruppe commented Apr 23, 2025 •
edited
Loading

The lto field is very confusing. If anything, I'd consider it a cautionary tale to not have any boolean fields and always prefer explicit strings even if true/false seem like the only possible option at the time (strip is another, milder example). How about e.g., frame-pointers = "force-on" in Cargo.toml to opt into rustc's -Cforce-frame-pointers=on? Then other variations (including an explicit name for "whatever the default is") can be added over time.

@brvtalcake
Copy link
Contributor

lto looks to be a pretty complex profile setting, so saying to do something like it carries a lot of weight that I assume you don't intend.

I meant to parse (or rather, deserialize) the (force-)?frame-pointers TOML field as cargo parses the lto one, that is, into a StringOrBool enum, but as @hanna-kruppe replied, this might be unnecessarily confusing.

Also there is apparently an unstable -Cforce-frame-pointers=non-leaf option, so the string idea could indeed be a better fit if cargo then wants to support more values

(now I feel like I got a little bit too excited and I should have waited for more input)

Tell me when you all agree on something and want me to rewrite the PR (and also feel free to close it if you instead prefer to do it). In the meantime I'm going to keep it as a draft

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-profiles Area: profiles C-feature-request Category: proposal for a feature. Before PR, ping rust-lang/cargo if this is not `Feature accepted` S-accepted Status: Issue or feature is accepted, and has a team member available to help mentor or review
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[WIP] feat(manifest): add a force-frame-pointers profile option brvtalcake/cargo
5 participants
@ehuss @epage @hanna-kruppe @davidlattimore @brvtalcake