Store predecessors and neighbours as mutable fields in graph nodes

This is a more traditional graph representation. It is slightly easier
to reason about, and it's clearer that graph node creation time
dominates graphs exploration.

This is a slight performance regression, but it enables better
exploration of parethesis nesting (see next commit). typing_before.ml
has regressed from 3.75B instructions to 3.85B instructions and
slow_before.rs has regressed from 1.73B instructions to 2.15B
instructions.

This change has also made the diff output for slow_before.rs slightly
worse (note the `lhs` variable is now claimed as changed in more
cases). It's not clear why, but presumably means that the node visit
order has changed slightly.

Closes #324

sample_files/compare.expected

@@ -155,7 +155,7 @@ sample_files/slider_before.rs sample_files/slider_after.rs
 50e1df5af0bf4a1fa7211e079196f1a9  -
 
 sample_files/slow_before.rs sample_files/slow_after.rs
-a1d8070fda3b8fa65886a90bde64a2ab  -
+e8e7521e32766879f7219788a113005a  -
 
 sample_files/small_before.js sample_files/small_after.js
 27bcac13aa17141718a3e6b8c0ac8f47  -

src/diff/dijkstra.rs

@@ -1,11 +1,11 @@
 //! Implements Dijkstra's algorithm for shortest path, to find an
 //! optimal and readable diff between two ASTs.
 
-use std::{cmp::Reverse, collections::hash_map::Entry, env};
+use std::{cmp::Reverse, env};
 
 use crate::{
     diff::changes::ChangeMap,
-    diff::graph::{neighbours, populate_change_map, Edge, Vertex},
+    diff::graph::{get_set_neighbours, populate_change_map, Edge, Vertex},
     parse::syntax::Syntax,
 };
 use bumpalo::Bump;
@@ -13,61 +13,49 @@ use itertools::Itertools;
 use radix_heap::RadixHeapMap;
 use rustc_hash::FxHashMap;
 
-type PredecessorInfo<'a, 'b> = (u64, &'b Vertex<'a>);
-
 #[derive(Debug)]
 pub struct ExceededGraphLimit {}
 
 /// Return the shortest route from `start` to the end vertex.
-fn shortest_vertex_path(
+fn shortest_vertex_path<'a, 'b>(
-    start: Vertex,
+    start: &'b Vertex<'a, 'b>,
+    vertex_arena: &'b Bump,
     size_hint: usize,
     graph_limit: usize,
-) -> Result<Vec<Vertex>, ExceededGraphLimit> {
+) -> Result<Vec<&'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<_>, &Vertex> = RadixHeapMap::new();
+    let mut heap: RadixHeapMap<Reverse<_>, &'b Vertex<'a, 'b>> = RadixHeapMap::new();
 
-    let vertex_arena = Bump::new();
+    heap.push(Reverse(0), start);
-    heap.push(Reverse(0), vertex_arena.alloc(start.clone()));
 
-    // TODO: this grows very big. Consider using IDA* to reduce memory
+    let mut seen = FxHashMap::default();
-    // usage.
+    seen.reserve(size_hint);
-    let mut predecessors: FxHashMap<&Vertex, PredecessorInfo> = FxHashMap::default();
-    predecessors.reserve(size_hint);
 
-    let mut neighbour_buf = [
+    let end: &'b Vertex<'a, 'b> = loop {
-        None, None, None, None, None, None, None, None, None, None, None, None,
-    ];
-    let end = loop {
         match heap.pop() {
             Some((Reverse(distance), current)) => {
                 if current.is_end() {
                     break current;
                 }
 
-                neighbours(current, &mut neighbour_buf, &vertex_arena);
+                for neighbour in &get_set_neighbours(current, vertex_arena, &mut seen) {
-                for neighbour in &mut neighbour_buf {
+                    let (edge, next) = neighbour;
-                    if let Some((edge, next)) = neighbour.take() {
+                    let distance_to_next = distance + edge.cost();
-                        let distance_to_next = distance + edge.cost();
+
-
+                    let found_shorter_route = match &*next.predecessor.borrow() {
-                        match predecessors.entry(next) {
+                        Some((prev_shortest, _)) => distance_to_next < *prev_shortest,
-                            Entry::Occupied(mut o) => {
+                        None => true,
-                                let prev_shortest: u64 = o.get().0;
+                    };
-                                if distance_to_next < prev_shortest {
+
-                                    *o.get_mut() = (distance_to_next, current);
+                    if found_shorter_route {
-                                    heap.push(Reverse(distance_to_next), next);
+                        next.predecessor.replace(Some((distance_to_next, current)));
-                                }
+                        heap.push(Reverse(distance_to_next), next);
-                            }
-                            Entry::Vacant(v) => {
-                                v.insert((distance_to_next, current));
-                                heap.push(Reverse(distance_to_next), next);
-                            }
-                        };
                     }
                 }
-                if predecessors.len() > graph_limit {
+
+                if seen.len() > graph_limit {
                     return Err(ExceededGraphLimit {});
                 }
             }
@@ -76,29 +64,31 @@ fn shortest_vertex_path(
     };
 
     debug!(
-        "Found predecessors for {} vertices (hashmap key: {} bytes, value: {} bytes), with {} left on heap.",
+        "Saw {} vertices (a Vertex is {} bytes), with {} left on heap.",
-        predecessors.len(),
+        seen.len(),
-        std::mem::size_of::<&Vertex>(),
+        std::mem::size_of::<Vertex>(),
-        std::mem::size_of::<PredecessorInfo>(),
         heap.len(),
     );
-    let mut current = end;
 
-    let mut vertex_route: Vec<Vertex> = vec![end.clone()];
+    let mut current = Some((0, end));
-    while let Some((_, node)) = predecessors.remove(&current) {
+    let mut vertex_route: Vec<&'b Vertex<'a, 'b>> = vec![];
-        vertex_route.push(node.clone());
+    while let Some((_, node)) = current {
-        current = node;
+        vertex_route.push(node);
+        current = *node.predecessor.borrow();
     }
 
     vertex_route.reverse();
     Ok(vertex_route)
 }
 
-fn shortest_path_with_edges<'a>(route: &[Vertex<'a>]) -> Vec<(Edge, Vertex<'a>)> {
+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.clone()));
@@ -115,29 +105,27 @@ fn shortest_path_with_edges<'a>(route: &[Vertex<'a>]) -> Vec<(Edge, Vertex<'a>)>
 ///
 /// The vec returned does not return the very last vertex. This is
 /// necessary because a route of N vertices only has N-1 edges.
-fn shortest_path(
+fn shortest_path<'a, 'b>(
-    start: Vertex,
+    start: Vertex<'a, 'b>,
+    vertex_arena: &'b Bump,
     size_hint: usize,
     graph_limit: usize,
-) -> Result<Vec<(Edge, Vertex)>, ExceededGraphLimit> {
+) -> Result<Vec<(Edge, &'b Vertex<'a, 'b>)>, ExceededGraphLimit> {
-    let vertex_path = shortest_vertex_path(start, size_hint, graph_limit)?;
+    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>(before: &Vertex<'a>, after: &Vertex<'a>) -> Edge {
+fn edge_between<'a, 'b>(before: &Vertex<'a, 'b>, after: &Vertex<'a, 'b>) -> Edge {
-    let mut neighbour_buf = [
+    assert_ne!(before, after);
-        None, None, None, None, None, None, None, None, None, None, None, None,
-    ];
-
-    let vertex_arena = Bump::new();
-    neighbours(before, &mut neighbour_buf, &vertex_arena);
 
     let mut shortest_edge: Option<Edge> = None;
-    for neighbour in &mut neighbour_buf {
+    if let Some(neighbours) = &*before.neighbours.borrow() {
-        if let Some((edge, next)) = neighbour.take() {
+        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 {
+            if *next == *after {
                 let is_shorter = match shortest_edge {
                     Some(prev_edge) => edge.cost() < prev_edge.cost(),
                     None => true,
@@ -214,7 +202,9 @@ pub fn mark_syntax<'a>(
     let size_hint = lhs_node_count * rhs_node_count;
 
     let start = Vertex::new(lhs_syntax, rhs_syntax);
-    let route = shortest_path(start, size_hint, graph_limit)?;
+    let vertex_arena = Bump::new();
+
+    let route = shortest_path(start, &vertex_arena, size_hint, graph_limit)?;
 
     let print_length = if env::var("DFT_VERBOSE").is_ok() {
         50
@@ -285,7 +275,8 @@ mod tests {
         init_all_info(&[lhs], &[rhs]);
 
         let start = Vertex::new(Some(lhs), Some(rhs));
-        let route = shortest_path(start, 0, DEFAULT_GRAPH_LIMIT).unwrap();
+        let vertex_arena = Bump::new();
+        let route = shortest_path(start, &vertex_arena, 0, DEFAULT_GRAPH_LIMIT).unwrap();
 
         let actions = route.iter().map(|(action, _)| *action).collect_vec();
         assert_eq!(
@@ -325,7 +316,8 @@ mod tests {
         init_all_info(&lhs, &rhs);
 
         let start = Vertex::new(lhs.get(0).copied(), rhs.get(0).copied());
-        let route = shortest_path(start, 0, DEFAULT_GRAPH_LIMIT).unwrap();
+        let vertex_arena = Bump::new();
+        let route = shortest_path(start, &vertex_arena, 0, DEFAULT_GRAPH_LIMIT).unwrap();
 
         let actions = route.iter().map(|(action, _)| *action).collect_vec();
         assert_eq!(
@@ -367,7 +359,8 @@ mod tests {
         init_all_info(&lhs, &rhs);
 
         let start = Vertex::new(lhs.get(0).copied(), rhs.get(0).copied());
-        let route = shortest_path(start, 0, DEFAULT_GRAPH_LIMIT).unwrap();
+        let vertex_arena = Bump::new();
+        let route = shortest_path(start, &vertex_arena, 0, DEFAULT_GRAPH_LIMIT).unwrap();
 
         let actions = route.iter().map(|(action, _)| *action).collect_vec();
         assert_eq!(
@@ -413,7 +406,8 @@ mod tests {
         init_all_info(&lhs, &rhs);
 
         let start = Vertex::new(lhs.get(0).copied(), rhs.get(0).copied());
-        let route = shortest_path(start, 0, DEFAULT_GRAPH_LIMIT).unwrap();
+        let vertex_arena = Bump::new();
+        let route = shortest_path(start, &vertex_arena, 0, DEFAULT_GRAPH_LIMIT).unwrap();
 
         let actions = route.iter().map(|(action, _)| *action).collect_vec();
         assert_eq!(
@@ -452,7 +446,8 @@ mod tests {
         init_all_info(&lhs, &rhs);
 
         let start = Vertex::new(lhs.get(0).copied(), rhs.get(0).copied());
-        let route = shortest_path(start, 0, DEFAULT_GRAPH_LIMIT).unwrap();
+        let vertex_arena = Bump::new();
+        let route = shortest_path(start, &vertex_arena, 0, DEFAULT_GRAPH_LIMIT).unwrap();
 
         let actions = route.iter().map(|(action, _)| *action).collect_vec();
         assert_eq!(
@@ -489,7 +484,8 @@ mod tests {
         init_all_info(&lhs, &rhs);
 
         let start = Vertex::new(lhs.get(0).copied(), rhs.get(0).copied());
-        let route = shortest_path(start, 0, DEFAULT_GRAPH_LIMIT).unwrap();
+        let vertex_arena = Bump::new();
+        let route = shortest_path(start, &vertex_arena, 0, DEFAULT_GRAPH_LIMIT).unwrap();
 
         let actions = route.iter().map(|(action, _)| *action).collect_vec();
         assert_eq!(
@@ -527,7 +523,8 @@ mod tests {
         init_all_info(&lhs, &rhs);
 
         let start = Vertex::new(lhs.get(0).copied(), rhs.get(0).copied());
-        let route = shortest_path(start, 0, DEFAULT_GRAPH_LIMIT).unwrap();
+        let vertex_arena = Bump::new();
+        let route = shortest_path(start, &vertex_arena, 0, DEFAULT_GRAPH_LIMIT).unwrap();
 
         let actions = route.iter().map(|(action, _)| *action).collect_vec();
         assert_eq!(
@@ -561,7 +558,8 @@ mod tests {
         init_all_info(&lhs, &rhs);
 
         let start = Vertex::new(lhs.get(0).copied(), rhs.get(0).copied());
-        let route = shortest_path(start, 0, DEFAULT_GRAPH_LIMIT).unwrap();
+        let vertex_arena = Bump::new();
+        let route = shortest_path(start, &vertex_arena, 0, DEFAULT_GRAPH_LIMIT).unwrap();
 
         let actions = route.iter().map(|(action, _)| *action).collect_vec();
         assert_eq!(
@@ -592,7 +590,8 @@ mod tests {
         init_all_info(&lhs, &rhs);
 
         let start = Vertex::new(lhs.get(0).copied(), rhs.get(0).copied());
-        let route = shortest_path(start, 0, DEFAULT_GRAPH_LIMIT).unwrap();
+        let vertex_arena = Bump::new();
+        let route = shortest_path(start, &vertex_arena, 0, DEFAULT_GRAPH_LIMIT).unwrap();
 
         let actions = route.iter().map(|(action, _)| *action).collect_vec();
         assert_eq!(
@@ -631,7 +630,8 @@ mod tests {
         init_all_info(&lhs, &rhs);
 
         let start = Vertex::new(lhs.get(0).copied(), rhs.get(0).copied());
-        let route = shortest_path(start, 0, DEFAULT_GRAPH_LIMIT).unwrap();
+        let vertex_arena = Bump::new();
+        let route = shortest_path(start, &vertex_arena, 0, DEFAULT_GRAPH_LIMIT).unwrap();
 
         let actions = route.iter().map(|(action, _)| *action).collect_vec();
         assert_eq!(

src/diff/graph.rs

@@ -2,7 +2,9 @@
 
 use bumpalo::Bump;
 use rpds::Stack;
+use rustc_hash::FxHashMap;
 use std::{
+    cell::RefCell,
     cmp::min,
     fmt,
     hash::{Hash, Hasher},
@@ -44,7 +46,9 @@ use Edge::*;
 ///      ^              ^
 /// ```
 #[derive(Debug, Clone)]
-pub struct Vertex<'a> {
+pub struct Vertex<'a, 'b> {
+    pub neighbours: RefCell<Option<Vec<(Edge, &'b Vertex<'a, 'b>)>>>,
+    pub predecessor: RefCell<Option<(u64, &'b Vertex<'a, 'b>)>>,
     pub lhs_syntax: Option<&'a Syntax<'a>>,
     pub rhs_syntax: Option<&'a Syntax<'a>>,
     parents: Stack<EnteredDelimiter<'a>>,
@@ -53,7 +57,7 @@ pub struct Vertex<'a> {
     can_pop_either: bool,
 }
 
-impl<'a> PartialEq for Vertex<'a> {
+impl<'a, 'b> PartialEq for Vertex<'a, 'b> {
     fn eq(&self, other: &Self) -> bool {
         self.lhs_syntax.map(|node| node.id()) == other.lhs_syntax.map(|node| node.id())
             && self.rhs_syntax.map(|node| node.id()) == other.rhs_syntax.map(|node| node.id())
@@ -83,9 +87,9 @@ impl<'a> PartialEq for Vertex<'a> {
             && self.can_pop_either == other.can_pop_either
     }
 }
-impl<'a> Eq for Vertex<'a> {}
+impl<'a, 'b> Eq for Vertex<'a, 'b> {}
 
-impl<'a> Hash for Vertex<'a> {
+impl<'a, 'b> Hash for Vertex<'a, 'b> {
     fn hash<H: Hasher>(&self, state: &mut H) {
         self.lhs_syntax.map(|node| node.id()).hash(state);
         self.rhs_syntax.map(|node| node.id()).hash(state);
@@ -245,7 +249,7 @@ fn push_rhs_delimiter<'a>(
     entered.push(EnteredDelimiter::PopEither((lhs_delims, rhs_delims)))
 }
 
-impl<'a> Vertex<'a> {
+impl<'a, 'b> Vertex<'a, 'b> {
     pub fn is_end(&self) -> bool {
         self.lhs_syntax.is_none() && self.rhs_syntax.is_none() && self.parents.is_empty()
     }
@@ -253,6 +257,8 @@ impl<'a> Vertex<'a> {
     pub fn new(lhs_syntax: Option<&'a Syntax<'a>>, rhs_syntax: Option<&'a Syntax<'a>>) -> Self {
         let parents = Stack::new();
         Vertex {
+            neighbours: RefCell::new(None),
+            predecessor: RefCell::new(None),
             lhs_syntax,
             rhs_syntax,
             parents,
@@ -331,17 +337,34 @@ impl Edge {
     }
 }
 
-/// Calculate all the neighbours from `v` and write them to `buf`.
+fn allocate_if_new<'syn, 'b>(
-pub fn neighbours<'a, 'b>(
+    v: Vertex<'syn, 'b>,
-    v: &Vertex<'a>,
-    buf: &mut [Option<(Edge, &'b Vertex<'a>)>],
     alloc: &'b Bump,
-) {
+    seen: &mut FxHashMap<&Vertex<'syn, 'b>, &'b Vertex<'syn, 'b>>,
-    for item in &mut *buf {
+) -> &'b Vertex<'syn, 'b> {
-        *item = None;
+    match seen.get(&v) {
+        Some(existing) => *existing,
+        None => {
+            let allocated = alloc.alloc(v);
+            seen.insert(allocated, allocated);
+            allocated
+        }
     }
+}
 
-    let mut i = 0;
+/// Compute the neighbours of `v` if we haven't previously done so,
+/// write them to the .neighbours cell inside `v`, and return them.
+pub fn get_set_neighbours<'syn, 'b>(
+    v: &Vertex<'syn, 'b>,
+    alloc: &'b Bump,
+    seen: &mut FxHashMap<&Vertex<'syn, 'b>, &'b Vertex<'syn, 'b>>,
+) -> Vec<(Edge, &'b Vertex<'syn, 'b>)> {
+    match &*v.neighbours.borrow() {
+        Some(neighbours) => return neighbours.clone(),
+        None => {}
+    }
+
+    let mut res: Vec<(Edge, &Vertex)> = vec![];
 
     if v.lhs_syntax.is_none() && v.rhs_syntax.is_none() {
         if let Some((lhs_parent, rhs_parent, parents_next)) = try_pop_both(&v.parents) {
@@ -349,18 +372,23 @@ pub fn neighbours<'a, 'b>(
             // move up to the parent node.
 
             // Continue from sibling of parent.
-            buf[i] = Some((
+            res.push((
                 ExitDelimiterBoth,
-                alloc.alloc(Vertex {
+                allocate_if_new(
-                    lhs_syntax: lhs_parent.next_sibling(),
+                    Vertex {
-                    rhs_syntax: rhs_parent.next_sibling(),
+                        neighbours: RefCell::new(None),
-                    can_pop_either: can_pop_either_parent(&parents_next),
+                        predecessor: RefCell::new(None),
-                    parents: parents_next,
+                        lhs_syntax: lhs_parent.next_sibling(),
-                    lhs_parent_id: lhs_parent.parent().map(Syntax::id),
+                        rhs_syntax: rhs_parent.next_sibling(),
-                    rhs_parent_id: rhs_parent.parent().map(Syntax::id),
+                        can_pop_either: can_pop_either_parent(&parents_next),
-                }),
+                        parents: parents_next,
+                        lhs_parent_id: lhs_parent.parent().map(Syntax::id),
+                        rhs_parent_id: rhs_parent.parent().map(Syntax::id),
+                    },
+                    alloc,
+                    seen,
+                ),
             ));
-            i += 1;
         }
     }
 
@@ -369,18 +397,23 @@ pub fn neighbours<'a, 'b>(
             // Move to next after LHS parent.
 
             // Continue from sibling of parent.
-            buf[i] = Some((
+            res.push((
                 ExitDelimiterLHS,
-                alloc.alloc(Vertex {
+                allocate_if_new(
-                    lhs_syntax: lhs_parent.next_sibling(),
+                    Vertex {
-                    rhs_syntax: v.rhs_syntax,
+                        neighbours: RefCell::new(None),
-                    can_pop_either: can_pop_either_parent(&parents_next),
+                        predecessor: RefCell::new(None),
-                    parents: parents_next,
+                        lhs_syntax: lhs_parent.next_sibling(),
-                    lhs_parent_id: lhs_parent.parent().map(Syntax::id),
+                        rhs_syntax: v.rhs_syntax,
-                    rhs_parent_id: v.rhs_parent_id,
+                        can_pop_either: can_pop_either_parent(&parents_next),
-                }),
+                        parents: parents_next,
+                        lhs_parent_id: lhs_parent.parent().map(Syntax::id),
+                        rhs_parent_id: v.rhs_parent_id,
+                    },
+                    alloc,
+                    seen,
+                ),
             ));
-            i += 1;
         }
     }
 
@@ -389,18 +422,23 @@ pub fn neighbours<'a, 'b>(
             // Move to next after RHS parent.
 
             // Continue from sibling of parent.
-            buf[i] = Some((
+            res.push((
                 ExitDelimiterRHS,
-                alloc.alloc(Vertex {
+                allocate_if_new(
-                    lhs_syntax: v.lhs_syntax,
+                    Vertex {
-                    rhs_syntax: rhs_parent.next_sibling(),
+                        neighbours: RefCell::new(None),
-                    can_pop_either: can_pop_either_parent(&parents_next),
+                        predecessor: RefCell::new(None),
-                    parents: parents_next,
+                        lhs_syntax: v.lhs_syntax,
-                    lhs_parent_id: v.lhs_parent_id,
+                        rhs_syntax: rhs_parent.next_sibling(),
-                    rhs_parent_id: rhs_parent.parent().map(Syntax::id),
+                        can_pop_either: can_pop_either_parent(&parents_next),
-                }),
+                        parents: parents_next,
+                        lhs_parent_id: v.lhs_parent_id,
+                        rhs_parent_id: rhs_parent.parent().map(Syntax::id),
+                    },
+                    alloc,
+                    seen,
+                ),
             ));
-            i += 1;
         }
     }
 
@@ -411,18 +449,23 @@ pub fn neighbours<'a, 'b>(
                 .unsigned_abs();
 
             // Both nodes are equal, the happy case.
-            buf[i] = Some((
+            res.push((
                 UnchangedNode { depth_difference },
-                alloc.alloc(Vertex {
+                allocate_if_new(
-                    lhs_syntax: lhs_syntax.next_sibling(),
+                    Vertex {
-                    rhs_syntax: rhs_syntax.next_sibling(),
+                        neighbours: RefCell::new(None),
-                    parents: v.parents.clone(),
+                        predecessor: RefCell::new(None),
-                    lhs_parent_id: v.lhs_parent_id,
+                        lhs_syntax: lhs_syntax.next_sibling(),
-                    rhs_parent_id: v.rhs_parent_id,
+                        rhs_syntax: rhs_syntax.next_sibling(),
-                    can_pop_either: v.can_pop_either,
+                        parents: v.parents.clone(),
-                }),
+                        lhs_parent_id: v.lhs_parent_id,
+                        rhs_parent_id: v.rhs_parent_id,
+                        can_pop_either: v.can_pop_either,
+                    },
+                    alloc,
+                    seen,
+                ),
             ));
-            i += 1;
         }
 
         if let (
@@ -452,18 +495,23 @@ pub fn neighbours<'a, 'b>(
                     - rhs_syntax.num_ancestors() as i32)
                     .unsigned_abs();
 
-                buf[i] = Some((
+                res.push((
                     EnterUnchangedDelimiter { depth_difference },
-                    alloc.alloc(Vertex {
+                    allocate_if_new(
-                        lhs_syntax: lhs_next,
+                        Vertex {
-                        rhs_syntax: rhs_next,
+                            neighbours: RefCell::new(None),
-                        parents: parents_next,
+                            predecessor: RefCell::new(None),
-                        lhs_parent_id: Some(lhs_syntax.id()),
+                            lhs_syntax: lhs_next,
-                        rhs_parent_id: Some(rhs_syntax.id()),
+                            rhs_syntax: rhs_next,
-                        can_pop_either: false,
+                            parents: parents_next,
-                    }),
+                            lhs_parent_id: Some(lhs_syntax.id()),
+                            rhs_parent_id: Some(rhs_syntax.id()),
+                            can_pop_either: false,
+                        },
+                        alloc,
+                        seen,
+                    ),
                 ));
-                i += 1;
             }
         }
 
@@ -485,18 +533,23 @@ pub fn neighbours<'a, 'b>(
             if lhs_content != rhs_content {
                 let levenshtein_pct =
                     (normalized_levenshtein(lhs_content, rhs_content) * 100.0).round() as u8;
-                buf[i] = Some((
+                res.push((
                     ReplacedComment { levenshtein_pct },
-                    alloc.alloc(Vertex {
+                    allocate_if_new(
-                        lhs_syntax: lhs_syntax.next_sibling(),
+                        Vertex {
-                        rhs_syntax: rhs_syntax.next_sibling(),
+                            neighbours: RefCell::new(None),
-                        parents: v.parents.clone(),
+                            predecessor: RefCell::new(None),
-                        lhs_parent_id: v.lhs_parent_id,
+                            lhs_syntax: lhs_syntax.next_sibling(),
-                        rhs_parent_id: v.rhs_parent_id,
+                            rhs_syntax: rhs_syntax.next_sibling(),
-                        can_pop_either: v.can_pop_either,
+                            parents: v.parents.clone(),
-                    }),
+                            lhs_parent_id: v.lhs_parent_id,
+                            rhs_parent_id: v.rhs_parent_id,
+                            can_pop_either: v.can_pop_either,
+                        },
+                        alloc,
+                        seen,
+                    ),
                 ));
-                i += 1;
             }
         }
     }
@@ -505,22 +558,27 @@ pub fn neighbours<'a, 'b>(
         match lhs_syntax {
             // Step over this novel atom.
             Syntax::Atom { .. } => {
-                buf[i] = Some((
+                res.push((
                     NovelAtomLHS {
                         // TODO: should this apply if prev is a parent
                         // node rather than a sibling?
                         contiguous: lhs_syntax.prev_is_contiguous(),
                     },
-                    alloc.alloc(Vertex {
+                    allocate_if_new(
-                        lhs_syntax: lhs_syntax.next_sibling(),
+                        Vertex {
-                        rhs_syntax: v.rhs_syntax,
+                            neighbours: RefCell::new(None),
-                        parents: v.parents.clone(),
+                            predecessor: RefCell::new(None),
-                        lhs_parent_id: v.lhs_parent_id,
+                            lhs_syntax: lhs_syntax.next_sibling(),
-                        rhs_parent_id: v.rhs_parent_id,
+                            rhs_syntax: v.rhs_syntax,
-                        can_pop_either: v.can_pop_either,
+                            parents: v.parents.clone(),
-                    }),
+                            lhs_parent_id: v.lhs_parent_id,
+                            rhs_parent_id: v.rhs_parent_id,
+                            can_pop_either: v.can_pop_either,
+                        },
+                        alloc,
+                        seen,
+                    ),
                 ));
-                i += 1;
             }
             // Step into this partially/fully novel list.
             Syntax::List { children, .. } => {
@@ -528,20 +586,25 @@ pub fn neighbours<'a, 'b>(
 
                 let parents_next = push_lhs_delimiter(&v.parents, lhs_syntax);
 
-                buf[i] = Some((
+                res.push((
                     EnterNovelDelimiterLHS {
                         contiguous: lhs_syntax.prev_is_contiguous(),
                     },
-                    alloc.alloc(Vertex {
+                    allocate_if_new(
-                        lhs_syntax: lhs_next,
+                        Vertex {
-                        rhs_syntax: v.rhs_syntax,
+                            neighbours: RefCell::new(None),
-                        parents: parents_next,
+                            predecessor: RefCell::new(None),
-                        lhs_parent_id: Some(lhs_syntax.id()),
+                            lhs_syntax: lhs_next,
-                        rhs_parent_id: v.rhs_parent_id,
+                            rhs_syntax: v.rhs_syntax,
-                        can_pop_either: true,
+                            parents: parents_next,
-                    }),
+                            lhs_parent_id: Some(lhs_syntax.id()),
+                            rhs_parent_id: v.rhs_parent_id,
+                            can_pop_either: true,
+                        },
+                        alloc,
+                        seen,
+                    ),
                 ));
-                i += 1;
             }
         }
     }
@@ -550,20 +613,25 @@ pub fn neighbours<'a, 'b>(
         match rhs_syntax {
             // Step over this novel atom.
             Syntax::Atom { .. } => {
-                buf[i] = Some((
+                res.push((
                     NovelAtomRHS {
                         contiguous: rhs_syntax.prev_is_contiguous(),
                     },
-                    alloc.alloc(Vertex {
+                    allocate_if_new(
-                        lhs_syntax: v.lhs_syntax,
+                        Vertex {
-                        rhs_syntax: rhs_syntax.next_sibling(),
+                            neighbours: RefCell::new(None),
-                        parents: v.parents.clone(),
+                            predecessor: RefCell::new(None),
-                        lhs_parent_id: v.lhs_parent_id,
+                            lhs_syntax: v.lhs_syntax,
-                        rhs_parent_id: v.rhs_parent_id,
+                            rhs_syntax: rhs_syntax.next_sibling(),
-                        can_pop_either: v.can_pop_either,
+                            parents: v.parents.clone(),
-                    }),
+                            lhs_parent_id: v.lhs_parent_id,
+                            rhs_parent_id: v.rhs_parent_id,
+                            can_pop_either: v.can_pop_either,
+                        },
+                        alloc,
+                        seen,
+                    ),
                 ));
-                i += 1;
             }
             // Step into this partially/fully novel list.
             Syntax::List { children, .. } => {
@@ -571,30 +639,41 @@ pub fn neighbours<'a, 'b>(
 
                 let parents_next = push_rhs_delimiter(&v.parents, rhs_syntax);
 
-                buf[i] = Some((
+                res.push((
                     EnterNovelDelimiterRHS {
                         contiguous: rhs_syntax.prev_is_contiguous(),
                     },
-                    alloc.alloc(Vertex {
+                    allocate_if_new(
-                        lhs_syntax: v.lhs_syntax,
+                        Vertex {
-                        rhs_syntax: rhs_next,
+                            neighbours: RefCell::new(None),
-                        parents: parents_next,
+                            predecessor: RefCell::new(None),
-                        lhs_parent_id: v.lhs_parent_id,
+                            lhs_syntax: v.lhs_syntax,
-                        rhs_parent_id: Some(rhs_syntax.id()),
+                            rhs_syntax: rhs_next,
-                        can_pop_either: true,
+                            parents: parents_next,
-                    }),
+                            lhs_parent_id: v.lhs_parent_id,
+                            rhs_parent_id: Some(rhs_syntax.id()),
+                            can_pop_either: true,
+                        },
+                        alloc,
+                        seen,
+                    ),
                 ));
-                i += 1;
             }
         }
     }
     assert!(
-        i > 0,
+        res.len() > 0,
         "Must always find some next steps if node is not the end"
     );
+
+    v.neighbours.replace(Some(res.clone()));
+    res
 }
 
-pub fn populate_change_map<'a>(route: &[(Edge, Vertex<'a>)], change_map: &mut ChangeMap<'a>) {
+pub fn populate_change_map<'a, 'b>(
+    route: &[(Edge, &'b Vertex<'a, 'b>)],
+    change_map: &mut ChangeMap<'a>,
+) {
     for (e, v) in route {
         match e {
             ExitDelimiterBoth | ExitDelimiterLHS | ExitDelimiterRHS => {