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

Use database of partial paths to speed up bindings resolution #1204

Merged
merged 25 commits into from
Jan 2, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
ad63a0b
Mark extension hooks and scopes with `is_exported` and `is_endpoint` …
ggiraldez Jan 1, 2025
8e6ca5f
Basic resolver using a database of minimal partial paths
ggiraldez Jan 1, 2025
4b0d85e
Disable graph debugging info (improving processing times)
ggiraldez Jan 1, 2025
b51949c
Add method to resolve all at once and save results
ggiraldez Jan 1, 2025
a037e6c
Perform scope extension with a database of partial paths
ggiraldez Jan 1, 2025
690bf26
Fix linter warnings
ggiraldez Jan 1, 2025
b6a1eb0
Reclaim `PartialPaths` allocated memory after each reference resolved
ggiraldez Jan 1, 2025
57b252a
Encapsulate building the database when constructing the `DatabaseReso…
ggiraldez Jan 1, 2025
3778463
Refactor: move most reference/definition getters to the `Bindings` owner
ggiraldez Jan 1, 2025
b7527df
Expose `DatabaseResolver` type as public
ggiraldez Jan 1, 2025
c63d183
Update public_api.txt
ggiraldez Jan 1, 2025
642c3c6
Replace old resolver with database resolver
ggiraldez Jan 1, 2025
e59a0eb
Remove obsolete `ResolutionError`
ggiraldez Jan 1, 2025
f7e4af8
`Definition` and `Reference` take an `Rc<>` instead of a reference to…
ggiraldez Jan 1, 2025
f44cd6b
Add a bit of documentation
ggiraldez Jan 1, 2025
542aaaf
Split builder part of `BindingGraph` to use before calling `resolve()`
ggiraldez Jan 1, 2025
443e4b1
Rename bindings `Builder` to `Loader`
ggiraldez Jan 1, 2025
5ec0487
Refactor: move bindings builder into its own module along with resolv…
ggiraldez Jan 1, 2025
abbf213
Remove wrappers for Definition/Reference used in WASM
ggiraldez Jan 1, 2025
fd2d907
Update stack-graphs dependency to the `nomic` branch of our fork
ggiraldez Jan 1, 2025
8bd409f
Refactor performance to try to keep consistent with previous bencher …
ggiraldez Jan 1, 2025
c6a0984
Update public_api.txt
ggiraldez Jan 1, 2025
68bf29c
fix ci
OmarTawfik Jan 1, 2025
3c4fb33
re-organize perf benchmarks
OmarTawfik Jan 2, 2025
856842b
skip identifiers due to #1213 bug
OmarTawfik Jan 2, 2025
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
10 changes: 4 additions & 6 deletions Cargo.lock

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

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ serde = { version = "1.0.216", features = ["derive", "rc"] }
serde_json = { version = "1.0.133", features = ["preserve_order"] }
similar-asserts = { version = "1.6.0" }
smallvec = { version = "1.7.0", features = ["union"] }
stack-graphs = { version = "0.13.0" }
stack-graphs = { git = "https://github.com/NomicFoundation/stack-graphs", branch = "nomic" }
string-interner = { version = "0.17.0", features = [
"std",
"inline-more",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
use semver::Version;

use crate::bindings::BindingGraph;
use crate::bindings::BindingGraphBuilder;
use crate::parser::ParserInitializationError;

#[allow(clippy::needless_pass_by_value)]
pub fn add_built_ins(
_binding_graph: &mut BindingGraph,
_binding_graph_builder: &mut BindingGraphBuilder,
_version: Version,
) -> Result<(), ParserInitializationError> {
unreachable!("Built-ins are Solidity-specific")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,10 @@ use semver::Version;

use crate::cst::KindTypes;

pub type BindingGraphBuilder = metaslang_bindings::BindingGraphBuilder<KindTypes>;
pub type BindingGraph = metaslang_bindings::BindingGraph<KindTypes>;
pub type Definition<'a> = metaslang_bindings::Definition<'a, KindTypes>;
pub type Reference<'a> = metaslang_bindings::Reference<'a, KindTypes>;
pub type Definition = metaslang_bindings::Definition<KindTypes>;
pub type Reference = metaslang_bindings::Reference<KindTypes>;
pub type BindingLocation = metaslang_bindings::BindingLocation<KindTypes>;
pub type UserFileLocation = metaslang_bindings::UserFileLocation<KindTypes>;

Expand All @@ -29,8 +30,8 @@ pub enum BindingGraphInitializationError {
pub fn create_with_resolver(
version: Version,
resolver: Rc<dyn PathResolver<KindTypes>>,
) -> Result<BindingGraph, ParserInitializationError> {
let mut binding_graph = BindingGraph::create(
) -> Result<BindingGraphBuilder, BindingGraphInitializationError> {
let mut binding_graph = BindingGraphBuilder::create(
version.clone(),
binding_rules::BINDING_RULES_SOURCE,
resolver,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,14 @@ impl CompilationUnit {
files: self.files.clone(),
};

let mut binding_graph =
let mut builder =
create_with_resolver(self.language_version.clone(), Rc::new(resolver))?;

for (id, file) in &self.files {
binding_graph.add_user_file(id, file.create_tree_cursor());
builder.add_user_file(id, file.create_tree_cursor());
}

Ok(Rc::new(binding_graph))
Ok(builder.build())
})
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ mod terminal_kind;
pub use edge_label::EdgeLabel;
pub(crate) use lexical_context::{IsLexicalContext, LexicalContext, LexicalContextType};
pub use metaslang_cst::kinds::{
EdgeLabelExtensions, NonterminalKindExtensions, TerminalKindExtensions,
EdgeLabelExtensions, NodeKind, NonterminalKindExtensions, TerminalKindExtensions,
};
pub use nonterminal_kind::NonterminalKind;
pub use terminal_kind::TerminalKind;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,52 +12,8 @@ mod ffi {

mod rust {
pub use crate::rust_crate::bindings::{
BindingGraph, BindingLocation, BuiltInLocation, UserFileLocation,
BindingGraph, BindingLocation, BuiltInLocation, Definition, Reference, UserFileLocation,
};

/// TODO: This is a work-around for the fact that `metaslang_bindings` internals (handles, locators, etc...) are exposed.
/// We should clean this when we finally publish `__experimental_bindings_api`.
/// That means removing the types below, and using the original types instead.
#[derive(Debug, Clone)]
pub struct Definition {
pub id: usize,
pub name_location: BindingLocation,
pub definiens_location: BindingLocation,
}

impl From<crate::rust_crate::bindings::Definition<'_>> for Definition {
fn from(definition: crate::rust_crate::bindings::Definition<'_>) -> Self {
Self {
id: definition.id(),
name_location: definition.name_location(),
definiens_location: definition.definiens_location(),
}
}
}

/// TODO: This is a work-around for the fact that `metaslang_bindings` internals (handles, locators, etc...) are exposed.
/// We should clean this when we finally publish `__experimental_bindings_api`.
/// That means removing the types below, and using the original types instead.
#[derive(Debug, Clone)]
pub struct Reference {
pub id: usize,
pub location: BindingLocation,
pub definitions: Vec<Definition>,
}

impl From<crate::rust_crate::bindings::Reference<'_>> for Reference {
fn from(reference: crate::rust_crate::bindings::Reference<'_>) -> Self {
Self {
id: reference.id(),
location: reference.location(),
definitions: reference
.definitions()
.into_iter()
.map(Into::into)
.collect(),
}
}
}
}

impl ffi::Guest for crate::wasm_crate::World {
Expand Down Expand Up @@ -100,15 +56,15 @@ define_rc_wrapper! { BindingGraph {

define_wrapper! { Definition {
fn id(&self) -> u32 {
self._borrow_ffi().id.try_into().unwrap()
self._borrow_ffi().id().try_into().unwrap()
}

fn name_location(&self) -> ffi::BindingLocation {
self._borrow_ffi().name_location.clone()._into_ffi()
self._borrow_ffi().name_location()._into_ffi()
}

fn definiens_location(&self) -> ffi::BindingLocation {
self._borrow_ffi().definiens_location.clone()._into_ffi()
self._borrow_ffi().definiens_location()._into_ffi()
}
} }

Expand All @@ -120,15 +76,15 @@ define_wrapper! { Definition {

define_wrapper! { Reference {
fn id(&self) -> u32 {
self._borrow_ffi().id.try_into().unwrap()
self._borrow_ffi().id().try_into().unwrap()
}

fn location(&self) -> ffi::BindingLocation {
self._borrow_ffi().location.clone()._into_ffi()
self._borrow_ffi().location().clone()._into_ffi()
}

fn definitions(&self) -> Vec<ffi::Definition> {
self._borrow_ffi().definitions.iter().cloned().map(IntoFFI::_into_ffi).collect()
self._borrow_ffi().definitions().iter().cloned().map(IntoFFI::_into_ffi).collect()
}
} }

Expand Down
92 changes: 41 additions & 51 deletions crates/metaslang/bindings/generated/public_api.txt

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

Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ mod resolver {
use metaslang_graph_builder::graph::{Graph, Value};
use metaslang_graph_builder::ExecutionError;

use crate::{FileDescriptor, PathResolver};
use crate::builder::FileDescriptor;
use crate::PathResolver;

pub fn add_functions<KT: KindTypes + 'static>(
functions: &mut Functions<KT>,
Expand Down
Loading