Skip to content

Added tracing spans for rustc invocations #15464

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Apr 29, 2025

Conversation

ranger-ross
Copy link
Contributor

What does this PR try to resolve?

While doing some investigation on the theoretical performance implications of #4282 (and #15010 by extension) I was profiling cargo with some experimental changes. (Still a work in progress)

But in the mean time, noticed that we do not have spans for rustc invocations. I think these would be useful when profiling cargo build. (cargo build --timing exists but is more geared towards debugging a slow building project, not cargo itself)

For reference below is an example before/after of a profile run of a dummy crate with a few random dependencies.

Before

image

After

image

@rustbot
Copy link
Collaborator

rustbot commented Apr 29, 2025

r? @ehuss

rustbot has assigned @ehuss.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added A-build-execution Area: anything dealing with executing the compiler S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 29, 2025
package = name.as_str(),
process = rustc.to_string()
)
.entered();

let result = exec
Copy link
Member

Choose a reason for hiding this comment

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

Alternatively, trace all execs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I think that also works.
I put it outside since Executor seems a bit generic, but looking at the function signatures it is very much designed to run rustc.

I went ahead and moved it inside of the Executor in addc570 since I think its a bit nicer to use the #[instrument] macro :)

Copy link
Member

Choose a reason for hiding this comment

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

Thanks!

@@ -140,10 +140,11 @@ pub trait Executor: Send + Sync + 'static {
pub struct DefaultExecutor;

impl Executor for DefaultExecutor {
#[instrument(name = "rustc", skip_all, fields(package = id.name().as_str(), process = cmd.to_string()))]
Copy link
Member

Choose a reason for hiding this comment

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

Just note that this is at INFO log level.

Our instruments are all like this, but I feel like we might want to downgrade to debug or trace. Not a blocker though.

@weihanglo weihanglo added this pull request to the merge queue Apr 29, 2025
Merged via the queue into rust-lang:master with commit 4a7e881 Apr 29, 2025
23 checks passed
bors added a commit to rust-lang-ci/rust that referenced this pull request May 10, 2025
Update cargo

25 commits in 7918c7eb59614c39f1c4e27e99d557720976bdd7..056f5f4f3c100cb36b5e9aed2d20b9ea70aae295
2025-04-27 09:44:23 +0000 to 2025-05-09 14:54:18 +0000
- Revert "doc: Mention `XDG_DATA_HOME`" (rust-lang/cargo#15512)
- docs: update version notice for deprecation removal (rust-lang/cargo#15511)
- doc: Update instructions on using native-completions (rust-lang/cargo#15480)
- feat(network): use Retry-After header for HTTP 429 responses (rust-lang/cargo#15463)
- CI: Require schema job to pass (rust-lang/cargo#15504)
- chore(config): migrate renovate config (rust-lang/cargo#15501)
- Make cargo script ignore workspaces (rust-lang/cargo#15496)
- fix(rustc): Don't panic on unknown bins (rust-lang/cargo#15497)
- test: Remove unused nightly requirements (rust-lang/cargo#15498)
- Add support for `-Zembed-metadata` (rust-lang/cargo#15378)
- Fix tracking issue template link (rust-lang/cargo#15494)
- Refactor artifact deps in FeatureResolver::deps (rust-lang/cargo#15492)
- Improved error message for versions prefixed with `v` (rust-lang/cargo#15484)
- chore: fix some typos in comment (rust-lang/cargo#15485)
- fix: default to all targets when using `--edition` and ` --edition-idioms` in cargo fix (rust-lang/cargo#15192)
- Update fingerprint footnote (rust-lang/cargo#15478)
- feat(add): suggest similarly named features (rust-lang/cargo#15438)
- In package-workspace, keep dev-dependencies if they have a version (rust-lang/cargo#15470)
- docs: fix a typo in DependencyUI (rust-lang/cargo#15472)
- fix grammar, and remove confusing example (rust-lang/cargo#15457)
- Added tracing spans for rustc invocations (rust-lang/cargo#15464)
- Trivial tweaks to 'target_short_hash' (rust-lang/cargo#15461)
- chore(deps): update msrv (3 versions) to v1.84 (rust-lang/cargo#15456)
- feat(add/install): check if given crate argument would be valid with inserted @ symbol (rust-lang/cargo#15441)
- chang 1 tries to 1 try (rust-lang/cargo#15328)

r? ghost
@rustbot rustbot added this to the 1.89.0 milestone May 10, 2025
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-build-execution Area: anything dealing with executing the compiler 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.

5 participants