Auto merge of #25323 - eddyb:coherent-coherence, r=pnkfelix

The loop to load all the known impls from external crates seems to have been used because `ty::populate_implementations_for_trait_if_necessary` wasn't doing its job, and solely relying on it resulted in loading only impls in the same crate as the trait.

Coherence for `librustc` was reduced from 18.310s to 0.610s, from stage1 to stage2.
Interestingly, type checking also went from 46.232s to 42.003s, though that could be noise or unrelated improvements.

On a smaller scale, `fn main() {}` now spends 0.003s in coherence instead of 0.368s, which fixes #22068.
It also peaks at only 1.2MB, instead of 16MB of heap usage.

Author: bors

src/librustc/metadata/common.rs

@@ -189,6 +189,7 @@ pub const tag_item_impl_vtables: usize = 0x7e;
 
 pub const tag_impls: usize = 0x109; // top-level only
 pub const tag_impls_impl: usize = 0x7f;
+pub const tag_impls_impl_trait_def_id: usize = 0x8d;
 
 pub const tag_items_data_item_inherent_impl: usize = 0x80;
 pub const tag_items_data_item_extension_impl: usize = 0x81;

src/librustc/metadata/csearch.rs

@@ -304,31 +304,23 @@ pub fn get_native_libraries(cstore: &cstore::CStore, crate_num: ast::CrateNum)
     decoder::get_native_libraries(&*cdata)
 }
 
-pub fn each_impl<F>(cstore: &cstore::CStore,
-                    crate_num: ast::CrateNum,
-                    callback: F) where
-    F: FnMut(ast::DefId),
-{
-    let cdata = cstore.get_crate_data(crate_num);
-    decoder::each_impl(&*cdata, callback)
-}
-
-pub fn each_implementation_for_type<F>(cstore: &cstore::CStore,
-                                       def_id: ast::DefId,
-                                       callback: F) where
+pub fn each_inherent_implementation_for_type<F>(cstore: &cstore::CStore,
+                                                def_id: ast::DefId,
+                                                callback: F) where
     F: FnMut(ast::DefId),
 {
     let cdata = cstore.get_crate_data(def_id.krate);
-    decoder::each_implementation_for_type(&*cdata, def_id.node, callback)
+    decoder::each_inherent_implementation_for_type(&*cdata, def_id.node, callback)
 }
 
 pub fn each_implementation_for_trait<F>(cstore: &cstore::CStore,
                                         def_id: ast::DefId,
-                                        callback: F) where
+                                        mut callback: F) where
     F: FnMut(ast::DefId),
 {
-    let cdata = cstore.get_crate_data(def_id.krate);
-    decoder::each_implementation_for_trait(&*cdata, def_id.node, callback)
+    cstore.iter_crate_data(|_, cdata| {
+        decoder::each_implementation_for_trait(cdata, def_id, &mut callback)
+    })
 }
 
 /// If the given def ID describes an item belonging to a trait (either a

src/librustc/metadata/decoder.rs

@@ -91,8 +91,8 @@ fn encode_impl_type_basename(rbml_w: &mut Encoder, name: ast::Name) {
     rbml_w.wr_tagged_str(tag_item_impl_type_basename, &token::get_name(name));
 }
 
-pub fn encode_def_id(rbml_w: &mut Encoder, id: DefId) {
-    rbml_w.wr_tagged_str(tag_def_id, &def_to_string(id));
+fn encode_def_id(rbml_w: &mut Encoder, id: DefId) {
+    rbml_w.wr_tagged_u64(tag_def_id, def_to_u64(id));
 }
 
 #[derive(Clone)]
@@ -122,6 +122,10 @@ fn encode_family(rbml_w: &mut Encoder, c: char) {
     rbml_w.wr_tagged_u8(tag_items_data_item_family, c as u8);
 }
 
+pub fn def_to_u64(did: DefId) -> u64 {
+    (did.krate as u64) << 32 | (did.node as u64)
+}
+
 pub fn def_to_string(did: DefId) -> String {
     format!("{}:{}", did.krate, did.node)
 }
@@ -153,9 +157,9 @@ fn encode_bounds_and_type<'a, 'tcx>(rbml_w: &mut Encoder,
 }
 
 fn encode_variant_id(rbml_w: &mut Encoder, vid: DefId) {
-    let s = def_to_string(vid);
-    rbml_w.wr_tagged_str(tag_items_data_item_variant, &s[..]);
-    rbml_w.wr_tagged_str(tag_mod_child, &s[..]);
+    let id = def_to_u64(vid);
+    rbml_w.wr_tagged_u64(tag_items_data_item_variant, id);
+    rbml_w.wr_tagged_u64(tag_mod_child, id);
 }
 
 pub fn write_closure_type<'a, 'tcx>(ecx: &EncodeContext<'a, 'tcx>,
@@ -260,7 +264,7 @@ fn encode_disr_val(_: &EncodeContext,
 }
 
 fn encode_parent_item(rbml_w: &mut Encoder, id: DefId) {
-    rbml_w.wr_tagged_str(tag_items_data_parent_item, &def_to_string(id));
+    rbml_w.wr_tagged_u64(tag_items_data_parent_item, def_to_u64(id));
 }
 
 fn encode_struct_fields(rbml_w: &mut Encoder,
@@ -275,7 +279,7 @@ fn encode_struct_fields(rbml_w: &mut Encoder,
         }
         encode_struct_field_family(rbml_w, f.vis);
         encode_def_id(rbml_w, f.id);
-        rbml_w.wr_tagged_str(tag_item_field_origin, &def_to_string(origin));
+        rbml_w.wr_tagged_u64(tag_item_field_origin, def_to_u64(origin));
         rbml_w.end_tag();
     }
 }
@@ -358,8 +362,8 @@ fn encode_reexported_static_method(rbml_w: &mut Encoder,
     debug!("(encode reexported static method) {}::{}",
             exp.name, token::get_name(method_name));
     rbml_w.start_tag(tag_items_data_item_reexport);
-    rbml_w.wr_tagged_str(tag_items_data_item_reexport_def_id,
-                         &def_to_string(method_def_id));
+    rbml_w.wr_tagged_u64(tag_items_data_item_reexport_def_id,
+                         def_to_u64(method_def_id));
     rbml_w.wr_tagged_str(tag_items_data_item_reexport_name,
                          &format!("{}::{}", exp.name,
                                             token::get_name(method_name)));
@@ -495,8 +499,8 @@ fn encode_reexports(ecx: &EncodeContext,
                        exp.def_id.node,
                        id);
                 rbml_w.start_tag(tag_items_data_item_reexport);
-                rbml_w.wr_tagged_str(tag_items_data_item_reexport_def_id,
-                                     &def_to_string(exp.def_id));
+                rbml_w.wr_tagged_u64(tag_items_data_item_reexport_def_id,
+                                     def_to_u64(exp.def_id));
                 rbml_w.wr_tagged_str(tag_items_data_item_reexport_name,
                                      exp.name.as_str());
                 rbml_w.end_tag();
@@ -526,12 +530,12 @@ fn encode_info_for_mod(ecx: &EncodeContext,
 
     // Encode info about all the module children.
     for item in &md.items {
-        rbml_w.wr_tagged_str(tag_mod_child,
-                             &def_to_string(local_def(item.id)));
+        rbml_w.wr_tagged_u64(tag_mod_child,
+                             def_to_u64(local_def(item.id)));
 
         each_auxiliary_node_id(&**item, |auxiliary_node_id| {
-            rbml_w.wr_tagged_str(tag_mod_child,
-                                 &def_to_string(local_def(auxiliary_node_id)));
+            rbml_w.wr_tagged_u64(tag_mod_child,
+                                 def_to_u64(local_def(auxiliary_node_id)));
             true
         });
 
@@ -541,8 +545,7 @@ fn encode_info_for_mod(ecx: &EncodeContext,
                    token::get_ident(ident),
                    did, ecx.tcx.map.node_to_string(did));
 
-            rbml_w.wr_tagged_str(tag_mod_impl,
-                                 &def_to_string(local_def(did)));
+            rbml_w.wr_tagged_u64(tag_mod_impl, def_to_u64(local_def(did)));
         }
     }
 
@@ -619,8 +622,7 @@ fn encode_parent_sort(rbml_w: &mut Encoder, sort: char) {
 fn encode_provided_source(rbml_w: &mut Encoder,
                           source_opt: Option<DefId>) {
     if let Some(source) = source_opt {
-        rbml_w.wr_tagged_str(tag_item_method_provided_source,
-                             &def_to_string(source));
+        rbml_w.wr_tagged_u64(tag_item_method_provided_source, def_to_u64(source));
     }
 }
 
@@ -725,8 +727,8 @@ fn encode_generics<'a, 'tcx>(rbml_w: &mut Encoder,
         encode_name(rbml_w, param.name);
         rbml_w.end_tag();
 
-        rbml_w.wr_tagged_str(tag_region_param_def_def_id,
-                             &def_to_string(param.def_id));
+        rbml_w.wr_tagged_u64(tag_region_param_def_def_id,
+                             def_to_u64(param.def_id));
 
         rbml_w.wr_tagged_u64(tag_region_param_def_space,
                              param.space.to_uint() as u64);
@@ -1089,8 +1091,8 @@ fn encode_info_for_item(ecx: &EncodeContext,
 
         // Encode all the items in this module.
         for foreign_item in &fm.items {
-            rbml_w.wr_tagged_str(tag_mod_child,
-                                 &def_to_string(local_def(foreign_item.id)));
+            rbml_w.wr_tagged_u64(tag_mod_child,
+                                 def_to_u64(local_def(foreign_item.id)));
         }
         encode_visibility(rbml_w, vis);
         encode_stability(rbml_w, stab);
@@ -1335,8 +1337,8 @@ fn encode_info_for_item(ecx: &EncodeContext,
             }
             rbml_w.end_tag();
 
-            rbml_w.wr_tagged_str(tag_mod_child,
-                                 &def_to_string(method_def_id.def_id()));
+            rbml_w.wr_tagged_u64(tag_mod_child,
+                                 def_to_u64(method_def_id.def_id()));
         }
         encode_path(rbml_w, path.clone());
 
@@ -1893,6 +1895,7 @@ impl<'a, 'b, 'c, 'tcx, 'v> Visitor<'v> for ImplVisitor<'a, 'b, 'c, 'tcx> {
                     def_id.krate != ast::LOCAL_CRATE {
                 self.rbml_w.start_tag(tag_impls_impl);
                 encode_def_id(self.rbml_w, local_def(item.id));
+                self.rbml_w.wr_tagged_u64(tag_impls_impl_trait_def_id, def_to_u64(def_id));
                 self.rbml_w.end_tag();
             }
         }
@@ -1932,12 +1935,12 @@ fn encode_misc_info(ecx: &EncodeContext,
     rbml_w.start_tag(tag_misc_info);
     rbml_w.start_tag(tag_misc_info_crate_items);
     for item in &krate.module.items {
-        rbml_w.wr_tagged_str(tag_mod_child,
-                             &def_to_string(local_def(item.id)));
+        rbml_w.wr_tagged_u64(tag_mod_child,
+                             def_to_u64(local_def(item.id)));
 
         each_auxiliary_node_id(&**item, |auxiliary_node_id| {
-            rbml_w.wr_tagged_str(tag_mod_child,
-                                 &def_to_string(local_def(auxiliary_node_id)));
+            rbml_w.wr_tagged_u64(tag_mod_child,
+                                 def_to_u64(local_def(auxiliary_node_id)));
             true
         });
     }

src/librustc/metadata/encoder.rs

@@ -2564,9 +2564,11 @@ impl<'tcx> TraitDef<'tcx> {
                        tcx: &ctxt<'tcx>,
                        impl_def_id: DefId,
                        impl_trait_ref: TraitRef<'tcx>) {
+        debug!("TraitDef::record_impl for {}, from {}",
+               self.repr(tcx), impl_trait_ref.repr(tcx));
+
         // We don't want to borrow_mut after we already populated all impls,
         // so check if an impl is present with an immutable borrow first.
-
         if let Some(sty) = fast_reject::simplify_type(tcx,
                                                       impl_trait_ref.self_ty(), false) {
             if let Some(is) = self.nonblanket_impls.borrow().get(&sty) {
@@ -6336,10 +6338,10 @@ pub fn populate_implementations_for_primitive_if_necessary(tcx: &ctxt,
     tcx.populated_external_primitive_impls.borrow_mut().insert(primitive_def_id);
 }
 
-/// Populates the type context with all the implementations for the given type
-/// if necessary.
-pub fn populate_implementations_for_type_if_necessary(tcx: &ctxt,
-                                                      type_id: ast::DefId) {
+/// Populates the type context with all the inherent implementations for
+/// the given type if necessary.
+pub fn populate_inherent_implementations_for_type_if_necessary(tcx: &ctxt,
+                                                               type_id: ast::DefId) {
     if type_id.krate == LOCAL_CRATE {
         return
     }
@@ -6348,37 +6350,15 @@ pub fn populate_implementations_for_type_if_necessary(tcx: &ctxt,
         return
     }
 
-    debug!("populate_implementations_for_type_if_necessary: searching for {:?}", type_id);
+    debug!("populate_inherent_implementations_for_type_if_necessary: searching for {:?}", type_id);
 
     let mut inherent_impls = Vec::new();
-    csearch::each_implementation_for_type(&tcx.sess.cstore, type_id, |impl_def_id| {
-        let impl_items = csearch::get_impl_items(&tcx.sess.cstore, impl_def_id);
-
-        // Record the implementation, if needed
-        if let Some(trait_ref) = csearch::get_impl_trait(tcx, impl_def_id) {
-            let trait_def = lookup_trait_def(tcx, trait_ref.def_id);
-            trait_def.record_impl(tcx, impl_def_id, trait_ref);
-        } else {
-            inherent_impls.push(impl_def_id);
-        }
-
-        // For any methods that use a default implementation, add them to
-        // the map. This is a bit unfortunate.
-        for impl_item_def_id in &impl_items {
-            let method_def_id = impl_item_def_id.def_id();
-            match impl_or_trait_item(tcx, method_def_id) {
-                MethodTraitItem(method) => {
-                    if let Some(source) = method.provided_source {
-                        tcx.provided_method_sources
-                           .borrow_mut()
-                           .insert(method_def_id, source);
-                    }
-                }
-                _ => {}
-            }
-        }
+    csearch::each_inherent_implementation_for_type(&tcx.sess.cstore, type_id, |impl_def_id| {
+        // Record the implementation.
+        inherent_impls.push(impl_def_id);
 
         // Store the implementation info.
+        let impl_items = csearch::get_impl_items(&tcx.sess.cstore, impl_def_id);
         tcx.impl_items.borrow_mut().insert(impl_def_id, impl_items);
     });
 
@@ -6388,18 +6368,18 @@ pub fn populate_implementations_for_type_if_necessary(tcx: &ctxt,
 
 /// Populates the type context with all the implementations for the given
 /// trait if necessary.
-pub fn populate_implementations_for_trait_if_necessary(
-        tcx: &ctxt,
-        trait_id: ast::DefId) {
+pub fn populate_implementations_for_trait_if_necessary(tcx: &ctxt, trait_id: ast::DefId) {
     if trait_id.krate == LOCAL_CRATE {
         return
     }
 
     let def = lookup_trait_def(tcx, trait_id);
     if def.flags.get().intersects(TraitFlags::IMPLS_VALID) {
-        return
+        return;
     }
 
+    debug!("populate_implementations_for_trait_if_necessary: searching for {}", def.repr(tcx));
+
     if csearch::is_defaulted_trait(&tcx.sess.cstore, trait_id) {
         record_trait_has_default_impl(tcx, trait_id);
     }

src/librustc/middle/ty.rs

@@ -371,7 +371,7 @@ impl<'a,'tcx> ProbeContext<'a,'tcx> {
     fn assemble_inherent_impl_candidates_for_type(&mut self, def_id: ast::DefId) {
         // Read the inherent implementation candidates for this type from the
         // metadata if necessary.
-        ty::populate_implementations_for_type_if_necessary(self.tcx(), def_id);
+        ty::populate_inherent_implementations_for_type_if_necessary(self.tcx(), def_id);
 
         if let Some(impl_infos) = self.tcx().inherent_impls.borrow().get(&def_id) {
             for &impl_def_id in &***impl_infos {