Skip to content
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

Make queries traceable again #112883

Merged
merged 1 commit into from
Jun 21, 2023
Merged

Make queries traceable again #112883

merged 1 commit into from
Jun 21, 2023

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Jun 21, 2023

This can't be tested without something along the lines of #111924 unfortunately.

We could benchmark turning query tracing into an info level tracing statement, but let's get this fix landed first so we can actually debug properly again

@rustbot
Copy link
Collaborator

rustbot commented Jun 21, 2023

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @cjgillot (or someone else) soon.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@rustbot rustbot added A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 21, 2023
cjgillot
cjgillot reviewed Jun 21, 2023
View reviewed changes
Copy link
Contributor

@cjgillot cjgillot left a comment

Choose a reason for hiding this comment

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

r=me with one nit.

TBH, I had trouble to use this debug tracing of queries: difficult to filter and drowning in some return values, HIR in particular. How do you use it?

compiler/rustc_query_impl/src/plumbing.rs Outdated
}
};
tracing::trace!(?ret);
ret
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you move this to the call site of call_provider?

@oli-obk
Copy link
Contributor Author

oli-obk commented Jun 21, 2023

difficult to filter and drowning in some return values, HIR in particular. How do you use it?

Yea, return values can be annoying. I mostly log to a file and then just skip over the bad parts 🙃

wrt filtering: I filter queries to specific arguments by filtering on the debug representation of the key: RUSTC_LOG=[typeck{key=\.\*main\.\*}]. Not sure if that is what you mean?

@oli-obk
Copy link
Contributor Author

oli-obk commented Jun 21, 2023

fwiw: I am looking into how to generally work with logs in a more reasonable scheme than "have a log file and grep through it". I want to start dumping json logs and then figure out some UI (possibly just a terminal UI for working with tracing logs specifically) that can actually fold nested calls instead of just indenting them

@oli-obk
Copy link
Contributor Author

oli-obk commented Jun 21, 2023

@bors r=cjgillot

@bors
Copy link
Contributor

bors commented Jun 21, 2023

📌 Commit 81e3774 has been approved by cjgillot

It is now in the queue for this repository.

@bors bors 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 Jun 21, 2023
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Jun 21, 2023
@GuillaumeGomez
Rollup merge of rust-lang#112883 - oli-obk:tracing_queries, r=cjgillot
9387fe7 
Make queries traceable again

This can't be tested without something along the lines of rust-lang#111924 unfortunately.

We could benchmark turning query tracing into an `info` level tracing statement, but let's get this fix landed first so we can actually debug properly again
This was referenced Jun 21, 2023
Closed
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 21, 2023
@bors
Auto merge of rust-lang#112890 - GuillaumeGomez:rollup-7e01q69, r=Gui…
536635c 
…llaumeGomez

Rollup of 7 pull requests

Successful merges:

 - rust-lang#99587 (Document memory orderings of `thread::{park, unpark}`)
 - rust-lang#112836 ([rustdoc] partially fix invalid files creation)
 - rust-lang#112853 (Add `lazy_type_alias` feature gate)
 - rust-lang#112863 (Fix copy-paste typo in `eprint(ln)` docs)
 - rust-lang#112883 (Make queries traceable again)
 - rust-lang#112885 (Fix msg passed to span_bug)
 - rust-lang#112886 (Revert 'Rename profile=user to profile=dist')

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 4f6c48b into rust-lang:master Jun 21, 2023
@rustbot rustbot added this to the 1.72.0 milestone Jun 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cjgillot cjgillot cjgillot left review comments

Assignees

@cjgillot cjgillot

Labels
A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.72.0
Development

Successfully merging this pull request may close these issues.
4 participants
@oli-obk @rustbot @bors @cjgillot