-
Notifications
You must be signed in to change notification settings - Fork 13.6k
Add tracing to resolve-related functions #144727
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
base: master
Are you sure you want to change the base?
Conversation
rustbot has assigned @petrochenkov. Use |
aaed279
to
035e767
Compare
Now that #144688 has been merged I rebased on |
@@ -730,6 +730,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { | |||
let existential_trait_ref = ty::ExistentialTraitRef::erase_self_ty(tcx, virtual_trait_ref); | |||
let concrete_trait_ref = existential_trait_ref.with_self_ty(tcx, dyn_ty); | |||
|
|||
let _span = enter_trace_span!(M, resolve::expect_resolve_for_vtable, ?def_id); |
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.
Did we call this _span
everywhere? I now realize that's confusing because usually that refers to the Span
type...
@@ -32,6 +32,8 @@ pub enum AccessKind { | |||
/// | |||
/// A `None` namespace indicates we are looking for a module. | |||
fn try_resolve_did(tcx: TyCtxt<'_>, path: &[&str], namespace: Option<Namespace>) -> Option<DefId> { | |||
let _span = enter_trace_span!(resolve::try_resolve_did, ?path); |
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.
This isn't the same kind of operation as the others at all, it is using resolve
to refer to something else. It's a good idea to trace this but separate from the normal resolve
calls.
@@ -730,6 +730,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> { | |||
let existential_trait_ref = ty::ExistentialTraitRef::erase_self_ty(tcx, virtual_trait_ref); | |||
let concrete_trait_ref = existential_trait_ref.with_self_ty(tcx, dyn_ty); | |||
|
|||
let _span = enter_trace_span!(M, resolve::expect_resolve_for_vtable, ?def_id); | |||
let concrete_method = Instance::expect_resolve_for_vtable( |
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.
Please use the same pattern with a nested block as everywhere else.
☔ The latest upstream changes (presumably #144773) made this pull request unmergeable. Please resolve the merge conflicts. |
@rustbot author |
Reminder, once the PR becomes ready for a review, use |
Resolve-related functions are not called often but still make up for ~3% of execution time for non-repetitive programs (as seen in the first table below, obtained from running the rust snippet at the bottom with
n=1
). On the other hand, for repetitive programs they become less relevant (I tested the same snippet but withn=100
and got ~1.5%), and it appears that onlytry_resolve
is called more often (see the last two tables).The first table was obtained by opening the trace file in https://ui.perfetto.dev and running the following query:
The following two tables show how many
resolve
spans there per subname/subcategory, and how much time is spent in each. The first is forn=1
and the second forn=100
. The query that was used is:The snippet I tested with Miri to obtain the above traces is:
r? @RalfJung