-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
force-frame-pointers
flag isn't honored always unless std is recompiled as such
#103711
Comments
force-frame-pointers
flag isn't honored always unless rustc is recompiledforce-frame-pointers
flag isn't honored always unless rustc is recompiled as such
That's probably prebuilt code shipped with the standard library, have you tried using |
hey, thanks for the tip. yep, i confirmed
|
so, this is a known issue? hopefully, a warn or something can be printed without being too noisy? |
Since this also applies to other codegen options (e.g. optimization levels) almost everyone setting RUSTFLAGS would get that warning. Maybe it could be documented in the cargo book. |
What would be the appropriate place to raise this issue regarding building the precompiled rustc binaries that rustup distributes with frame-pointers enabled? |
I am assuming the final comment by @alatiera is essentially the "real issue" here and leaving |
I guess there are 2 or maybe 3 different issues here probably:
All kinda related, but also can be tackled independently I think |
I think there are 3 ways to address this question, with varying tradeoffs:
|
Enabling frame-pointers I think it's the easiest option and developers will thank you for it. There is gonna be backlash from all the people with an opinion on the internet but I think the issue is pretty clear for any person that ever profiled an application. CPython is also making frame-pointers mandatory for 3.12 last I heard. |
Even with still allowing frame pointer elision for leaf functions, it induced a pretty hard compiler performance hit (test results in #115521 (comment) if you're wondering). I'm not sure if I could sell the compiler performance WG on it, much less the entire Rust community, despite frame pointers usually helping profiling. |
Hmm, this is definitely a place where using instructions is sub-optimal, though, because it'll clearly be more instructions -- it has to be, to maintain them -- but that means the "they're a fine proxy for speed because it'll be about the same run-to-run" argument for looking at them most of the time doesn't apply. If you instead look at its wall-time results and include everything (not just the regressions/improvements), then it's:
That's not nothing, but I think it's a better summary than the "+1.5%" that the instruction-count-changes summary shows. I still don't know that it's small enough to say "yeah, of course just take it", but it doesn't seem entirely unreasonable either. Maybe we could find the functions where most of the impact is coming from and Like having frame pointers in |
(Just found my way here through random thread-hopping and want to add whatever persuasive/rhetorical weight I can: having frame pointers everywhere by default is so, so worth the sprinkling of instructions it costs. Please do try to turn them on!) |
Hey, I also recently got hit by this issue while using https://github.com/tikv/pprof-rs on MacOS because of tikv/pprof-rs#131 (comment). I also want to point out that increased ability to profile code will unlock much greater optimizations than the raw perf loss, specially at ~1% or less. It will also get closer to perf tools "just working". |
The perf numbers being discussed here are about the performance of the compiler, not of user code. When hacking on the compiler we don't use a precompiled standard library, so limitations like this issue which are caused by the build configuration used for the precompiled standard library do not affect the compiler dev workflow. I agree with the plain "the small icount regression is worth the improved behavior" argument, but not "we'll claw back that 1% via optimizations people find because profiling is easier". |
Worth pointing out that Fedora and Ubuntu are switching to having frame pointers everywhere as well. There's a good write-up about the what and why here: https://www.brendangregg.com/blog/2024-03-17/the-return-of-the-frame-pointers.html I wonder if we could update the Rust default to simply always enable them? |
force-frame-pointers
flag isn't honored always unless rustc is recompiled as suchforce-frame-pointers
flag isn't honored always unless std is recompiled as such
As-of #122646 we now have frame pointers enabled in the stdlib! This does not "enable frame pointers by default", but it resolves the primary concern here and the one the issue title is actually about. For those wanting to recommend we enable frame pointers by default, it may be best to propose it to the compiler team and perhaps first get answers to some questions like "wait, doesn't the aarch64 call ABI use frame-pointers anyways? so the perf regressions, such as they are, don't necessarily materialize on all platforms? has anyone checked the assembly to verify this?" |
The compiler flag
-C force-frame-pointers=yes
isn't always honored, clobbering the%rbp
register and resulting in the unrecoverable stackframe forperf --call-graph fp
.It seems that if I compile
rustc
under-C force-frame-pointers=yes
, it works as shown below:setup
pre-built stock binary
rustc
re-built
rustc
under-C force-frame-pointer=rustc
I picked
std::sys::unix::futex::futex_wait
for the above example. But it seems that there's many:I suspect this is caused by pre-built fn machine code is statically linked into the binary, skipping compilation and not considering
RUSTFLAGS
.The text was updated successfully, but these errors were encountered: