From 20ff27e2ba112b32559916dcd93acdcc4b94e440 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Sat, 18 Jan 2025 12:02:34 +0100 Subject: [PATCH] Less allocs --- crates/ide-db/src/imports/import_assets.rs | 8 +++++++- crates/ide-db/src/items_locator.rs | 21 +++++++++++---------- crates/ide-db/src/symbol_index.rs | 20 ++++++++++++-------- 3 files changed, 30 insertions(+), 19 deletions(-) diff --git a/crates/ide-db/src/imports/import_assets.rs b/crates/ide-db/src/imports/import_assets.rs index 3ed101ff36..573018f9cc 100644 --- a/crates/ide-db/src/imports/import_assets.rs +++ b/crates/ide-db/src/imports/import_assets.rs @@ -1,5 +1,7 @@ //! Look up accessible paths for items. +use std::ops::ControlFlow; + use hir::{ db::HirDatabase, AsAssocItem, AssocItem, AssocItemContainer, Crate, HasCrate, ImportPathConfig, ItemInNs, ModPath, Module, ModuleDef, PathResolution, PrefixKind, ScopeDef, Semantics, @@ -353,6 +355,7 @@ fn path_applicable_imports( // we have some unresolved qualifier that we search an import for // The key here is that whatever we import must form a resolved path for the remainder of // what follows + // FIXME: This doesn't handle visibility [first_qsegment, qualifier_rest @ ..] => items_locator::items_with_name( sema, current_crate, @@ -417,8 +420,11 @@ fn validate_resolvable( module, candidate.clone(), AssocSearchMode::Exclude, + |it| match scope_filter(it) { + true => ControlFlow::Break(it), + false => ControlFlow::Continue(()), + }, ) - .find(|&it| scope_filter(it)) .map(|item| LocatedImport::new(import_path_candidate, resolved_qualifier, item)) } // FIXME diff --git a/crates/ide-db/src/items_locator.rs b/crates/ide-db/src/items_locator.rs index d6dcf7147f..405d2d91d8 100644 --- a/crates/ide-db/src/items_locator.rs +++ b/crates/ide-db/src/items_locator.rs @@ -2,6 +2,8 @@ //! by its name and a few criteria. //! The main reason for this module to exist is the fact that project's items and dependencies' items //! are located in different caches, with different APIs. +use std::ops::ControlFlow; + use either::Either; use hir::{import_map, Crate, ItemInNs, Module, Semantics}; use limit::Limit; @@ -17,6 +19,7 @@ pub static DEFAULT_QUERY_SEARCH_LIMIT: Limit = Limit::new(100); pub use import_map::AssocSearchMode; +// FIXME: Do callbacks instead to avoid allocations. /// Searches for importable items with the given name in the crate and its dependencies. pub fn items_with_name<'a>( sema: &'a Semantics<'_, RootDatabase>, @@ -70,12 +73,13 @@ pub fn items_with_name<'a>( } /// Searches for importable items with the given name in the crate and its dependencies. -pub fn items_with_name_in_module<'a>( - sema: &'a Semantics<'_, RootDatabase>, +pub fn items_with_name_in_module( + sema: &Semantics<'_, RootDatabase>, module: Module, name: NameToImport, assoc_item_search: AssocSearchMode, -) -> impl Iterator + 'a { + mut cb: impl FnMut(ItemInNs) -> ControlFlow, +) -> Option { let _p = tracing::info_span!("items_with_name_in", name = name.text(), assoc_item_search = ?assoc_item_search, ?module) .entered(); @@ -107,15 +111,12 @@ pub fn items_with_name_in_module<'a>( local_query } }; - let mut local_results = Vec::new(); - // FIXME: This using module_symbols is likely wrong? local_query.search(&[sema.db.module_symbols(module)], |local_candidate| { - local_results.push(match local_candidate.def { + cb(match local_candidate.def { hir::ModuleDef::Macro(macro_def) => ItemInNs::Macros(macro_def), def => ItemInNs::from(def), }) - }); - local_results.into_iter() + }) } fn find_items<'a>( @@ -140,10 +141,10 @@ fn find_items<'a>( // Query the local crate using the symbol index. let mut local_results = Vec::new(); local_query.search(&symbol_index::crate_symbols(db, krate), |local_candidate| { - local_results.push(match local_candidate.def { + ControlFlow::<()>::Continue(local_results.push(match local_candidate.def { hir::ModuleDef::Macro(macro_def) => ItemInNs::Macros(macro_def), def => ItemInNs::from(def), - }) + })) }); local_results.into_iter().chain(external_importables) } diff --git a/crates/ide-db/src/symbol_index.rs b/crates/ide-db/src/symbol_index.rs index 79f89a89a1..738db95133 100644 --- a/crates/ide-db/src/symbol_index.rs +++ b/crates/ide-db/src/symbol_index.rs @@ -25,6 +25,7 @@ use std::{ fmt, hash::{Hash, Hasher}, mem, + ops::ControlFlow, }; use base_db::{ @@ -219,7 +220,7 @@ pub fn world_symbols(db: &RootDatabase, query: Query) -> Vec { }; let mut res = vec![]; - query.search(&indices, |f| res.push(f.clone())); + query.search::<()>(&indices, |f| ControlFlow::Continue(res.push(f.clone()))); res } @@ -313,11 +314,11 @@ impl SymbolIndex { } impl Query { - pub(crate) fn search<'sym>( + pub(crate) fn search<'sym, T>( self, indices: &'sym [Arc], - cb: impl FnMut(&'sym FileSymbol), - ) { + cb: impl FnMut(&'sym FileSymbol) -> ControlFlow, + ) -> Option { let _p = tracing::info_span!("symbol_index::Query::search").entered(); let mut op = fst::map::OpBuilder::new(); match self.mode { @@ -348,12 +349,12 @@ impl Query { } } - fn search_maps<'sym>( + fn search_maps<'sym, T>( &self, indices: &'sym [Arc], mut stream: fst::map::Union<'_>, - mut cb: impl FnMut(&'sym FileSymbol), - ) { + mut cb: impl FnMut(&'sym FileSymbol) -> ControlFlow, + ) -> Option { let ignore_underscore_prefixed = !self.query.starts_with("__"); while let Some((_, indexed_values)) = stream.next() { for &IndexedValue { index, value } in indexed_values { @@ -379,11 +380,14 @@ impl Query { continue; } if self.mode.check(&self.query, self.case_sensitive, symbol_name) { - cb(symbol); + if let Some(b) = cb(symbol).break_value() { + return Some(b); + } } } } } + None } fn matches_assoc_mode(&self, is_trait_assoc_item: bool) -> bool {