rustc_metadata: Minimize use of Lrc in crate store

petrochenkov · petrochenkov · commit 9f5a530b84c7 · 2019-10-24T20:54:16.000+03:00
Crate metadatas are still stored as `Lrc&lt;CrateMetadata&gt;` in `CStore` because crate store has to be cloneable due to `Resolver::clone_outputs`.
diff --git a/src/librustc/middle/cstore.rs b/src/librustc/middle/cstore.rs
@@ -16,7 +16,7 @@ use syntax::ast;
 use syntax::symbol::Symbol;
 use syntax_pos::Span;
 use rustc_target::spec::Target;
-use rustc_data_structures::sync::{self, MetadataRef, Lrc};
+use rustc_data_structures::sync::{self, MetadataRef};
 use rustc_macros::HashStable;
 
 pub use self::NativeLibraryKind::*;
@@ -203,13 +203,13 @@ pub type MetadataLoaderDyn = dyn MetadataLoader + Sync;
 /// (it'd break incremental compilation) and should only be called pre-HIR (e.g.
 /// during resolve)
 pub trait CrateStore {
-    fn crate_data_as_rc_any(&self, krate: CrateNum) -> Lrc<dyn Any>;
+    fn crate_data_as_any(&self, cnum: CrateNum) -> &dyn Any;
 
     // resolve
     fn def_key(&self, def: DefId) -> DefKey;
     fn def_path(&self, def: DefId) -> hir_map::DefPath;
     fn def_path_hash(&self, def: DefId) -> hir_map::DefPathHash;
-    fn def_path_table(&self, cnum: CrateNum) -> Lrc<DefPathTable>;
+    fn def_path_table(&self, cnum: CrateNum) -> &DefPathTable;
 
     // "queries" used in resolve that aren't tracked for incremental compilation
     fn crate_name_untracked(&self, cnum: CrateNum) -> Symbol;
diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs
@@ -1213,34 +1213,27 @@ impl<'tcx> TyCtxt<'tcx> {
         let common_consts = CommonConsts::new(&interners, &common_types);
         let dep_graph = hir.dep_graph.clone();
         let cstore = resolutions.cstore;
-        let max_cnum = cstore.crates_untracked().iter().map(|c| c.as_usize()).max().unwrap_or(0);
+        let crates = cstore.crates_untracked();
+        let max_cnum = crates.iter().map(|c| c.as_usize()).max().unwrap_or(0);
         let mut providers = IndexVec::from_elem_n(extern_providers, max_cnum + 1);
         providers[LOCAL_CRATE] = local_providers;
 
         let def_path_hash_to_def_id = if s.opts.build_dep_graph() {
-            let upstream_def_path_tables: Vec<(CrateNum, Lrc<_>)> = cstore
-                .crates_untracked()
+            let def_path_tables = crates
                 .iter()
                 .map(|&cnum| (cnum, cstore.def_path_table(cnum)))
-                .collect();
-
-            let def_path_tables = || {
-                upstream_def_path_tables
-                    .iter()
-                    .map(|&(cnum, ref rc)| (cnum, &**rc))
-                    .chain(iter::once((LOCAL_CRATE, hir.definitions().def_path_table())))
-            };
+                .chain(iter::once((LOCAL_CRATE, hir.definitions().def_path_table())));
 
             // Precompute the capacity of the hashmap so we don't have to
             // re-allocate when populating it.
-            let capacity = def_path_tables().map(|(_, t)| t.size()).sum::<usize>();
+            let capacity = def_path_tables.clone().map(|(_, t)| t.size()).sum::<usize>();
 
             let mut map: FxHashMap<_, _> = FxHashMap::with_capacity_and_hasher(
                 capacity,
                 ::std::default::Default::default()
             );
 
-            for (cnum, def_path_table) in def_path_tables() {
+            for (cnum, def_path_table) in def_path_tables {
                 def_path_table.add_def_path_hashes_to(cnum, &mut map);
             }
 
@@ -1417,8 +1410,8 @@ impl<'tcx> TyCtxt<'tcx> {
 
     // Note that this is *untracked* and should only be used within the query
     // system if the result is otherwise tracked through queries
-    pub fn crate_data_as_rc_any(self, cnum: CrateNum) -> Lrc<dyn Any> {
-        self.cstore.crate_data_as_rc_any(cnum)
+    pub fn crate_data_as_any(self, cnum: CrateNum) -> &'tcx dyn Any {
+        self.cstore.crate_data_as_any(cnum)
     }
 
     #[inline(always)]
diff --git a/src/librustc_metadata/creader.rs b/src/librustc_metadata/creader.rs
@@ -3,7 +3,7 @@
 use crate::cstore::{self, CStore, MetadataBlob};
 use crate::locator::{self, CratePaths};
 use crate::schema::{CrateRoot, CrateDep};
-use rustc_data_structures::sync::{Lrc, RwLock, Lock, AtomicCell};
+use rustc_data_structures::sync::{RwLock, Lock, AtomicCell};
 
 use rustc::hir::def_id::CrateNum;
 use rustc_data_structures::svh::Svh;
@@ -186,7 +186,7 @@ impl<'a> CrateLoader<'a> {
         lib: Library,
         dep_kind: DepKind,
         name: Symbol
-    ) -> (CrateNum, Lrc<cstore::CrateMetadata>) {
+    ) -> CrateNum {
         let _prof_timer = self.sess.prof.generic_activity("metadata_register_crate");
 
         let Library { source, metadata } = lib;
@@ -240,9 +240,9 @@ impl<'a> CrateLoader<'a> {
             crate_root.def_path_table.decode((&metadata, self.sess))
         });
 
-        let cmeta = cstore::CrateMetadata {
+        self.cstore.set_crate_data(cnum, cstore::CrateMetadata {
             extern_crate: Lock::new(None),
-            def_path_table: Lrc::new(def_path_table),
+            def_path_table,
             trait_impls,
             root: crate_root,
             blob: metadata,
@@ -256,11 +256,9 @@ impl<'a> CrateLoader<'a> {
             private_dep,
             raw_proc_macros,
             dep_node_index: AtomicCell::new(DepNodeIndex::INVALID),
-        };
+        });
 
-        let cmeta = Lrc::new(cmeta);
-        self.cstore.set_crate_data(cnum, cmeta.clone());
-        (cnum, cmeta)
+        cnum
     }
 
     fn load_proc_macro<'b>(
@@ -324,7 +322,7 @@ impl<'a> CrateLoader<'a> {
         span: Span,
         dep_kind: DepKind,
         dep: Option<(&'b CratePaths, &'b CrateDep)>,
-    ) -> (CrateNum, Lrc<cstore::CrateMetadata>) {
+    ) -> CrateNum {
         self.maybe_resolve_crate(name, span, dep_kind, dep).unwrap_or_else(|err| err.report())
     }
 
@@ -334,7 +332,7 @@ impl<'a> CrateLoader<'a> {
         span: Span,
         mut dep_kind: DepKind,
         dep: Option<(&'b CratePaths, &'b CrateDep)>,
-    ) -> Result<(CrateNum, Lrc<cstore::CrateMetadata>), LoadError<'b>> {
+    ) -> Result<CrateNum, LoadError<'b>> {
         info!("resolving crate `{}`", name);
         let (root, hash, extra_filename, path_kind) = match dep {
             Some((root, dep)) =>
@@ -380,7 +378,7 @@ impl<'a> CrateLoader<'a> {
                 data.dep_kind.with_lock(|data_dep_kind| {
                     *data_dep_kind = cmp::max(*data_dep_kind, dep_kind);
                 });
-                Ok((cnum, data))
+                Ok(cnum)
             }
             (LoadResult::Loaded(library), host_library) => {
                 Ok(self.register_crate(host_library, root, span, library, dep_kind, name))
@@ -484,7 +482,7 @@ impl<'a> CrateLoader<'a> {
                 DepKind::MacrosOnly => DepKind::MacrosOnly,
                 _ => dep.kind,
             };
-            self.resolve_crate(dep.name, span, dep_kind, Some((root, &dep))).0
+            self.resolve_crate(dep.name, span, dep_kind, Some((root, &dep)))
         })).collect()
     }
 
@@ -581,7 +579,8 @@ impl<'a> CrateLoader<'a> {
         };
         info!("panic runtime not found -- loading {}", name);
 
-        let (cnum, data) = self.resolve_crate(name, DUMMY_SP, DepKind::Implicit, None);
+        let cnum = self.resolve_crate(name, DUMMY_SP, DepKind::Implicit, None);
+        let data = self.cstore.get_crate_data(cnum);
 
         // Sanity check the loaded crate to ensure it is indeed a panic runtime
         // and the panic strategy is indeed what we thought it was.
@@ -685,7 +684,8 @@ impl<'a> CrateLoader<'a> {
                 });
                 info!("loading sanitizer: {}", name);
 
-                let data = self.resolve_crate(name, DUMMY_SP, DepKind::Explicit, None).1;
+                let cnum = self.resolve_crate(name, DUMMY_SP, DepKind::Explicit, None);
+                let data = self.cstore.get_crate_data(cnum);
 
                 // Sanity check the loaded crate to ensure it is indeed a sanitizer runtime
                 if !data.root.sanitizer_runtime {
@@ -705,7 +705,8 @@ impl<'a> CrateLoader<'a> {
             info!("loading profiler");
 
             let name = Symbol::intern("profiler_builtins");
-            let data = self.resolve_crate(name, DUMMY_SP, DepKind::Implicit, None).1;
+            let cnum = self.resolve_crate(name, DUMMY_SP, DepKind::Implicit, None);
+            let data = self.cstore.get_crate_data(cnum);
 
             // Sanity check the loaded crate to ensure it is indeed a profiler runtime
             if !data.root.profiler_runtime {
@@ -886,7 +887,7 @@ impl<'a> CrateLoader<'a> {
                     DepKind::Explicit
                 };
 
-                let cnum = self.resolve_crate(name, item.span, dep_kind, None).0;
+                let cnum = self.resolve_crate(name, item.span, dep_kind, None);
 
                 let def_id = definitions.opt_local_def_id(item.id).unwrap();
                 let path_len = definitions.def_path(def_id.index).data.len();
@@ -907,7 +908,7 @@ impl<'a> CrateLoader<'a> {
     }
 
     pub fn process_path_extern(&mut self, name: Symbol, span: Span) -> CrateNum {
-        let cnum = self.resolve_crate(name, span, DepKind::Explicit, None).0;
+        let cnum = self.resolve_crate(name, span, DepKind::Explicit, None);
 
         self.update_extern_crate(
             cnum,
@@ -925,7 +926,7 @@ impl<'a> CrateLoader<'a> {
     }
 
     pub fn maybe_process_path_extern(&mut self, name: Symbol, span: Span) -> Option<CrateNum> {
-        let cnum = self.maybe_resolve_crate(name, span, DepKind::Explicit, None).ok()?.0;
+        let cnum = self.maybe_resolve_crate(name, span, DepKind::Explicit, None).ok()?;
 
         self.update_extern_crate(
             cnum,
diff --git a/src/librustc_metadata/cstore.rs b/src/librustc_metadata/cstore.rs
@@ -54,7 +54,7 @@ crate struct CrateMetadata {
     /// hashmap, which gives the reverse mapping. This allows us to
     /// quickly retrace a `DefPath`, which is needed for incremental
     /// compilation support.
-    crate def_path_table: Lrc<DefPathTable>,
+    crate def_path_table: DefPathTable,
     /// Trait impl data.
     /// FIXME: Used only from queries and can use query cache,
     /// so pre-decoding can probably be avoided.
@@ -123,18 +123,18 @@ impl CStore {
         CrateNum::new(self.metas.len() - 1)
     }
 
-    crate fn get_crate_data(&self, cnum: CrateNum) -> Lrc<CrateMetadata> {
-        self.metas[cnum].clone()
+    crate fn get_crate_data(&self, cnum: CrateNum) -> &CrateMetadata {
+        self.metas[cnum].as_ref()
             .unwrap_or_else(|| panic!("Failed to get crate data for {:?}", cnum))
     }
 
-    crate fn set_crate_data(&mut self, cnum: CrateNum, data: Lrc<CrateMetadata>) {
+    crate fn set_crate_data(&mut self, cnum: CrateNum, data: CrateMetadata) {
         assert!(self.metas[cnum].is_none(), "Overwriting crate metadata entry");
-        self.metas[cnum] = Some(data);
+        self.metas[cnum] = Some(Lrc::new(data));
     }
 
     crate fn iter_crate_data<I>(&self, mut i: I)
-        where I: FnMut(CrateNum, &Lrc<CrateMetadata>)
+        where I: FnMut(CrateNum, &CrateMetadata)
     {
         for (k, v) in self.metas.iter_enumerated() {
             if let &Some(ref v) = v {
diff --git a/src/librustc_metadata/cstore_impl.rs b/src/librustc_metadata/cstore_impl.rs
@@ -53,7 +53,7 @@ macro_rules! provide {
                 let ($def_id, $other) = def_id_arg.into_args();
                 assert!(!$def_id.is_local());
 
-                let $cdata = $tcx.crate_data_as_rc_any($def_id.krate);
+                let $cdata = $tcx.crate_data_as_any($def_id.krate);
                 let $cdata = $cdata.downcast_ref::<cstore::CrateMetadata>()
                     .expect("CrateStore created data is not a CrateMetadata");
 
@@ -478,8 +478,8 @@ impl cstore::CStore {
 }
 
 impl CrateStore for cstore::CStore {
-    fn crate_data_as_rc_any(&self, krate: CrateNum) -> Lrc<dyn Any> {
-        self.get_crate_data(krate)
+    fn crate_data_as_any(&self, cnum: CrateNum) -> &dyn Any {
+        self.get_crate_data(cnum)
     }
 
     fn item_generics_cloned_untracked(&self, def: DefId, sess: &Session) -> ty::Generics {
@@ -520,8 +520,8 @@ impl CrateStore for cstore::CStore {
         self.get_crate_data(def.krate).def_path_hash(def.index)
     }
 
-    fn def_path_table(&self, cnum: CrateNum) -> Lrc<DefPathTable> {
-        self.get_crate_data(cnum).def_path_table.clone()
+    fn def_path_table(&self, cnum: CrateNum) -> &DefPathTable {
+        &self.get_crate_data(cnum).def_path_table
     }
 
     fn crates_untracked(&self) -> Vec<CrateNum>

Original file line number	Diff line number	Diff line change
`@@ -53,7 +53,7 @@ macro_rules! provide {`
`53`	`53`	`let ($def_id, $other) = def_id_arg.into_args();`
`54`	`54`	`assert!(!$def_id.is_local());`
`55`	`55`
`56`		`- let $cdata = $tcx.crate_data_as_rc_any($def_id.krate);`
	`56`	`+ let $cdata = $tcx.crate_data_as_any($def_id.krate);`
`57`	`57`	`let $cdata = $cdata.downcast_ref::<cstore::CrateMetadata>()`
`58`	`58`	`.expect("CrateStore created data is not a CrateMetadata");`
`59`	`59`
`@@ -478,8 +478,8 @@ impl cstore::CStore {`
`478`	`478`	`}`
`479`	`479`
`480`	`480`	`impl CrateStore for cstore::CStore {`
`481`		`- fn crate_data_as_rc_any(&self, krate: CrateNum) -> Lrc<dyn Any> {`
`482`		`- self.get_crate_data(krate)`
	`481`	`+ fn crate_data_as_any(&self, cnum: CrateNum) -> &dyn Any {`
	`482`	`+ self.get_crate_data(cnum)`
`483`	`483`	`}`
`484`	`484`
`485`	`485`	`fn item_generics_cloned_untracked(&self, def: DefId, sess: &Session) -> ty::Generics {`
`@@ -520,8 +520,8 @@ impl CrateStore for cstore::CStore {`
`520`	`520`	`self.get_crate_data(def.krate).def_path_hash(def.index)`
`521`	`521`	`}`
`522`	`522`
`523`		`- fn def_path_table(&self, cnum: CrateNum) -> Lrc<DefPathTable> {`
`524`		`- self.get_crate_data(cnum).def_path_table.clone()`
	`523`	`+ fn def_path_table(&self, cnum: CrateNum) -> &DefPathTable {`
	`524`	`+ &self.get_crate_data(cnum).def_path_table`
`525`	`525`	`}`
`526`	`526`
`527`	`527`	`fn crates_untracked(&self) -> Vec<CrateNum>`