Skip to content

Commit 6d1d744

Browse files
authored
Use unqualified NameIDs for tracking constant references (#217)
This PR continues the work to unlock constant resolution. This one adds an identity map of unqualified names using the `NameId` from #213 and starts using it for constant references. With unqualified Name IDs + listing namespace members (next PR), we can actually resolve constant references based on lexical scope - unblocking a good chunk of the resolution phase. **Note**: there's not much difference in terms of indexing time from this change, but memory was reduced quite a bit. Maybe we can store method parameter names as unqualified name ids too? <details> <summary>Benchmark on Core</summary> ``` BEFORE Initialization 0.001s ( 0.0%) Listing 1.234s ( 34.1%) Indexing 2.156s ( 59.5%) Querying 0.230s ( 6.4%) Cleanup 0.000s ( 0.0%) Total: 3.621s Indexed 87014 files Found 700824 names Found 894881 definitions Found 87014 URIs ---------------------------------------- Maximum Resident Set Size: 708247552 bytes (675.43 MB) Peak Memory Footprint: 685114280 bytes (653.37 MB) Execution Time: 3.86 seconds AFTER Initialization 0.001s ( 0.0%) Listing 1.230s ( 33.4%) Indexing 2.173s ( 59.0%) Querying 0.277s ( 7.5%) Cleanup 0.000s ( 0.0%) Total: 3.681s Indexed 87014 files Found 700824 names Found 894881 definitions Found 87014 URIs ---------------------------------------- Maximum Resident Set Size: 577536000 bytes (550.78 MB) Peak Memory Footprint: 551961392 bytes (526.39 MB) Execution Time: 3.90 seconds ``` </details>
1 parent 8965244 commit 6d1d744

File tree

3 files changed

+46
-35
lines changed

3 files changed

+46
-35
lines changed

rust/saturn/src/indexing/ruby_indexer.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use crate::model::definitions::{
77
ModuleDefinition, Parameter, ParameterStruct,
88
};
99
use crate::model::graph::Graph;
10-
use crate::model::ids::{DeclarationId, UriId};
10+
use crate::model::ids::{DeclarationId, NameId, UriId};
1111
use crate::model::references::{UnresolvedConstantRef, UnresolvedReference};
1212
use crate::offset::Offset;
1313
use crate::source_location::SourceLocationConverter;
@@ -314,15 +314,16 @@ impl<'a> RubyIndexer<'a> {
314314
fn index_constant_reference(&mut self, location: &ruby_prism::Location) {
315315
let offset = Offset::from_prism_location(location);
316316
let name = Self::location_to_string(location);
317+
let name_id = self.local_index.add_name(name);
317318
let nesting = self
318319
.nesting_stacks
319320
.last()
320321
.expect("There should always be at least one stack. This is a bug")
321322
.clone();
322-
let name_id_nesting: Vec<DeclarationId> = nesting.iter().map(DeclarationId::from).collect();
323+
let name_id_nesting: Vec<NameId> = nesting.iter().map(NameId::from).collect();
323324

324325
let reference = UnresolvedReference::Constant(Box::new(UnresolvedConstantRef::new(
325-
name,
326+
name_id,
326327
name_id_nesting,
327328
self.uri_id,
328329
offset,

rust/saturn/src/model/graph.rs

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use crate::model::declaration::Declaration;
66
use crate::model::definitions::Definition;
77
use crate::model::document::Document;
88
use crate::model::identity_maps::IdentityHashMap;
9-
use crate::model::ids::{DeclarationId, DefinitionId, UriId};
9+
use crate::model::ids::{DeclarationId, DefinitionId, NameId, UriId};
1010
use crate::model::integrity::IntegrityChecker;
1111
use crate::model::references::UnresolvedReference;
1212

@@ -26,6 +26,8 @@ pub struct Graph {
2626
documents: IdentityHashMap<UriId, Document>,
2727
// Map of definition nodes
2828
definitions: IdentityHashMap<DefinitionId, Definition>,
29+
// Map of unqualified names
30+
names: IdentityHashMap<NameId, String>,
2931
// List of references that still need to be resolved
3032
unresolved_references: Vec<UnresolvedReference>,
3133
db: Db,
@@ -38,6 +40,7 @@ impl Graph {
3840
declarations: IdentityHashMap::default(),
3941
definitions: IdentityHashMap::default(),
4042
documents: IdentityHashMap::default(),
43+
names: IdentityHashMap::default(),
4144
unresolved_references: Vec::new(),
4245
db: Db::new(),
4346
}
@@ -90,6 +93,13 @@ impl Graph {
9093
uri_id
9194
}
9295

96+
/// Register an unqualified name into the graph
97+
pub fn add_name(&mut self, name: String) -> NameId {
98+
let name_id = NameId::from(&name);
99+
self.names.entry(name_id).or_insert(name);
100+
name_id
101+
}
102+
93103
/// Handles when a document (identified by `uri`) is deleted. This removes the URI from the graph along with all
94104
/// relationships and definitions that had been discovered in it
95105
///
@@ -339,6 +349,7 @@ impl Graph {
339349
self.declarations = IdentityHashMap::default();
340350
self.definitions = IdentityHashMap::default();
341351
self.documents = IdentityHashMap::default();
352+
self.names = IdentityHashMap::default();
342353
self.unresolved_references = Vec::new();
343354
}
344355

@@ -649,10 +660,10 @@ mod tests {
649660

650661
match reference {
651662
UnresolvedReference::Constant(unresolved) => {
652-
assert_eq!(unresolved.name(), "String");
663+
assert_eq!(unresolved.name_id(), &NameId::from("String"));
653664
assert_eq!(
654665
unresolved.nesting(),
655-
vec![DeclarationId::from("Foo"), DeclarationId::from("Bar::Baz")]
666+
vec![NameId::from("Foo"), NameId::from("Bar::Baz")]
656667
);
657668
assert_eq!(unresolved.uri_id(), UriId::from("file:///foo.rb"));
658669
assert_eq!(unresolved.offset(), &Offset::new(32, 38));
@@ -681,11 +692,8 @@ mod tests {
681692

682693
match reference {
683694
UnresolvedReference::Constant(unresolved) => {
684-
assert_eq!(unresolved.name(), "String");
685-
assert_eq!(
686-
unresolved.nesting(),
687-
vec![DeclarationId::from("Foo"), DeclarationId::from("Bar")]
688-
);
695+
assert_eq!(unresolved.name_id(), &NameId::from("String"));
696+
assert_eq!(unresolved.nesting(), vec![NameId::from("Foo"), NameId::from("Bar")]);
689697
assert_eq!(unresolved.uri_id(), UriId::from("file:///foo.rb"));
690698
assert_eq!(unresolved.offset(), &Offset::new(27, 33));
691699
}
@@ -713,11 +721,8 @@ mod tests {
713721

714722
match reference {
715723
UnresolvedReference::Constant(unresolved) => {
716-
assert_eq!(unresolved.name(), "Object::String");
717-
assert_eq!(
718-
unresolved.nesting(),
719-
vec![DeclarationId::from("Foo"), DeclarationId::from("Bar")]
720-
);
724+
assert_eq!(unresolved.name_id(), &NameId::from("Object::String"));
725+
assert_eq!(unresolved.nesting(), vec![NameId::from("Foo"), NameId::from("Bar")]);
721726
assert_eq!(unresolved.uri_id(), UriId::from("file:///foo.rb"));
722727
assert_eq!(unresolved.offset(), &Offset::new(27, 41));
723728
}

rust/saturn/src/model/references.rs

Lines changed: 24 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use crate::{
2-
model::ids::{DeclarationId, UriId},
2+
model::ids::{NameId, UriId},
33
offset::Offset,
44
};
55

@@ -8,43 +8,48 @@ pub enum UnresolvedReference {
88
Constant(Box<UnresolvedConstantRef>),
99
}
1010

11-
// An unresolved constant reference is a usage of a constant in a context where we can't immediately determine what it
12-
// refers to. For example:
13-
//
14-
// ```ruby
15-
// module Foo
16-
// BAR
17-
// end
18-
// ```
19-
//
20-
// Here, we don't immediately know if `BAR` refers to `Foo::BAR` or top level `BAR`. Until we resolve it, it is
21-
// considered an unresolved constant
11+
/// An unresolved constant reference is a usage of a constant in a context where we can't immediately determine what it
12+
/// refers to. For example:
13+
///
14+
/// ```ruby
15+
/// module Foo
16+
/// BAR
17+
/// end
18+
/// ```
19+
///
20+
/// Here, we don't immediately know if `BAR` refers to `Foo::BAR` or top level `BAR`. Until we resolve it, it is
21+
/// considered an unresolved constant
2222
#[derive(Debug)]
2323
pub struct UnresolvedConstantRef {
24-
name: String,
25-
nesting: Vec<DeclarationId>,
24+
/// The unqualified name of the constant
25+
name_id: NameId,
26+
/// The nesting where we found the constant reference. This is a list of unqualified name IDs, so that we can
27+
/// traverse the graph through the member relationships
28+
nesting: Vec<NameId>,
29+
/// The document where we found the reference
2630
uri_id: UriId,
31+
/// The offsets inside of the document where we found the reference
2732
offset: Offset,
2833
}
2934

3035
impl UnresolvedConstantRef {
3136
#[must_use]
32-
pub fn new(name: String, nesting: Vec<DeclarationId>, uri_id: UriId, offset: Offset) -> Self {
37+
pub fn new(name_id: NameId, nesting: Vec<NameId>, uri_id: UriId, offset: Offset) -> Self {
3338
Self {
34-
name,
39+
name_id,
3540
nesting,
3641
uri_id,
3742
offset,
3843
}
3944
}
4045

4146
#[must_use]
42-
pub fn name(&self) -> &str {
43-
&self.name
47+
pub fn name_id(&self) -> &NameId {
48+
&self.name_id
4449
}
4550

4651
#[must_use]
47-
pub fn nesting(&self) -> &[DeclarationId] {
52+
pub fn nesting(&self) -> &[NameId] {
4853
&self.nesting
4954
}
5055

0 commit comments

Comments
 (0)