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.

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

rustc_metadata: track the simplified Self type for every trait impl. #75008

Merged
merged 1 commit into from
Aug 6, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 13 additions & 8 deletions src/librustc_metadata/rmeta/decoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,8 @@ crate struct CrateMetadata {
/// Trait impl data.
/// FIXME: Used only from queries and can use query cache,
/// so pre-decoding can probably be avoided.
trait_impls: FxHashMap<(u32, DefIndex), Lazy<[DefIndex]>>,
trait_impls:
FxHashMap<(u32, DefIndex), Lazy<[(DefIndex, Option<ty::fast_reject::SimplifiedType>)]>>,
/// Proc macro descriptions for this crate, if it's a proc macro crate.
raw_proc_macros: Option<&'static [ProcMacro]>,
/// Source maps for code from the crate.
Expand Down Expand Up @@ -1289,7 +1290,7 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
&self,
tcx: TyCtxt<'tcx>,
filter: Option<DefId>,
) -> &'tcx [DefId] {
) -> &'tcx [(DefId, Option<ty::fast_reject::SimplifiedType>)] {
if self.root.is_proc_macro_crate() {
// proc-macro crates export no trait impls.
return &[];
Expand All @@ -1305,16 +1306,20 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {

if let Some(filter) = filter {
if let Some(impls) = self.trait_impls.get(&filter) {
tcx.arena.alloc_from_iter(impls.decode(self).map(|idx| self.local_def_id(idx)))
tcx.arena.alloc_from_iter(
impls.decode(self).map(|(idx, simplified_self_ty)| {
(self.local_def_id(idx), simplified_self_ty)
}),
)
} else {
&[]
}
} else {
tcx.arena.alloc_from_iter(
self.trait_impls
.values()
.flat_map(|impls| impls.decode(self).map(|idx| self.local_def_id(idx))),
)
tcx.arena.alloc_from_iter(self.trait_impls.values().flat_map(|impls| {
impls
.decode(self)
.map(|(idx, simplified_self_ty)| (self.local_def_id(idx), simplified_self_ty))
}))
}
}

Expand Down
12 changes: 9 additions & 3 deletions src/librustc_metadata/rmeta/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1607,7 +1607,7 @@ impl EncodeContext<'a, 'tcx> {
.into_iter()
.map(|(trait_def_id, mut impls)| {
// Bring everything into deterministic order for hashing
impls.sort_by_cached_key(|&index| {
impls.sort_by_cached_key(|&(index, _)| {
tcx.hir().definitions().def_path_hash(LocalDefId { local_def_index: index })
});

Expand Down Expand Up @@ -1849,15 +1849,21 @@ impl EncodeContext<'a, 'tcx> {

struct ImplVisitor<'tcx> {
tcx: TyCtxt<'tcx>,
impls: FxHashMap<DefId, Vec<DefIndex>>,
impls: FxHashMap<DefId, Vec<(DefIndex, Option<ty::fast_reject::SimplifiedType>)>>,
}

impl<'tcx, 'v> ItemLikeVisitor<'v> for ImplVisitor<'tcx> {
fn visit_item(&mut self, item: &hir::Item<'_>) {
if let hir::ItemKind::Impl { .. } = item.kind {
let impl_id = self.tcx.hir().local_def_id(item.hir_id);
if let Some(trait_ref) = self.tcx.impl_trait_ref(impl_id.to_def_id()) {
self.impls.entry(trait_ref.def_id).or_default().push(impl_id.local_def_index);
let simplified_self_ty =
ty::fast_reject::simplify_type(self.tcx, trait_ref.self_ty(), false);

self.impls
.entry(trait_ref.def_id)
.or_default()
.push((impl_id.local_def_index, simplified_self_ty));
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_metadata/rmeta/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ crate struct CrateDep {
#[derive(RustcEncodable, RustcDecodable)]
crate struct TraitImpls {
trait_id: (u32, DefIndex),
impls: Lazy<[DefIndex]>,
impls: Lazy<[(DefIndex, Option<ty::fast_reject::SimplifiedType>)]>,
}

/// Define `LazyTables` and `TableBuilders` at the same time.
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_middle/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1125,11 +1125,11 @@ rustc_queries! {

TypeChecking {
query implementations_of_trait(_: (CrateNum, DefId))
-> &'tcx [DefId] {
-> &'tcx [(DefId, Option<ty::fast_reject::SimplifiedType>)] {
desc { "looking up implementations of a trait in a crate" }
}
query all_trait_implementations(_: CrateNum)
-> &'tcx [DefId] {
-> &'tcx [(DefId, Option<ty::fast_reject::SimplifiedType>)] {
desc { "looking up all (?) trait implementations" }
}
}
Expand Down
50 changes: 28 additions & 22 deletions src/librustc_middle/ty/trait_def.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,32 +187,38 @@ pub(super) fn all_local_trait_impls<'tcx>(
pub(super) fn trait_impls_of_provider(tcx: TyCtxt<'_>, trait_id: DefId) -> TraitImpls {
let mut impls = TraitImpls::default();

{
let mut add_impl = |impl_def_id: DefId| {
let impl_self_ty = tcx.type_of(impl_def_id);
Copy link
Contributor

Choose a reason for hiding this comment

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

so is the performance improvement here just that we don't have to compute type_of and then simplify the type, but instead we do it "up front" when encoding?

Copy link
Member Author

@eddyb eddyb Aug 3, 2020

Choose a reason for hiding this comment

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

Right, I think all the cost is in decoding the Ty from type_of (simplification should be relatively cheap).

And based on the detailed query profiling info, I'm guessing it's ty::Adt containing a &'tcx ty::AdtDef instead of just a DefId, and somehow item_attrs getting called for every struct/enum/union implementing the trait (in order to create the ty::AdtDef?).

I'm suspecting this is similar to what @nnethercote was talking about a few months ago, where the deserialization of doc comment attributes was a significant cost (I don't recall what happened with that line of investigation though).

Copy link
Contributor

Choose a reason for hiding this comment

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

#65750 introduced a specialized representation for doc comments, that might be what you are referring to.

if impl_def_id.is_local() && impl_self_ty.references_error() {
return;
}

if let Some(simplified_self_ty) = fast_reject::simplify_type(tcx, impl_self_ty, false) {
impls.non_blanket_impls.entry(simplified_self_ty).or_default().push(impl_def_id);
} else {
impls.blanket_impls.push(impl_def_id);
}
};

// Traits defined in the current crate can't have impls in upstream
// crates, so we don't bother querying the cstore.
if !trait_id.is_local() {
for &cnum in tcx.crates().iter() {
for &def_id in tcx.implementations_of_trait((cnum, trait_id)).iter() {
add_impl(def_id);
// Traits defined in the current crate can't have impls in upstream
// crates, so we don't bother querying the cstore.
if !trait_id.is_local() {
for &cnum in tcx.crates().iter() {
for &(impl_def_id, simplified_self_ty) in
tcx.implementations_of_trait((cnum, trait_id)).iter()
{
if let Some(simplified_self_ty) = simplified_self_ty {
impls
.non_blanket_impls
.entry(simplified_self_ty)
.or_default()
.push(impl_def_id);
} else {
impls.blanket_impls.push(impl_def_id);
}
}
}
}

for &hir_id in tcx.hir().trait_impls(trait_id) {
let impl_def_id = tcx.hir().local_def_id(hir_id).to_def_id();

let impl_self_ty = tcx.type_of(impl_def_id);
if impl_self_ty.references_error() {
continue;
}

for &hir_id in tcx.hir().trait_impls(trait_id) {
add_impl(tcx.hir().local_def_id(hir_id).to_def_id());
if let Some(simplified_self_ty) = fast_reject::simplify_type(tcx, impl_self_ty, false) {
impls.non_blanket_impls.entry(simplified_self_ty).or_default().push(impl_def_id);
} else {
impls.blanket_impls.push(impl_def_id);
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/librustdoc/passes/collect_trait_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ pub fn collect_trait_impls(krate: Crate, cx: &DocContext<'_>) -> Crate {
let mut new_items = Vec::new();

for &cnum in cx.tcx.crates().iter() {
for &did in cx.tcx.all_trait_implementations(cnum).iter() {
for &(did, _) in cx.tcx.all_trait_implementations(cnum).iter() {
inline::build_impl(cx, did, None, &mut new_items);
}
}
Expand Down