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

minor: Speed up fst items lookup during completions #9206

Merged
merged 2 commits into from
Jun 10, 2021
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 crates/base_db/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ impl<T: SourceDatabaseExt> FileLoader for FileLoaderDelegate<&'_ T> {
}

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: 4 additions & 0 deletions crates/hir/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ impl Crate {
db: &dyn DefDatabase,
query: import_map::Query,
) -> impl Iterator<Item = Either<ModuleDef, MacroDef>> {
let _p = profile::span("query_external_importables");
import_map::search_dependencies(db, self.into(), query).into_iter().map(|item| match item {
ItemInNs::Types(mod_id) | ItemInNs::Values(mod_id) => Either::Left(mod_id.into()),
ItemInNs::Macros(mac_id) => Either::Right(mac_id.into()),
Expand Down Expand Up @@ -2185,6 +2186,7 @@ impl Type {
name: Option<&Name>,
mut callback: impl FnMut(&Ty, Function) -> Option<T>,
) -> Option<T> {
let _p = profile::span("iterate_method_candidates");
// There should be no inference vars in types passed here
// FIXME check that?
// FIXME replace Unknown by bound vars here
Expand Down Expand Up @@ -2218,6 +2220,7 @@ impl Type {
name: Option<&Name>,
mut callback: impl FnMut(&Ty, AssocItem) -> Option<T>,
) -> Option<T> {
let _p = profile::span("iterate_path_candidates");
let canonical = hir_ty::replace_errors_with_variables(&self.ty);

let env = self.env.clone();
Expand Down Expand Up @@ -2255,6 +2258,7 @@ impl Type {
&'a self,
db: &'a dyn HirDatabase,
) -> impl Iterator<Item = Trait> + 'a {
let _p = profile::span("applicable_inherent_traits");
self.autoderef(db)
.filter_map(|derefed_type| derefed_type.ty.dyn_trait())
.flat_map(move |dyn_trait_id| hir_ty::all_super_traits(db.upcast(), dyn_trait_id))
Expand Down
169 changes: 87 additions & 82 deletions crates/hir_def/src/import_map.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! A map of all publicly exported items in a crate.

use std::{cmp::Ordering, fmt, hash::BuildHasherDefault, sync::Arc};
use std::{fmt, hash::BuildHasherDefault, sync::Arc};

use base_db::CrateId;
use fst::{self, Streamer};
Expand Down Expand Up @@ -69,95 +69,25 @@ pub struct ImportMap {
impl ImportMap {
pub fn import_map_query(db: &dyn DefDatabase, krate: CrateId) -> Arc<Self> {
let _p = profile::span("import_map_query");
let def_map = db.crate_def_map(krate);
let mut import_map = Self::default();

// We look only into modules that are public(ly reexported), starting with the crate root.
let empty = ImportPath { segments: vec![] };
let root = def_map.module_id(def_map.root());
let mut worklist = vec![(root, empty)];
while let Some((module, mod_path)) = worklist.pop() {
let ext_def_map;
let mod_data = if module.krate == krate {
&def_map[module.local_id]
} else {
// The crate might reexport a module defined in another crate.
ext_def_map = module.def_map(db);
&ext_def_map[module.local_id]
};

let visible_items = mod_data.scope.entries().filter_map(|(name, per_ns)| {
let per_ns = per_ns.filter_visibility(|vis| vis == Visibility::Public);
if per_ns.is_none() {
None
} else {
Some((name, per_ns))
}
});

for (name, per_ns) in visible_items {
let mk_path = || {
let mut path = mod_path.clone();
path.segments.push(name.clone());
path
};

for item in per_ns.iter_items() {
let path = mk_path();
let path_len = path.len();
let import_info =
ImportInfo { path, container: module, is_trait_assoc_item: false };

if let Some(ModuleDefId::TraitId(tr)) = item.as_module_def_id() {
import_map.collect_trait_assoc_items(
db,
tr,
matches!(item, ItemInNs::Types(_)),
&import_info,
);
}

match import_map.map.entry(item) {
Entry::Vacant(entry) => {
entry.insert(import_info);
}
Entry::Occupied(mut entry) => {
// If the new path is shorter, prefer that one.
if path_len < entry.get().path.len() {
*entry.get_mut() = import_info;
} else {
continue;
}
}
}

// If we've just added a path to a module, descend into it. We might traverse
// modules multiple times, but only if the new path to it is shorter than the
// first (else we `continue` above).
if let Some(ModuleDefId::ModuleId(mod_id)) = item.as_module_def_id() {
worklist.push((mod_id, mk_path()));
}
}
}
}
let mut import_map = collect_import_map(db, krate);

let mut importables = import_map.map.iter().collect::<Vec<_>>();

importables.sort_by(cmp);
importables.sort_by_cached_key(|(_, import_info)| fst_path(&import_info.path));

// Build the FST, taking care not to insert duplicate values.

let mut builder = fst::MapBuilder::memory();
let mut last_batch_start = 0;

for idx in 0..importables.len() {
if let Some(next_item) = importables.get(idx + 1) {
if cmp(&importables[last_batch_start], next_item) == Ordering::Equal {
let key = fst_path(&importables[last_batch_start].1.path);
if let Some((_, next_import_info)) = importables.get(idx + 1) {
if key == fst_path(&next_import_info.path) {
continue;
}
}

let key = fst_path(&importables[last_batch_start].1.path);
builder.insert(key, last_batch_start as u64).unwrap();

last_batch_start = idx + 1;
Expand Down Expand Up @@ -185,6 +115,7 @@ impl ImportMap {
is_type_in_ns: bool,
original_import_info: &ImportInfo,
) {
let _p = profile::span("collect_trait_assoc_items");
for (assoc_item_name, item) in &db.trait_data(tr).items {
let module_def_id = match item {
AssocItemId::FunctionId(f) => ModuleDefId::from(*f),
Expand All @@ -210,6 +141,84 @@ impl ImportMap {
}
}

fn collect_import_map(db: &dyn DefDatabase, krate: CrateId) -> ImportMap {
let _p = profile::span("collect_import_map");

let def_map = db.crate_def_map(krate);
let mut import_map = ImportMap::default();

// We look only into modules that are public(ly reexported), starting with the crate root.
let empty = ImportPath { segments: vec![] };
let root = def_map.module_id(def_map.root());
let mut worklist = vec![(root, empty)];
while let Some((module, mod_path)) = worklist.pop() {
let ext_def_map;
let mod_data = if module.krate == krate {
&def_map[module.local_id]
} else {
// The crate might reexport a module defined in another crate.
ext_def_map = module.def_map(db);
&ext_def_map[module.local_id]
};

let visible_items = mod_data.scope.entries().filter_map(|(name, per_ns)| {
let per_ns = per_ns.filter_visibility(|vis| vis == Visibility::Public);
if per_ns.is_none() {
None
} else {
Some((name, per_ns))
}
});

for (name, per_ns) in visible_items {
let mk_path = || {
let mut path = mod_path.clone();
path.segments.push(name.clone());
path
};

for item in per_ns.iter_items() {
let path = mk_path();
let path_len = path.len();
let import_info =
ImportInfo { path, container: module, is_trait_assoc_item: false };

if let Some(ModuleDefId::TraitId(tr)) = item.as_module_def_id() {
import_map.collect_trait_assoc_items(
db,
tr,
matches!(item, ItemInNs::Types(_)),
&import_info,
);
}

match import_map.map.entry(item) {
Entry::Vacant(entry) => {
entry.insert(import_info);
}
Entry::Occupied(mut entry) => {
// If the new path is shorter, prefer that one.
if path_len < entry.get().path.len() {
*entry.get_mut() = import_info;
} else {
continue;
}
}
}

// If we've just added a path to a module, descend into it. We might traverse
// modules multiple times, but only if the new path to it is shorter than the
// first (else we `continue` above).
if let Some(ModuleDefId::ModuleId(mod_id)) = item.as_module_def_id() {
worklist.push((mod_id, mk_path()));
}
}
}
}

import_map
}

impl PartialEq for ImportMap {
fn eq(&self, other: &Self) -> bool {
// `fst` and `importables` are built from `map`, so we don't need to compare them.
Expand Down Expand Up @@ -240,17 +249,12 @@ impl fmt::Debug for ImportMap {
}

fn fst_path(path: &ImportPath) -> String {
let _p = profile::span("fst_path");
let mut s = path.to_string();
s.make_ascii_lowercase();
s
}

fn cmp((_, lhs): &(&ItemInNs, &ImportInfo), (_, rhs): &(&ItemInNs, &ImportInfo)) -> Ordering {
let lhs_str = fst_path(&lhs.path);
let rhs_str = fst_path(&rhs.path);
lhs_str.cmp(&rhs_str)
}

#[derive(Debug, Eq, PartialEq, Hash)]
pub enum ImportKind {
Module,
Expand Down Expand Up @@ -338,6 +342,7 @@ impl Query {
}

fn import_matches(&self, import: &ImportInfo, enforce_lowercase: bool) -> bool {
let _p = profile::span("import_map::Query::import_matches");
if import.is_trait_assoc_item {
if self.exclude_import_kinds.contains(&ImportKind::AssociatedItem) {
return false;
Expand Down
1 change: 1 addition & 0 deletions crates/hir_def/src/lang_item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ impl LangItems {
) where
T: Into<AttrDefId> + Copy,
{
let _p = profile::span("collect_lang_item");
if let Some(lang_item_name) = lang_attr(db, item) {
self.items.entry(lang_item_name).or_insert_with(|| constructor(item));
}
Expand Down
2 changes: 2 additions & 0 deletions crates/hir_def/src/per_ns.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ impl PerNs {
}

pub fn filter_visibility(self, mut f: impl FnMut(Visibility) -> bool) -> PerNs {
let _p = profile::span("PerNs::filter_visibility");
PerNs {
types: self.types.filter(|(_, v)| f(*v)),
values: self.values.filter(|(_, v)| f(*v)),
Expand All @@ -86,6 +87,7 @@ impl PerNs {
}

pub fn iter_items(self) -> impl Iterator<Item = ItemInNs> {
let _p = profile::span("PerNs::iter_items");
self.types
.map(|it| ItemInNs::Types(it.0))
.into_iter()
Expand Down
2 changes: 2 additions & 0 deletions crates/ide_db/src/symbol_index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ pub fn world_symbols(db: &RootDatabase, query: Query) -> Vec<FileSymbol> {
}

pub fn crate_symbols(db: &RootDatabase, krate: CrateId, query: Query) -> Vec<FileSymbol> {
let _p = profile::span("crate_symbols").detail(|| format!("{:?}", query));
// FIXME(#4842): This now depends on CrateDefMap, why not build the entire symbol index from
// that instead?

Expand Down Expand Up @@ -321,6 +322,7 @@ impl SymbolIndex {

impl Query {
pub(crate) fn search(self, indices: &[&SymbolIndex]) -> Vec<FileSymbol> {
let _p = profile::span("symbol_index::Query::search");
let mut op = fst::map::OpBuilder::new();
for file_symbols in indices.iter() {
let automaton = fst::automaton::Subsequence::new(&self.lowercased);
Expand Down