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

Some optimizations (cont) #395

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
40 changes: 39 additions & 1 deletion Cargo.lock

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

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ clap = { version = "3.1.8", features = ["cargo", "env", "wrap_help"] }
itertools = "0.10.1"
typed-arena = "2.0.1"
rustc-hash = "1.1.0"
hashbrown = "0.12.3"
strsim = "0.10.0"
lazy_static = "1.4.0"
atty = "0.2.14"
Expand Down
92 changes: 20 additions & 72 deletions src/diff/dijkstra.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,12 @@ use std::{cmp::Reverse, env};

use crate::{
diff::changes::ChangeMap,
diff::graph::{get_set_neighbours, populate_change_map, Edge, Vertex},
diff::graph::{get_neighbours, populate_change_map, Edge, SeenMap, Vertex},
parse::syntax::Syntax,
};
use bumpalo::Bump;
use itertools::Itertools;
use radix_heap::RadixHeapMap;
use rustc_hash::FxHashMap;

#[derive(Debug)]
pub struct ExceededGraphLimit {}
Expand All @@ -22,35 +21,35 @@ fn shortest_vertex_path<'a, 'b>(
vertex_arena: &'b Bump,
size_hint: usize,
graph_limit: usize,
) -> Result<Vec<&'b Vertex<'a, 'b>>, ExceededGraphLimit> {
) -> Result<Vec<(Edge, &'b Vertex<'a, 'b>)>, ExceededGraphLimit> {
// We want to visit nodes with the shortest distance first, but
// RadixHeapMap is a max-heap. Ensure nodes are wrapped with
// Reverse to flip comparisons.
let mut heap: RadixHeapMap<Reverse<_>, &'b Vertex<'a, 'b>> = RadixHeapMap::new();

heap.push(Reverse(0), start);

let mut seen = FxHashMap::default();
let mut seen = SeenMap::default();
seen.reserve(size_hint);

let end: &'b Vertex<'a, 'b> = loop {
match heap.pop() {
Some((Reverse(distance), current)) => {
if current.is_visited.get() {
continue;
}
current.is_visited.set(true);

if current.is_end() {
break current;
}

for neighbour in &get_set_neighbours(current, vertex_arena, &mut seen) {
let (edge, next) = neighbour;
for (edge, next) in get_neighbours(current, vertex_arena, &mut seen) {
let distance_to_next = distance + edge.cost();

let found_shorter_route = match next.predecessor.get() {
Some((prev_shortest, _)) => distance_to_next < prev_shortest,
None => true,
};

if found_shorter_route {
next.predecessor.replace(Some((distance_to_next, current)));
if distance_to_next < next.shortest_distance.get() {
next.shortest_distance.set(distance_to_next);
next.predecessor.set(Some((edge, current)));
heap.push(Reverse(distance_to_next), next);
}
}
Expand All @@ -70,37 +69,17 @@ fn shortest_vertex_path<'a, 'b>(
heap.len(),
);

let mut current = Some((0, end));
let mut vertex_route: Vec<&'b Vertex<'a, 'b>> = vec![];
while let Some((_, node)) = current {
vertex_route.push(node);
let mut current = end.predecessor.get();
let mut vertex_route = vec![];
while let Some((edge, node)) = current {
vertex_route.push((edge, node));
current = node.predecessor.get();
}

vertex_route.reverse();
Ok(vertex_route)
}

fn shortest_path_with_edges<'a, 'b>(
route: &[&'b Vertex<'a, 'b>],
) -> Vec<(Edge, &'b Vertex<'a, 'b>)> {
let mut prev = route.first().expect("Expected non-empty route");

let mut cost = 0;
let mut res = vec![];

for vertex in route.iter().skip(1) {
let edge = edge_between(prev, vertex);
res.push((edge, *prev));
cost += edge.cost();

prev = vertex;
}
debug!("Found a path of {} with cost {}.", route.len(), cost);

res
}

/// Return the shortest route from the `start` to the end vertex.
///
/// The vec returned does not return the very last vertex. This is
Expand All @@ -111,41 +90,8 @@ fn shortest_path<'a, 'b>(
size_hint: usize,
graph_limit: usize,
) -> Result<Vec<(Edge, &'b Vertex<'a, 'b>)>, ExceededGraphLimit> {
let start: &'b Vertex<'a, 'b> = vertex_arena.alloc(start.clone());
let vertex_path = shortest_vertex_path(start, vertex_arena, size_hint, graph_limit)?;
Ok(shortest_path_with_edges(&vertex_path))
}

fn edge_between<'a, 'b>(before: &Vertex<'a, 'b>, after: &Vertex<'a, 'b>) -> Edge {
assert_ne!(before, after);

let mut shortest_edge: Option<Edge> = None;
if let Some(neighbours) = &*before.neighbours.borrow() {
for neighbour in neighbours {
let (edge, next) = *neighbour;
// If there are multiple edges that can take us to `next`,
// prefer the shortest.
if *next == *after {
let is_shorter = match shortest_edge {
Some(prev_edge) => edge.cost() < prev_edge.cost(),
None => true,
};

if is_shorter {
shortest_edge = Some(edge);
}
}
}
}

if let Some(edge) = shortest_edge {
return edge;
}

panic!(
"Expected a route between the two vertices {:#?} and {:#?}",
before, after
);
let start: &'b Vertex<'a, 'b> = vertex_arena.alloc(start);
shortest_vertex_path(start, vertex_arena, size_hint, graph_limit)
}

/// What is the total number of AST nodes?
Expand Down Expand Up @@ -220,8 +166,10 @@ pub fn mark_syntax<'a>(
format!(
"{:20} {:20} --- {:3} {:?}",
x.1.lhs_syntax
.get_ref()
.map_or_else(|| "None".into(), Syntax::dbg_content),
x.1.rhs_syntax
.get_ref()
.map_or_else(|| "None".into(), Syntax::dbg_content),
x.0.cost(),
x.0,
Expand Down
Loading