-
Notifications
You must be signed in to change notification settings - Fork 13.4k
Use the fn_span when emitting function calls for better debug info. #141372
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
Conversation
Some changes occurred in compiler/rustc_codegen_ssa |
@rustbot label A-debuginfo |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! I left a test suggestion.
@bors try |
@rustbot author |
Reminder, once the PR becomes ready for a review, use |
Use the fn_span when emitting function calls for better debug info. This especially improves the developer experience for long chains of function calls that span multiple lines, which is common with builder patterns, chains of iterator/future combinators, etc. try-job: armhf-gnu try-job: test-various try-job: x86_64-msvc-1 r? `@jieyouxu`
This comment has been minimized.
This comment has been minimized.
💔 Test failed - checks-actions |
@bors try |
Use the fn_span when emitting function calls for better debug info. This especially improves the developer experience for long chains of function calls that span multiple lines, which is common with builder patterns, chains of iterator/future combinators, etc. try-job: armhf-gnu try-job: test-various try-job: x86_64-msvc-1 r? `@jieyouxu`
@bors delegate+ (r=me if try job comes back green) |
@bors rollup=iffy (debuginfo) |
This comment has been minimized.
This comment has been minimized.
I'll look at the regex tmrw. |
Oh, doh, the line number has to be adjusted now of course. |
(You should be able to run try jobs yourself on this PR, since I delegated r+) |
@bors try |
Use the fn_span when emitting function calls for better debug info. This especially improves the developer experience for long chains of function calls that span multiple lines, which is common with builder patterns, chains of iterator/future combinators, etc. try-job: armhf-gnu try-job: test-various try-job: x86_64-msvc-1 try-job: arm-android r? `@jieyouxu`
This comment has been minimized.
This comment has been minimized.
💔 Test failed - checks-actions |
☀️ Test successful - checks-actions |
What is this?This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.Comparing 038d599 (parent) -> 5e16c66 (this PR) Test differencesShow 10 test diffsStage 1
Stage 2
Job group index
Test dashboardRun cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard 5e16c662062fd6dee91f0fe2a1580483488d80cf --output-dir test-dashboard And then open Job duration changes
How to interpret the job duration changes?Job durations can vary a lot, based on the actual runner instance |
Finished benchmarking commit (5e16c66): comparison URL. Overall result: ❌✅ regressions and improvements - please read the text belowOur benchmarks found a performance regression caused by this PR. Next Steps:
@rustbot label: +perf-regression Instruction countThis is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.
Max RSS (memory usage)Results (primary 3.7%, secondary 3.0%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResults (primary -0.5%, secondary 3.7%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeResults (primary 0.0%, secondary 0.0%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Bootstrap: 775.41s -> 775.893s (0.06%) |
Hm, interesting, I suppose we are indeed generating more debuginfo? 🤔 |
What does tt_muncher actually do? If it has a lot of function callsites this would make sense. |
There are only a handful of calls in tt_muncher and this PR adds exactly one new metadata node to the LLVM IR that produces the final binary ... But the line I added in this PR executes 5659 times on tt_muncher (as opposed to 9 times on a "Hello, world!" program). How does the macro work? Does that get compiled into code and executed and thrown away? Do we generate debug info for that code? |
That is a modified version intended to stress test macro expansion: https://github.com/rust-lang/rustc-perf/blob/9cae27ac44e0adfec64826dfcb005ea69896643a/collector/compile-benchmarks/tt-muncher/quote-1.0.17-modified/src/lib.rs#L120-L129 Which is then exercised by https://github.com/rust-lang/rustc-perf/blob/9cae27ac44e0adfec64826dfcb005ea69896643a/collector/compile-benchmarks/tt-muncher/src/main.rs#L7 which AFAICT would trigger a lot of calls. |
I'm not too sure for this case tbh:
|
@rylev for the time being, I think having more precise debuginfo justifies the perf hit. We are necessarily doing more work and generating more information for LLVM to digest. If we find that users report real world crates to exhibit even worse regressions, then I think we could consider a revert then. I'll keep an eye out for bug reports re. this. |
This especially improves the developer experience for long chains of function calls that span multiple lines, which is common with builder patterns, chains of iterator/future combinators, etc.
try-job: armhf-gnu
try-job: test-various
try-job: x86_64-msvc-1
try-job: arm-android
r? @jieyouxu