Auto merge of rust-lang#14490 - Veykril:crategraph-dedup, r=Veykril

internal: Switch crate graph to use an Arena instead of a hashmap

Author: bors

Cargo.lock

@@ -15,6 +15,8 @@ doctest = false
 salsa = "0.17.0-pre.2"
 rustc-hash = "1.1.0"
 
+la-arena = { version = "0.3.0", path = "../../lib/la-arena" }
+
 # local deps
 cfg.workspace = true
 profile.workspace = true

crates/base-db/Cargo.toml

@@ -9,8 +9,8 @@
 use std::{fmt, mem, ops, panic::RefUnwindSafe, str::FromStr, sync::Arc};
 
 use cfg::CfgOptions;
-use rustc_hash::FxHashMap;
-use stdx::hash::{NoHashHashMap, NoHashHashSet};
+use la_arena::{Arena, Idx, RawIdx};
+use rustc_hash::{FxHashMap, FxHashSet};
 use syntax::SmolStr;
 use tt::token_id::Subtree;
 use vfs::{file_set::FileSet, AbsPathBuf, AnchoredPath, FileId, VfsPath};
@@ -84,17 +84,22 @@ impl SourceRoot {
 ///
 /// `CrateGraph` is `!Serialize` by design, see
 /// <https://github.com/rust-lang/rust-analyzer/blob/master/docs/dev/architecture.md#serialization>
-#[derive(Debug, Clone, Default /* Serialize, Deserialize */)]
+#[derive(Clone, Default)]
 pub struct CrateGraph {
-    arena: NoHashHashMap<CrateId, CrateData>,
+    arena: Arena<CrateData>,
 }
 
-#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
-pub struct CrateId(pub u32);
+impl fmt::Debug for CrateGraph {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        f.debug_map()
+            .entries(self.arena.iter().map(|(id, data)| (u32::from(id.into_raw()), data)))
+            .finish()
+    }
+}
 
-impl stdx::hash::NoHashHashable for CrateId {}
+pub type CrateId = Idx<CrateData>;
 
-#[derive(Debug, Clone, PartialEq, Eq, Hash)]
+#[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
 pub struct CrateName(SmolStr);
 
 impl CrateName {
@@ -182,7 +187,7 @@ impl fmt::Display for LangCrateOrigin {
     }
 }
 
-#[derive(Debug, Clone, PartialEq, Eq, Hash)]
+#[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
 pub struct CrateDisplayName {
     // The name we use to display various paths (with `_`).
     crate_name: CrateName,
@@ -261,7 +266,7 @@ pub struct ProcMacro {
     pub expander: Arc<dyn ProcMacroExpander>,
 }
 
-#[derive(Debug, Copy, Clone)]
+#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
 pub enum ReleaseChannel {
     Stable,
     Beta,
@@ -287,7 +292,7 @@ impl ReleaseChannel {
     }
 }
 
-#[derive(Debug, Clone)]
+#[derive(Debug, Clone, PartialEq, Eq)]
 pub struct CrateData {
     pub root_file_id: FileId,
     pub edition: Edition,
@@ -327,7 +332,7 @@ pub struct Env {
     entries: FxHashMap<String, String>,
 }
 
-#[derive(Debug, Clone, PartialEq, Eq)]
+#[derive(Debug, Clone, PartialEq, Eq, Hash)]
 pub struct Dependency {
     pub crate_id: CrateId,
     pub name: CrateName,
@@ -378,10 +383,7 @@ impl CrateGraph {
             is_proc_macro,
             channel,
         };
-        let crate_id = CrateId(self.arena.len() as u32);
-        let prev = self.arena.insert(crate_id, data);
-        assert!(prev.is_none());
-        crate_id
+        self.arena.alloc(data)
     }
 
     pub fn add_dep(
@@ -394,14 +396,14 @@ impl CrateGraph {
         // Check if adding a dep from `from` to `to` creates a cycle. To figure
         // that out, look for a  path in the *opposite* direction, from `to` to
         // `from`.
-        if let Some(path) = self.find_path(&mut NoHashHashSet::default(), dep.crate_id, from) {
+        if let Some(path) = self.find_path(&mut FxHashSet::default(), dep.crate_id, from) {
             let path = path.into_iter().map(|it| (it, self[it].display_name.clone())).collect();
             let err = CyclicDependenciesError { path };
             assert!(err.from().0 == from && err.to().0 == dep.crate_id);
             return Err(err);
         }
 
-        self.arena.get_mut(&from).unwrap().add_dep(dep);
+        self.arena[from].add_dep(dep);
         Ok(())
     }
 
@@ -410,14 +412,14 @@ impl CrateGraph {
     }
 
     pub fn iter(&self) -> impl Iterator<Item = CrateId> + '_ {
-        self.arena.keys().copied()
+        self.arena.iter().map(|(idx, _)| idx)
     }
 
     /// Returns an iterator over all transitive dependencies of the given crate,
     /// including the crate itself.
     pub fn transitive_deps(&self, of: CrateId) -> impl Iterator<Item = CrateId> {
         let mut worklist = vec![of];
-        let mut deps = NoHashHashSet::default();
+        let mut deps = FxHashSet::default();
 
         while let Some(krate) = worklist.pop() {
             if !deps.insert(krate) {
@@ -434,11 +436,11 @@ impl CrateGraph {
     /// including the crate itself.
     pub fn transitive_rev_deps(&self, of: CrateId) -> impl Iterator<Item = CrateId> {
         let mut worklist = vec![of];
-        let mut rev_deps = NoHashHashSet::default();
+        let mut rev_deps = FxHashSet::default();
         rev_deps.insert(of);
 
-        let mut inverted_graph = NoHashHashMap::<_, Vec<_>>::default();
-        self.arena.iter().for_each(|(&krate, data)| {
+        let mut inverted_graph = FxHashMap::<_, Vec<_>>::default();
+        self.arena.iter().for_each(|(krate, data)| {
             data.dependencies
                 .iter()
                 .for_each(|dep| inverted_graph.entry(dep.crate_id).or_default().push(krate))
@@ -461,17 +463,17 @@ impl CrateGraph {
     /// come before the crate itself).
     pub fn crates_in_topological_order(&self) -> Vec<CrateId> {
         let mut res = Vec::new();
-        let mut visited = NoHashHashSet::default();
+        let mut visited = FxHashSet::default();
 
-        for krate in self.arena.keys().copied() {
+        for krate in self.iter() {
             go(self, &mut visited, &mut res, krate);
         }
 
         return res;
 
         fn go(
             graph: &CrateGraph,
-            visited: &mut NoHashHashSet<CrateId>,
+            visited: &mut FxHashSet<CrateId>,
             res: &mut Vec<CrateId>,
             source: CrateId,
         ) {
@@ -487,7 +489,7 @@ impl CrateGraph {
 
     // FIXME: this only finds one crate with the given root; we could have multiple
     pub fn crate_id_for_crate_root(&self, file_id: FileId) -> Option<CrateId> {
-        let (&crate_id, _) =
+        let (crate_id, _) =
             self.arena.iter().find(|(_crate_id, data)| data.root_file_id == file_id)?;
         Some(crate_id)
     }
@@ -499,24 +501,26 @@ impl CrateGraph {
     /// amount.
     pub fn extend(&mut self, other: CrateGraph, proc_macros: &mut ProcMacroPaths) -> u32 {
         let start = self.arena.len() as u32;
-        self.arena.extend(other.arena.into_iter().map(|(id, mut data)| {
-            let new_id = id.shift(start);
+        self.arena.extend(other.arena.into_iter().map(|(_, mut data)| {
             for dep in &mut data.dependencies {
-                dep.crate_id = dep.crate_id.shift(start);
+                dep.crate_id =
+                    CrateId::from_raw(RawIdx::from(u32::from(dep.crate_id.into_raw()) + start));
             }
-            (new_id, data)
+            data
         }));
 
         *proc_macros = mem::take(proc_macros)
             .into_iter()
-            .map(|(id, macros)| (id.shift(start), macros))
+            .map(|(id, macros)| {
+                (CrateId::from_raw(RawIdx::from(u32::from(id.into_raw()) + start)), macros)
+            })
             .collect();
         start
     }
 
     fn find_path(
         &self,
-        visited: &mut NoHashHashSet<CrateId>,
+        visited: &mut FxHashSet<CrateId>,
         from: CrateId,
         to: CrateId,
     ) -> Option<Vec<CrateId>> {
@@ -546,10 +550,8 @@ impl CrateGraph {
         let std = self.hacky_find_crate("std");
         match (cfg_if, std) {
             (Some(cfg_if), Some(std)) => {
-                self.arena.get_mut(&cfg_if).unwrap().dependencies.clear();
-                self.arena
-                    .get_mut(&std)
-                    .unwrap()
+                self.arena[cfg_if].dependencies.clear();
+                self.arena[std]
                     .dependencies
                     .push(Dependency::new(CrateName::new("cfg_if").unwrap(), cfg_if));
                 true
@@ -566,13 +568,7 @@ impl CrateGraph {
 impl ops::Index<CrateId> for CrateGraph {
     type Output = CrateData;
     fn index(&self, crate_id: CrateId) -> &CrateData {
-        &self.arena[&crate_id]
-    }
-}
-
-impl CrateId {
-    fn shift(self, amount: u32) -> CrateId {
-        CrateId(self.0 + amount)
+        &self.arena[crate_id]
     }
 }
 

crates/base-db/src/input.rs

@@ -8,7 +8,7 @@ pub mod fixture;
 
 use std::{panic, sync::Arc};
 
-use stdx::hash::NoHashHashSet;
+use rustc_hash::FxHashSet;
 use syntax::{ast, Parse, SourceFile, TextRange, TextSize};
 
 pub use crate::{
@@ -59,7 +59,7 @@ pub trait FileLoader {
     /// Text of the file.
     fn file_text(&self, file_id: FileId) -> Arc<String>;
     fn resolve_path(&self, path: AnchoredPath<'_>) -> Option<FileId>;
-    fn relevant_crates(&self, file_id: FileId) -> Arc<NoHashHashSet<CrateId>>;
+    fn relevant_crates(&self, file_id: FileId) -> Arc<FxHashSet<CrateId>>;
 }
 
 /// Database which stores all significant input facts: source code and project
@@ -99,10 +99,10 @@ pub trait SourceDatabaseExt: SourceDatabase {
     #[salsa::input]
     fn source_root(&self, id: SourceRootId) -> Arc<SourceRoot>;
 
-    fn source_root_crates(&self, id: SourceRootId) -> Arc<NoHashHashSet<CrateId>>;
+    fn source_root_crates(&self, id: SourceRootId) -> Arc<FxHashSet<CrateId>>;
 }
 
-fn source_root_crates(db: &dyn SourceDatabaseExt, id: SourceRootId) -> Arc<NoHashHashSet<CrateId>> {
+fn source_root_crates(db: &dyn SourceDatabaseExt, id: SourceRootId) -> Arc<FxHashSet<CrateId>> {
     let graph = db.crate_graph();
     let res = graph
         .iter()
@@ -128,7 +128,7 @@ impl<T: SourceDatabaseExt> FileLoader for FileLoaderDelegate<&'_ T> {
         source_root.resolve_path(path)
     }
 
-    fn relevant_crates(&self, file_id: FileId) -> Arc<NoHashHashSet<CrateId>> {
+    fn relevant_crates(&self, file_id: FileId) -> Arc<FxHashSet<CrateId>> {
         let _p = profile::span("relevant_crates");
         let source_root = self.0.file_source_root(file_id);
         self.0.source_root_crates(source_root)

crates/base-db/src/lib.rs

@@ -148,8 +148,8 @@ fn f() {
 }
     "#,
         expect![[r#"
-            BlockId(1) in ModuleId { krate: CrateId(0), block: Some(BlockId(0)), local_id: Idx::<ModuleData>(1) }
-            BlockId(0) in ModuleId { krate: CrateId(0), block: None, local_id: Idx::<ModuleData>(0) }
+            BlockId(1) in ModuleId { krate: Idx::<CrateData>(0), block: Some(BlockId(0)), local_id: Idx::<ModuleData>(1) }
+            BlockId(0) in ModuleId { krate: Idx::<CrateData>(0), block: None, local_id: Idx::<ModuleData>(0) }
             crate scope
         "#]],
     );

crates/hir-def/src/body/tests/block.rs

@@ -11,7 +11,7 @@ use base_db::{
     Upcast,
 };
 use hir_expand::{db::ExpandDatabase, InFile};
-use stdx::hash::NoHashHashSet;
+use rustc_hash::FxHashSet;
 use syntax::{algo, ast, AstNode};
 
 use crate::{
@@ -77,7 +77,7 @@ impl FileLoader for TestDB {
     fn resolve_path(&self, path: AnchoredPath<'_>) -> Option<FileId> {
         FileLoaderDelegate(self).resolve_path(path)
     }
-    fn relevant_crates(&self, file_id: FileId) -> Arc<NoHashHashSet<CrateId>> {
+    fn relevant_crates(&self, file_id: FileId) -> Arc<FxHashSet<CrateId>> {
         FileLoaderDelegate(self).relevant_crates(file_id)
     }
 }

crates/hir-def/src/test_db.rs

@@ -11,7 +11,8 @@ use base_db::{
 };
 use hir_def::{db::DefDatabase, ModuleId};
 use hir_expand::db::ExpandDatabase;
-use stdx::hash::{NoHashHashMap, NoHashHashSet};
+use rustc_hash::FxHashSet;
+use stdx::hash::NoHashHashMap;
 use syntax::TextRange;
 use test_utils::extract_annotations;
 
@@ -81,7 +82,7 @@ impl FileLoader for TestDB {
     fn resolve_path(&self, path: AnchoredPath<'_>) -> Option<FileId> {
         FileLoaderDelegate(self).resolve_path(path)
     }
-    fn relevant_crates(&self, file_id: FileId) -> Arc<NoHashHashSet<CrateId>> {
+    fn relevant_crates(&self, file_id: FileId) -> Arc<FxHashSet<CrateId>> {
         FileLoaderDelegate(self).relevant_crates(file_id)
     }
 }

crates/hir-ty/src/test_db.rs

@@ -53,7 +53,6 @@ use hir::{
     db::{DefDatabase, ExpandDatabase, HirDatabase},
     symbols::FileSymbolKind,
 };
-use stdx::hash::NoHashHashSet;
 
 use crate::{line_index::LineIndex, symbol_index::SymbolsDatabase};
 pub use rustc_hash::{FxHashMap, FxHashSet, FxHasher};
@@ -120,7 +119,7 @@ impl FileLoader for RootDatabase {
     fn resolve_path(&self, path: AnchoredPath<'_>) -> Option<FileId> {
         FileLoaderDelegate(self).resolve_path(path)
     }
-    fn relevant_crates(&self, file_id: FileId) -> Arc<NoHashHashSet<CrateId>> {
+    fn relevant_crates(&self, file_id: FileId) -> Arc<FxHashSet<CrateId>> {
         FileLoaderDelegate(self).relevant_crates(file_id)
     }
 }

crates/ide-db/src/lib.rs

@@ -2,9 +2,7 @@
     (
         Module {
             id: ModuleId {
-                krate: CrateId(
-                    0,
-                ),
+                krate: Idx::<CrateData>(0),
                 block: None,
                 local_id: Idx::<ModuleData>(0),
             },
@@ -381,9 +379,7 @@
     (
         Module {
             id: ModuleId {
-                krate: CrateId(
-                    0,
-                ),
+                krate: Idx::<CrateData>(0),
                 block: None,
                 local_id: Idx::<ModuleData>(1),
             },
@@ -412,9 +408,7 @@
     (
         Module {
             id: ModuleId {
-                krate: CrateId(
-                    0,
-                ),
+                krate: Idx::<CrateData>(0),
                 block: None,
                 local_id: Idx::<ModuleData>(2),
             },