From 2e03b198cacf177a47402254b9ab702e1de50b11 Mon Sep 17 00:00:00 2001 From: Lukas Wirth Date: Sat, 13 May 2023 17:41:09 +0200 Subject: [PATCH] fix: Fix perf regression from symbol index refactor --- crates/hir/src/symbols.rs | 99 +++++- .../test_symbol_index_collection.txt | 314 ++++++++++++++++++ crates/ide/src/navigation_target.rs | 34 +- 3 files changed, 434 insertions(+), 13 deletions(-) diff --git a/crates/hir/src/symbols.rs b/crates/hir/src/symbols.rs index 386758ccab4..5d68aa52e62 100644 --- a/crates/hir/src/symbols.rs +++ b/crates/hir/src/symbols.rs @@ -1,12 +1,15 @@ //! File symbol extraction. +use base_db::FileRange; use hir_def::{ - AdtId, AssocItemId, DefWithBodyId, HasModule, ImplId, MacroId, ModuleDefId, ModuleId, TraitId, + src::HasSource, AdtId, AssocItemId, DefWithBodyId, HasModule, ImplId, Lookup, MacroId, + ModuleDefId, ModuleId, TraitId, }; +use hir_expand::{HirFileId, InFile}; use hir_ty::db::HirDatabase; -use syntax::SmolStr; +use syntax::{ast::HasName, AstNode, SmolStr, SyntaxNode, SyntaxNodePtr}; -use crate::{Module, ModuleDef}; +use crate::{Module, ModuleDef, Semantics}; /// The actual data that is stored in the index. It should be as compact as /// possible. @@ -15,6 +18,45 @@ pub struct FileSymbol { // even though name can be derived from the def, we store it for efficiency pub name: SmolStr, pub def: ModuleDef, + pub loc: DeclarationLocation, + pub container_name: Option, +} + +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub struct DeclarationLocation { + /// The file id for both the `ptr` and `name_ptr`. + pub hir_file_id: HirFileId, + /// This points to the whole syntax node of the declaration. + pub ptr: SyntaxNodePtr, + /// This points to the [`syntax::ast::Name`] identifier of the declaration. + pub name_ptr: SyntaxNodePtr, +} + +impl DeclarationLocation { + pub fn syntax(&self, sema: &Semantics<'_, DB>) -> SyntaxNode { + let root = sema.parse_or_expand(self.hir_file_id); + self.ptr.to_node(&root) + } + + pub fn original_range(&self, db: &dyn HirDatabase) -> FileRange { + let node = resolve_node(db, self.hir_file_id, &self.ptr); + node.as_ref().original_file_range(db.upcast()) + } + + pub fn original_name_range(&self, db: &dyn HirDatabase) -> Option { + let node = resolve_node(db, self.hir_file_id, &self.name_ptr); + node.as_ref().original_file_range_opt(db.upcast()) + } +} + +fn resolve_node( + db: &dyn HirDatabase, + file_id: HirFileId, + ptr: &SyntaxNodePtr, +) -> InFile { + let root = db.parse_or_expand(file_id); + let node = ptr.to_node(&root); + InFile::new(file_id, node) } /// Represents an outstanding module that the symbol collector must collect symbols from. @@ -193,17 +235,52 @@ fn push_assoc_item(&mut self, assoc_item_id: AssocItemId) { } } - fn push_decl(&mut self, id: impl Into) { - let def = ModuleDef::from(id.into()); - if let Some(name) = def.name(self.db) { - self.symbols.push(FileSymbol { name: name.to_smol_str(), def }); - } + fn push_decl(&mut self, id: L) + where + L: Lookup + Into, + ::Data: HasSource, + <::Data as HasSource>::Value: HasName, + { + self.push_file_symbol(|s| { + let loc = id.lookup(s.db.upcast()); + let source = loc.source(s.db.upcast()); + let name_node = source.value.name()?; + Some(FileSymbol { + name: name_node.text().into(), + def: ModuleDef::from(id.into()), + container_name: s.current_container_name.clone(), + loc: DeclarationLocation { + hir_file_id: source.file_id, + ptr: SyntaxNodePtr::new(source.value.syntax()), + name_ptr: SyntaxNodePtr::new(name_node.syntax()), + }, + }) + }) } fn push_module(&mut self, module_id: ModuleId) { - let def = Module::from(module_id); - if let Some(name) = def.name(self.db) { - self.symbols.push(FileSymbol { name: name.to_smol_str(), def: ModuleDef::Module(def) }); + self.push_file_symbol(|s| { + let def_map = module_id.def_map(s.db.upcast()); + let module_data = &def_map[module_id.local_id]; + let declaration = module_data.origin.declaration()?; + let module = declaration.to_node(s.db.upcast()); + let name_node = module.name()?; + Some(FileSymbol { + name: name_node.text().into(), + def: ModuleDef::Module(module_id.into()), + container_name: s.current_container_name.clone(), + loc: DeclarationLocation { + hir_file_id: declaration.file_id, + ptr: SyntaxNodePtr::new(module.syntax()), + name_ptr: SyntaxNodePtr::new(name_node.syntax()), + }, + }) + }) + } + + fn push_file_symbol(&mut self, f: impl FnOnce(&Self) -> Option) { + if let Some(file_symbol) = f(self) { + self.symbols.push(file_symbol); } } } diff --git a/crates/ide-db/src/test_data/test_symbol_index_collection.txt b/crates/ide-db/src/test_data/test_symbol_index_collection.txt index 1223e8d6b69..1e34dd633c8 100644 --- a/crates/ide-db/src/test_data/test_symbol_index_collection.txt +++ b/crates/ide-db/src/test_data/test_symbol_index_collection.txt @@ -17,6 +17,20 @@ ), }, ), + loc: DeclarationLocation { + hir_file_id: HirFileId( + 0, + ), + ptr: SyntaxNodePtr { + kind: TYPE_ALIAS, + range: 397..417, + }, + name_ptr: SyntaxNodePtr { + kind: NAME, + range: 402..407, + }, + }, + container_name: None, }, FileSymbol { name: "CONST", @@ -27,6 +41,20 @@ ), }, ), + loc: DeclarationLocation { + hir_file_id: HirFileId( + 0, + ), + ptr: SyntaxNodePtr { + kind: CONST, + range: 340..361, + }, + name_ptr: SyntaxNodePtr { + kind: NAME, + range: 346..351, + }, + }, + container_name: None, }, FileSymbol { name: "CONST_WITH_INNER", @@ -37,6 +65,20 @@ ), }, ), + loc: DeclarationLocation { + hir_file_id: HirFileId( + 0, + ), + ptr: SyntaxNodePtr { + kind: CONST, + range: 520..592, + }, + name_ptr: SyntaxNodePtr { + kind: NAME, + range: 526..542, + }, + }, + container_name: None, }, FileSymbol { name: "Enum", @@ -49,6 +91,20 @@ }, ), ), + loc: DeclarationLocation { + hir_file_id: HirFileId( + 0, + ), + ptr: SyntaxNodePtr { + kind: ENUM, + range: 185..207, + }, + name_ptr: SyntaxNodePtr { + kind: NAME, + range: 190..194, + }, + }, + container_name: None, }, FileSymbol { name: "Macro", @@ -61,6 +117,20 @@ ), }, ), + loc: DeclarationLocation { + hir_file_id: HirFileId( + 0, + ), + ptr: SyntaxNodePtr { + kind: MACRO_DEF, + range: 153..168, + }, + name_ptr: SyntaxNodePtr { + kind: NAME, + range: 159..164, + }, + }, + container_name: None, }, FileSymbol { name: "STATIC", @@ -71,6 +141,20 @@ ), }, ), + loc: DeclarationLocation { + hir_file_id: HirFileId( + 0, + ), + ptr: SyntaxNodePtr { + kind: STATIC, + range: 362..396, + }, + name_ptr: SyntaxNodePtr { + kind: NAME, + range: 369..375, + }, + }, + container_name: None, }, FileSymbol { name: "Struct", @@ -83,6 +167,20 @@ }, ), ), + loc: DeclarationLocation { + hir_file_id: HirFileId( + 0, + ), + ptr: SyntaxNodePtr { + kind: STRUCT, + range: 170..184, + }, + name_ptr: SyntaxNodePtr { + kind: NAME, + range: 177..183, + }, + }, + container_name: None, }, FileSymbol { name: "StructFromMacro", @@ -95,6 +193,20 @@ }, ), ), + loc: DeclarationLocation { + hir_file_id: HirFileId( + 2147483648, + ), + ptr: SyntaxNodePtr { + kind: STRUCT, + range: 0..22, + }, + name_ptr: SyntaxNodePtr { + kind: NAME, + range: 6..21, + }, + }, + container_name: None, }, FileSymbol { name: "StructInFn", @@ -107,6 +219,22 @@ }, ), ), + loc: DeclarationLocation { + hir_file_id: HirFileId( + 0, + ), + ptr: SyntaxNodePtr { + kind: STRUCT, + range: 318..336, + }, + name_ptr: SyntaxNodePtr { + kind: NAME, + range: 325..335, + }, + }, + container_name: Some( + "main", + ), }, FileSymbol { name: "StructInNamedConst", @@ -119,6 +247,22 @@ }, ), ), + loc: DeclarationLocation { + hir_file_id: HirFileId( + 0, + ), + ptr: SyntaxNodePtr { + kind: STRUCT, + range: 555..581, + }, + name_ptr: SyntaxNodePtr { + kind: NAME, + range: 562..580, + }, + }, + container_name: Some( + "CONST_WITH_INNER", + ), }, FileSymbol { name: "StructInUnnamedConst", @@ -131,6 +275,20 @@ }, ), ), + loc: DeclarationLocation { + hir_file_id: HirFileId( + 0, + ), + ptr: SyntaxNodePtr { + kind: STRUCT, + range: 479..507, + }, + name_ptr: SyntaxNodePtr { + kind: NAME, + range: 486..506, + }, + }, + container_name: None, }, FileSymbol { name: "Trait", @@ -141,6 +299,20 @@ ), }, ), + loc: DeclarationLocation { + hir_file_id: HirFileId( + 0, + ), + ptr: SyntaxNodePtr { + kind: TRAIT, + range: 261..300, + }, + name_ptr: SyntaxNodePtr { + kind: NAME, + range: 267..272, + }, + }, + container_name: None, }, FileSymbol { name: "Union", @@ -153,6 +325,20 @@ }, ), ), + loc: DeclarationLocation { + hir_file_id: HirFileId( + 0, + ), + ptr: SyntaxNodePtr { + kind: UNION, + range: 208..222, + }, + name_ptr: SyntaxNodePtr { + kind: NAME, + range: 214..219, + }, + }, + container_name: None, }, FileSymbol { name: "a_mod", @@ -165,6 +351,20 @@ }, }, ), + loc: DeclarationLocation { + hir_file_id: HirFileId( + 0, + ), + ptr: SyntaxNodePtr { + kind: MODULE, + range: 419..457, + }, + name_ptr: SyntaxNodePtr { + kind: NAME, + range: 423..428, + }, + }, + container_name: None, }, FileSymbol { name: "b_mod", @@ -177,6 +377,20 @@ }, }, ), + loc: DeclarationLocation { + hir_file_id: HirFileId( + 0, + ), + ptr: SyntaxNodePtr { + kind: MODULE, + range: 594..604, + }, + name_ptr: SyntaxNodePtr { + kind: NAME, + range: 598..603, + }, + }, + container_name: None, }, FileSymbol { name: "define_struct", @@ -189,6 +403,20 @@ ), }, ), + loc: DeclarationLocation { + hir_file_id: HirFileId( + 0, + ), + ptr: SyntaxNodePtr { + kind: MACRO_RULES, + range: 51..131, + }, + name_ptr: SyntaxNodePtr { + kind: NAME, + range: 64..77, + }, + }, + container_name: None, }, FileSymbol { name: "impl_fn", @@ -199,6 +427,20 @@ ), }, ), + loc: DeclarationLocation { + hir_file_id: HirFileId( + 0, + ), + ptr: SyntaxNodePtr { + kind: FN, + range: 242..257, + }, + name_ptr: SyntaxNodePtr { + kind: NAME, + range: 245..252, + }, + }, + container_name: None, }, FileSymbol { name: "macro_rules_macro", @@ -211,6 +453,20 @@ ), }, ), + loc: DeclarationLocation { + hir_file_id: HirFileId( + 0, + ), + ptr: SyntaxNodePtr { + kind: MACRO_RULES, + range: 1..48, + }, + name_ptr: SyntaxNodePtr { + kind: NAME, + range: 14..31, + }, + }, + container_name: None, }, FileSymbol { name: "main", @@ -221,6 +477,20 @@ ), }, ), + loc: DeclarationLocation { + hir_file_id: HirFileId( + 0, + ), + ptr: SyntaxNodePtr { + kind: FN, + range: 302..338, + }, + name_ptr: SyntaxNodePtr { + kind: NAME, + range: 305..309, + }, + }, + container_name: None, }, FileSymbol { name: "trait_fn", @@ -231,6 +501,22 @@ ), }, ), + loc: DeclarationLocation { + hir_file_id: HirFileId( + 0, + ), + ptr: SyntaxNodePtr { + kind: FN, + range: 279..298, + }, + name_ptr: SyntaxNodePtr { + kind: NAME, + range: 282..290, + }, + }, + container_name: Some( + "Trait", + ), }, ], ), @@ -254,6 +540,20 @@ }, ), ), + loc: DeclarationLocation { + hir_file_id: HirFileId( + 0, + ), + ptr: SyntaxNodePtr { + kind: STRUCT, + range: 435..455, + }, + name_ptr: SyntaxNodePtr { + kind: NAME, + range: 442..454, + }, + }, + container_name: None, }, ], ), @@ -277,6 +577,20 @@ }, ), ), + loc: DeclarationLocation { + hir_file_id: HirFileId( + 1, + ), + ptr: SyntaxNodePtr { + kind: STRUCT, + range: 0..20, + }, + name_ptr: SyntaxNodePtr { + kind: NAME, + range: 7..19, + }, + }, + container_name: None, }, ], ), diff --git a/crates/ide/src/navigation_target.rs b/crates/ide/src/navigation_target.rs index b5e410eaeb9..d8ce79de375 100644 --- a/crates/ide/src/navigation_target.rs +++ b/crates/ide/src/navigation_target.rs @@ -4,8 +4,8 @@ use either::Either; use hir::{ - AssocItem, Documentation, FieldSource, HasAttrs, HasContainer, HasSource, HirDisplay, InFile, - LocalSource, ModuleSource, + symbols::FileSymbol, AssocItem, Documentation, FieldSource, HasAttrs, HasContainer, HasSource, + HirDisplay, InFile, LocalSource, ModuleSource, }; use ide_db::{ base_db::{FileId, FileRange}, @@ -158,6 +158,36 @@ fn from_syntax( } } +impl TryToNav for FileSymbol { + fn try_to_nav(&self, db: &RootDatabase) -> Option { + let full_range = self.loc.original_range(db); + let name_range = self.loc.original_name_range(db)?; + + Some(NavigationTarget { + file_id: full_range.file_id, + name: self.name.clone(), + kind: Some(hir::ModuleDefId::from(self.def).into()), + full_range: full_range.range, + focus_range: Some(name_range.range), + container_name: self.container_name.clone(), + description: match self.def { + hir::ModuleDef::Module(it) => Some(it.display(db).to_string()), + hir::ModuleDef::Function(it) => Some(it.display(db).to_string()), + hir::ModuleDef::Adt(it) => Some(it.display(db).to_string()), + hir::ModuleDef::Variant(it) => Some(it.display(db).to_string()), + hir::ModuleDef::Const(it) => Some(it.display(db).to_string()), + hir::ModuleDef::Static(it) => Some(it.display(db).to_string()), + hir::ModuleDef::Trait(it) => Some(it.display(db).to_string()), + hir::ModuleDef::TraitAlias(it) => Some(it.display(db).to_string()), + hir::ModuleDef::TypeAlias(it) => Some(it.display(db).to_string()), + hir::ModuleDef::Macro(it) => Some(it.display(db).to_string()), + hir::ModuleDef::BuiltinType(_) => None, + }, + docs: None, + }) + } +} + impl TryToNav for Definition { fn try_to_nav(&self, db: &RootDatabase) -> Option { match self {