Skip to content

Commit

Permalink
Merge pull request #537 from erg-lang/perf-graph
Browse files Browse the repository at this point in the history 
Improve computational complexity of `ModuleGraph`
  • Loading branch information
@mtshiba
mtshiba authored Dec 4, 2024
2 parents dc18307 + 3b07ef2 commit d648e6e
Show file tree
Hide file tree
Showing 3 changed files with 137 additions and 43 deletions.
6 changes: 6 additions & 0 deletions crates/erg_common/set.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,12 @@ impl<T: Hash + Eq + Clone> Set<T> {
}
}

impl<T: Hash + Eq + Clone> Set<&T> {
pub fn cloned(&self) -> Set<T> {
self.iter().map(|&x| x.clone()).collect()
}
}

impl<T: Hash + Ord> Set<T> {
pub fn max(&self) -> Option<&T> {
self.iter().max_by(|x, y| x.cmp(y))
Expand Down
7 changes: 5 additions & 2 deletions crates/erg_compiler/build_package.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -851,7 +851,7 @@ impl<ASTBuilder: ASTBuildable, HIRBuilder: Buildable>
graph: &mut ModuleGraph,
) -> Set<NormalizedPathBuf> {
let mut deps = Set::new();
let mut ancestors = graph.ancestors(path).into_vec();
let mut ancestors = graph.ancestors(path).cloned().into_vec();
let nmods = ancestors.len();
let pad = nmods.to_string().len();
let print_progress =
Expand All @@ -862,7 +862,10 @@ impl<ASTBuilder: ASTBuildable, HIRBuilder: Buildable>
out.flush().unwrap();
}
while let Some(ancestor) = ancestors.pop() {
if graph.ancestors(&ancestor).is_empty() {
if graph
.parents(&ancestor)
.is_none_or(|parents| parents.is_empty())
{
graph.remove(&ancestor);
if let Some(entry) = self.asts.remove(&ancestor) {
deps.insert(ancestor.clone());
Expand Down
167 changes: 126 additions & 41 deletions crates/erg_compiler/module/graph.rs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use std::fmt;

use erg_common::consts::DEBUG_MODE;
use erg_common::dict::Dict;
use erg_common::pathutil::NormalizedPathBuf;
use erg_common::set;
use erg_common::set::Set;
Expand All @@ -19,12 +20,15 @@ impl IncRefError {
}

#[derive(Debug, Clone, Default)]
pub struct ModuleGraph(Graph<NormalizedPathBuf, ()>);
pub struct ModuleGraph {
graph: Graph<NormalizedPathBuf, ()>,
index: Dict<NormalizedPathBuf, usize>,
}

impl fmt::Display for ModuleGraph {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
writeln!(f, "ModuleGraph {{")?;
for node in self.0.iter() {
for node in self.graph.iter() {
writeln!(f, "{} depends on {{", node.id.display())?;
for dep in node.depends_on.iter() {
writeln!(f, "{}, ", dep.display())?;
Expand All @@ -40,59 +44,115 @@ impl IntoIterator for ModuleGraph {
type IntoIter = std::vec::IntoIter<Self::Item>;

fn into_iter(self) -> Self::IntoIter {
self.0.into_iter()
self.graph.into_iter()
}
}

impl ModuleGraph {
pub fn new() -> Self {
Self(Graph::new())
Self {
graph: Graph::new(),
index: Dict::new(),
}
}

pub fn get_node(&self, path: &NormalizedPathBuf) -> Option<&Node<NormalizedPathBuf, ()>> {
self.0.iter().find(|n| &n.id == path)
self.index.get(path).map(|&i| &self.graph[i])
}

/// if `path` depends on `target`, returns `true`, else `false`.
/// if `path` not found, returns `false`
pub fn get_mut_node(
&mut self,
path: &NormalizedPathBuf,
) -> Option<&mut Node<NormalizedPathBuf, ()>> {
self.index.get(path).map(|&i| &mut self.graph[i])
}

/// if `path` directly depends on `target`, returns `true`, else `false`.
/// if `path` not found, returns `false`.
/// O(1)
pub fn depends_on(&self, path: &NormalizedPathBuf, target: &NormalizedPathBuf) -> bool {
let path = NormalizedPathBuf::new(path.to_path_buf());
let target = NormalizedPathBuf::new(target.to_path_buf());
self.0
.iter()
.find(|n| n.id == path)
self.get_node(&path)
.map(|n| n.depends_on.contains(&target))
.unwrap_or(false)
}

/// (usually) `path` is not contained
pub fn children(&self, path: &NormalizedPathBuf) -> Set<NormalizedPathBuf> {
self.0
/// if `path` depends on `target`, returns `true`, else `false`.
/// if `path` not found, returns `false`.
pub fn deep_depends_on(&self, path: &NormalizedPathBuf, target: &NormalizedPathBuf) -> bool {
let path = NormalizedPathBuf::new(path.to_path_buf());
let target = NormalizedPathBuf::new(target.to_path_buf());
let mut visited = set! {};
self.deep_depends_on_(&path, &target, &mut visited)
}

fn deep_depends_on_<'p>(
&'p self,
path: &'p NormalizedPathBuf,
target: &NormalizedPathBuf,
visited: &mut Set<&'p NormalizedPathBuf>,
) -> bool {
if !visited.insert(path) {
return false;
}
self.get_node(path)
.map(|n| {
n.depends_on.contains(target)
|| n.depends_on
.iter()
.any(|p| self.deep_depends_on_(p, target, visited))
})
.unwrap_or(false)
}

/// (usually) `path` is not contained.
/// O(N)
pub fn children<'p>(
&'p self,
path: &'p NormalizedPathBuf,
) -> impl Iterator<Item = NormalizedPathBuf> + 'p {
self.graph
.iter()
.filter(|n| n.depends_on.contains(path))
.map(|n| n.id.clone())
.collect()
}

/// (usually) `path` is not contained
fn parents(&self, path: &NormalizedPathBuf) -> Option<&Set<NormalizedPathBuf>> {
self.0.iter().find(|n| &n.id == path).map(|n| &n.depends_on)
/// (usually) `path` is not contained.
/// O(1)
pub fn parents(&self, path: &NormalizedPathBuf) -> Option<&Set<NormalizedPathBuf>> {
self.get_node(path).map(|n| &n.depends_on)
}

/// ```erg
/// # a.er
/// b = import "b"
/// ```
/// -> a: child, b: parent
pub fn ancestors(&self, path: &NormalizedPathBuf) -> Set<NormalizedPathBuf> {
/// -> a: child, b: parent<br>
/// O(N)
pub fn ancestors<'p>(&'p self, path: &'p NormalizedPathBuf) -> Set<&'p NormalizedPathBuf> {
let mut ancestors = set! {};
let mut visited = set! {};
self.ancestors_(path, &mut ancestors, &mut visited);
ancestors
}

fn ancestors_<'p>(
&'p self,
path: &'p NormalizedPathBuf,
ancestors: &mut Set<&'p NormalizedPathBuf>,
visited: &mut Set<&'p NormalizedPathBuf>,
) {
if !visited.insert(path) {
return;
}
if let Some(parents) = self.parents(path) {
for parent in parents.iter() {
ancestors.insert(parent.clone());
ancestors.extend(self.ancestors(parent));
if ancestors.insert(parent) {
self.ancestors_(parent, ancestors, visited);
}
}
}
ancestors
}

pub fn add_node_if_none(&mut self, path: &NormalizedPathBuf) {
Expand All @@ -102,9 +162,10 @@ impl ModuleGraph {
}
return;
}
if self.0.iter().all(|n| &n.id != path) {
if self.index.get(path).is_none() {
let node = Node::new(path.clone(), (), set! {});
self.0.push(node);
self.graph.push(node);
self.index.insert(path.clone(), self.graph.len() - 1);
}
}

Expand All @@ -124,10 +185,10 @@ impl ModuleGraph {
if referrer == &depends_on {
return Ok(());
}
if self.ancestors(&depends_on).contains(referrer) {
if self.deep_depends_on(&depends_on, referrer) {
return Err(IncRefError::CycleDetected);
}
if let Some(node) = self.0.iter_mut().find(|n| &n.id == referrer) {
if let Some(node) = self.get_mut_node(referrer) {
node.push_dep(depends_on);
} else {
unreachable!("node not found: {}", referrer.display());
Expand All @@ -136,12 +197,19 @@ impl ModuleGraph {
}

pub fn iter(&self) -> impl Iterator<Item = &Node<NormalizedPathBuf, ()>> {
self.0.iter()
self.graph.iter()
}

#[allow(clippy::result_unit_err)]
pub fn sorted(self) -> Result<Self, TopoSortError> {
tsort(self.0).map(Self)
tsort(self.graph).map(|graph| {
let index = graph
.iter()
.map(|n| n.id.clone())
.enumerate()
.map(|(i, path)| (path, i))
.collect();
Self { graph, index }
})
}

#[allow(clippy::result_unit_err)]
Expand All @@ -150,16 +218,27 @@ impl ModuleGraph {
Ok(())
}

/// Do not erase relationships with modules that depend on `path`
/// Do not erase relationships with modules that depend on `path`.
/// O(N)
pub fn remove(&mut self, path: &NormalizedPathBuf) {
self.0.retain(|n| &n.id != path);
for node in self.0.iter_mut() {
if let Some(&i) = self.index.get(path) {
self.graph.remove(i);
self.index.remove(path);
// shift indices
for index in self.index.values_mut() {
if *index > i {
*index -= 1;
}
}
}
for node in self.graph.iter_mut() {
node.depends_on.retain(|p| p != path);
}
}

/// O(N)
pub fn rename_path(&mut self, old: &NormalizedPathBuf, new: NormalizedPathBuf) {
for node in self.0.iter_mut() {
for node in self.graph.iter_mut() {
if &node.id == old {
node.id = new.clone();
}
Expand All @@ -171,7 +250,8 @@ impl ModuleGraph {
}

pub fn initialize(&mut self) {
self.0.clear();
self.graph.clear();
self.index.clear();
}

pub fn display_parents(
Expand All @@ -198,9 +278,9 @@ impl ModuleGraph {
pub fn display(&self) -> String {
let mut s = String::new();
let mut appeared = set! {};
for node in self.0.iter() {
let children = self.children(&node.id);
if !children.is_empty() || appeared.contains(&node.id) {
for node in self.graph.iter() {
let mut children = self.children(&node.id);
if children.next().is_some() || appeared.contains(&node.id) {
continue;
}
s.push_str(&format!("{}\n", node.id.display()));
Expand Down Expand Up @@ -241,18 +321,23 @@ impl SharedModuleGraph {
RwLockReadGuard::try_map(self.0.borrow(), |graph| graph.get_node(path)).ok()
}

/// if `path` directly depends on `target`, returns `true`, else `false`.
/// if `path` not found, returns `false`.
/// O(1)
pub fn depends_on(&self, path: &NormalizedPathBuf, target: &NormalizedPathBuf) -> bool {
self.0.borrow().depends_on(path, target)
}

/// (usually) `path` is not contained
/// (usually) `path` is not contained.
/// O(N)
pub fn children(&self, path: &NormalizedPathBuf) -> Set<NormalizedPathBuf> {
self.0.borrow().children(path)
self.0.borrow().children(path).collect()
}

/// (usually) `path` is not contained
/// (usually) `path` is not contained.
/// O(N)
pub fn ancestors(&self, path: &NormalizedPathBuf) -> Set<NormalizedPathBuf> {
self.0.borrow().ancestors(path)
self.0.borrow().ancestors(path).cloned()
}

pub fn add_node_if_none(&self, path: &NormalizedPathBuf) {
Expand Down

0 comments on commit d648e6e

Please sign in to comment.