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

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

Merged
merged 1 commit into from
Apr 5, 2023
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
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions crates/base-db/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
84 changes: 40 additions & 44 deletions crates/base-db/src/input.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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(
Expand All @@ -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(())
}

Expand All @@ -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) {
Expand All @@ -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))
Expand All @@ -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,
) {
Expand All @@ -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)
}
Expand All @@ -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>> {
Expand Down Expand Up @@ -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
Expand All @@ -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]
}
}

Expand Down
10 changes: 5 additions & 5 deletions crates/base-db/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand All @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions crates/hir-def/src/body/tests/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
"#]],
);
Expand Down
4 changes: 2 additions & 2 deletions crates/hir-def/src/test_db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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)
}
}
Expand Down
5 changes: 3 additions & 2 deletions crates/hir-ty/src/test_db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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)
}
}
Expand Down
3 changes: 1 addition & 2 deletions crates/ide-db/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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)
}
}
Expand Down
12 changes: 3 additions & 9 deletions crates/ide-db/src/test_data/test_symbol_index_collection.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,7 @@
(
Module {
id: ModuleId {
krate: CrateId(
0,
),
krate: Idx::<CrateData>(0),
block: None,
local_id: Idx::<ModuleData>(0),
},
Expand Down Expand Up @@ -381,9 +379,7 @@
(
Module {
id: ModuleId {
krate: CrateId(
0,
),
krate: Idx::<CrateData>(0),
block: None,
local_id: Idx::<ModuleData>(1),
},
Expand Down Expand Up @@ -412,9 +408,7 @@
(
Module {
id: ModuleId {
krate: CrateId(
0,
),
krate: Idx::<CrateData>(0),
block: None,
local_id: Idx::<ModuleData>(2),
},
Expand Down
Loading