From 2c28c4ed69fd99eea1972e7d796435b9bb0e171d Mon Sep 17 00:00:00 2001
From: Ariel Ben-Yehuda <ariel.byd@gmail.com>
Date: Sat, 22 Sep 2018 21:53:58 +0300
Subject: [PATCH] avoid loading constructor attributes in AdtDef decoding

During metadata loading, the AdtDefs for every ADT in the universe need
to be loaded (for example, for coherence of builtin traits). For that,
the attributes of the AdtDef need to be loaded too.

The attributes of a struct are duplicated between 2 def ids - the
constructor def-id, and the "type" def id. Loading attributes for both
def-ids, which was done in #53721, slowed the compilation of small
crates by 2-3%. This PR makes sure we only load the attributes for the
"type" def-id, avoiding the slowdown.
---
 src/librustc/ty/mod.rs           | 18 ++++++++++++++----
 src/librustc_metadata/decoder.rs |  8 ++++++--
 src/librustc_typeck/collect.rs   | 15 ++++++++++-----
 3 files changed, 30 insertions(+), 11 deletions(-)

diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs
index 940da6099e0..9c91ecca8d9 100644
--- a/src/librustc/ty/mod.rs
+++ b/src/librustc/ty/mod.rs
@@ -1724,19 +1724,29 @@ impl<'a, 'gcx, 'tcx> VariantDef {
     /// - `did` is the DefId used for the variant - for tuple-structs, it is the constructor DefId,
     /// and for everything else, it is the variant DefId.
     /// - `attribute_def_id` is the DefId that has the variant's attributes.
+    /// this is the struct DefId for structs, and the variant DefId for variants.
+    ///
+    /// Note that we *could* use the constructor DefId, because the constructor attributes
+    /// redirect to the base attributes, but compiling a small crate requires
+    /// loading the AdtDefs for all the structs in the universe (e.g. coherence for any
+    /// built-in trait), and we do not want to load attributes twice.
+    ///
+    /// If someone speeds up attribute loading to not be a performance concern, they can
+    /// remove this hack and use the constructor DefId everywhere.
     pub fn new(tcx: TyCtxt<'a, 'gcx, 'tcx>,
                did: DefId,
                name: Name,
                discr: VariantDiscr,
                fields: Vec<FieldDef>,
                adt_kind: AdtKind,
-               ctor_kind: CtorKind)
+               ctor_kind: CtorKind,
+               attribute_def_id: DefId)
                -> Self
     {
-        debug!("VariantDef::new({:?}, {:?}, {:?}, {:?}, {:?}, {:?})", did, name, discr, fields,
-               adt_kind, ctor_kind);
+        debug!("VariantDef::new({:?}, {:?}, {:?}, {:?}, {:?}, {:?}, {:?})", did, name, discr,
+               fields, adt_kind, ctor_kind, attribute_def_id);
         let mut flags = VariantFlags::NO_VARIANT_FLAGS;
-        if adt_kind == AdtKind::Struct && tcx.has_attr(did, "non_exhaustive") {
+        if adt_kind == AdtKind::Struct && tcx.has_attr(attribute_def_id, "non_exhaustive") {
             debug!("found non-exhaustive field list for {:?}", did);
             flags = flags | VariantFlags::IS_FIELD_LIST_NON_EXHAUSTIVE;
         }
diff --git a/src/librustc_metadata/decoder.rs b/src/librustc_metadata/decoder.rs
index 43b03cb863c..c2296e4f78b 100644
--- a/src/librustc_metadata/decoder.rs
+++ b/src/librustc_metadata/decoder.rs
@@ -556,9 +556,12 @@ fn get_variant(&self,
             _ => bug!(),
         };
 
+        let def_id = self.local_def_id(data.struct_ctor.unwrap_or(index));
+        let attribute_def_id = self.local_def_id(index);
+
         ty::VariantDef::new(
             tcx,
-            self.local_def_id(data.struct_ctor.unwrap_or(index)),
+            def_id,
             self.item_name(index).as_symbol(),
             data.discr,
             item.children.decode(self).map(|index| {
@@ -570,7 +573,8 @@ fn get_variant(&self,
                 }
             }).collect(),
             adt_kind,
-            data.ctor_kind
+            data.ctor_kind,
+            attribute_def_id
         )
     }
 
diff --git a/src/librustc_typeck/collect.rs b/src/librustc_typeck/collect.rs
index 5309be21768..7d627e8228a 100644
--- a/src/librustc_typeck/collect.rs
+++ b/src/librustc_typeck/collect.rs
@@ -555,7 +555,8 @@ fn convert_variant<'a, 'tcx>(
     name: ast::Name,
     discr: ty::VariantDiscr,
     def: &hir::VariantData,
-    adt_kind: ty::AdtKind
+    adt_kind: ty::AdtKind,
+    attribute_def_id: DefId
 ) -> ty::VariantDef {
     let mut seen_fields: FxHashMap<ast::Ident, Span> = FxHashMap();
     let node_id = tcx.hir.as_local_node_id(did).unwrap();
@@ -592,7 +593,8 @@ fn convert_variant<'a, 'tcx>(
         discr,
         fields,
         adt_kind,
-        CtorKind::from_hir(def))
+        CtorKind::from_hir(def),
+        attribute_def_id)
 }
 
 fn adt_def<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> &'tcx ty::AdtDef {
@@ -622,7 +624,8 @@ fn adt_def<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> &'tcx ty::Ad
                         };
                         distance_from_explicit += 1;
 
-                        convert_variant(tcx, did, v.node.name, discr, &v.node.data, AdtKind::Enum)
+                        convert_variant(tcx, did, v.node.name, discr, &v.node.data, AdtKind::Enum,
+                                        did)
                     })
                     .collect(),
             )
@@ -642,7 +645,8 @@ fn adt_def<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> &'tcx ty::Ad
                     item.name,
                     ty::VariantDiscr::Relative(0),
                     def,
-                    AdtKind::Struct
+                    AdtKind::Struct,
+                    def_id
                 )],
             )
         }
@@ -654,7 +658,8 @@ fn adt_def<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> &'tcx ty::Ad
                 item.name,
                 ty::VariantDiscr::Relative(0),
                 def,
-                AdtKind::Union
+                AdtKind::Union,
+                def_id
             )],
         ),
         _ => bug!(),
-- 
GitLab