sascha
/
difftastic
mirror of https://github.com/Wilfred/difftastic/
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Make Stack be allocated on the arena. 

This fixes another memory leak, and also removes the need for
refcounting the Stack objects and the Node objects they point to.
pull/708/head
Steinar H. Gunderson 2024-04-08 13:57:36 +07:00 committed by Wilfred Hughes
parent 4fb1478817
commit 302570591f
2 changed files with 77 additions and 58 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

117
src/diff/graph.rs
Unescape Escape View File

@ -57,7 +57,7 @@ pub(crate) struct Vertex<'s, 'b> {
// from SyntaxId to &Syntax.
pub(crate) lhs_syntax: Option<&'s Syntax<'s>>,
pub(crate) rhs_syntax: Option<&'s Syntax<'s>>,
parents: Stack<EnteredDelimiter<'s>>,
parents: Stack<'b, EnteredDelimiter<'s, 'b>>,
lhs_parent_id: Option<SyntaxId>,
rhs_parent_id: Option<SyntaxId>,
}
@ -115,12 +115,12 @@ impl<'s, 'b> Hash for Vertex<'s, 'b> {
/// Tracks entering syntax List nodes.
#[derive(Clone, PartialEq)]
enum EnteredDelimiter<'s> {
enum EnteredDelimiter<'s, 'b> {
/// If we've entered the LHS or RHS separately, we can pop either
/// side independently.
///
/// Assumes that at least one stack is non-empty.
PopEither((Stack<&'s Syntax<'s>>, Stack<&'s Syntax<'s>>)),
PopEither((Stack<'b, &'s Syntax<'s>>, Stack<'b, &'s Syntax<'s>>)),
/// If we've entered the LHS and RHS together, we must pop both
/// sides together too. Otherwise we'd consider the following case to have no changes.
///
@ -131,7 +131,7 @@ enum EnteredDelimiter<'s> {
PopBoth((&'s Syntax<'s>, &'s Syntax<'s>)),
}
impl<'s> fmt::Debug for EnteredDelimiter<'s> {
impl<'s, 'b> fmt::Debug for EnteredDelimiter<'s, 'b> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let desc = match self {
EnteredDelimiter::PopEither((lhs_delims, rhs_delims)) => {
@ -147,21 +147,26 @@ impl<'s> fmt::Debug for EnteredDelimiter<'s> {
}
}
fn push_both_delimiters<'s>(
entered: &Stack<EnteredDelimiter<'s>>,
fn push_both_delimiters<'s, 'b>(
entered: &Stack<'b, EnteredDelimiter<'s, 'b>>,
lhs_delim: &'s Syntax<'s>,
rhs_delim: &'s Syntax<'s>,
) -> Stack<EnteredDelimiter<'s>> {
entered.push(EnteredDelimiter::PopBoth((lhs_delim, rhs_delim)))
alloc: &'b Bump,
) -> Stack<'b, EnteredDelimiter<'s, 'b>> {
entered.push(EnteredDelimiter::PopBoth((lhs_delim, rhs_delim)), alloc)
}
fn can_pop_either_parent(entered: &Stack<EnteredDelimiter>) -> bool {
matches!(entered.peek(), Some(EnteredDelimiter::PopEither(_)))
}
fn try_pop_both<'s>(
entered: &Stack<EnteredDelimiter<'s>>,
) -> Option<(&'s Syntax<'s>, &'s Syntax<'s>, Stack<EnteredDelimiter<'s>>)> {
fn try_pop_both<'s, 'b>(
entered: &Stack<'b, EnteredDelimiter<'s, 'b>>,
) -> Option<(
&'s Syntax<'s>,
&'s Syntax<'s>,
Stack<'b, EnteredDelimiter<'s, 'b>>,
)> {
match entered.peek() {
Some(EnteredDelimiter::PopBoth((lhs_delim, rhs_delim))) => {
Some((lhs_delim, rhs_delim, entered.pop().unwrap()))
@ -170,9 +175,10 @@ fn try_pop_both<'s>(
}
}
fn try_pop_lhs<'s>(
entered: &Stack<EnteredDelimiter<'s>>,
) -> Option<(&'s Syntax<'s>, Stack<EnteredDelimiter<'s>>)> {
fn try_pop_lhs<'s, 'b>(
entered: &Stack<'b, EnteredDelimiter<'s, 'b>>,
alloc: &'b Bump,
) -> Option<(&'s Syntax<'s>, Stack<'b, EnteredDelimiter<'s, 'b>>)> {
match entered.peek() {
Some(EnteredDelimiter::PopEither((lhs_delims, rhs_delims))) => match lhs_delims.peek() {
Some(lhs_delim) => {
@ -180,10 +186,10 @@ fn try_pop_lhs<'s>(
let new_lhs_delims = lhs_delims.pop().unwrap();
if !new_lhs_delims.is_empty() || !rhs_delims.is_empty() {
entered = entered.push(EnteredDelimiter::PopEither((
new_lhs_delims,
rhs_delims.clone(),
)));
entered = entered.push(
EnteredDelimiter::PopEither((new_lhs_delims, rhs_delims.clone())),
alloc,
);
}
Some((lhs_delim, entered))
@ -194,9 +200,10 @@ fn try_pop_lhs<'s>(
}
}
fn try_pop_rhs<'s>(
entered: &Stack<EnteredDelimiter<'s>>,
) -> Option<(&'s Syntax<'s>, Stack<EnteredDelimiter<'s>>)> {
fn try_pop_rhs<'s, 'b>(
entered: &Stack<'b, EnteredDelimiter<'s, 'b>>,
alloc: &'b Bump,
) -> Option<(&'s Syntax<'s>, Stack<'b, EnteredDelimiter<'s, 'b>>)> {
match entered.peek() {
Some(EnteredDelimiter::PopEither((lhs_delims, rhs_delims))) => match rhs_delims.peek() {
Some(rhs_delim) => {
@ -204,10 +211,10 @@ fn try_pop_rhs<'s>(
let new_rhs_delims = rhs_delims.pop().unwrap();
if !lhs_delims.is_empty() || !new_rhs_delims.is_empty() {
entered = entered.push(EnteredDelimiter::PopEither((
lhs_delims.clone(),
new_rhs_delims,
)));
entered = entered.push(
EnteredDelimiter::PopEither((lhs_delims.clone(), new_rhs_delims)),
alloc,
);
}
Some((rhs_delim, entered))
@ -218,33 +225,37 @@ fn try_pop_rhs<'s>(
}
}
fn push_lhs_delimiter<'s>(
entered: &Stack<EnteredDelimiter<'s>>,
fn push_lhs_delimiter<'s, 'b>(
entered: &Stack<'b, EnteredDelimiter<'s, 'b>>,
delimiter: &'s Syntax<'s>,
) -> Stack<EnteredDelimiter<'s>> {
alloc: &'b Bump,
) -> Stack<'b, EnteredDelimiter<'s, 'b>> {
match entered.peek() {
Some(EnteredDelimiter::PopEither((lhs_delims, rhs_delims))) => entered.pop().unwrap().push(
EnteredDelimiter::PopEither((lhs_delims.push(delimiter), rhs_delims.clone())),
EnteredDelimiter::PopEither((lhs_delims.push(delimiter, alloc), rhs_delims.clone())),
alloc,
),
_ => entered.push(
EnteredDelimiter::PopEither((Stack::new().push(delimiter, alloc), Stack::new())),
alloc,
),
_ => entered.push(EnteredDelimiter::PopEither((
Stack::new().push(delimiter),
Stack::new(),
))),
}
}
fn push_rhs_delimiter<'s>(
entered: &Stack<EnteredDelimiter<'s>>,
fn push_rhs_delimiter<'s, 'b>(
entered: &Stack<'b, EnteredDelimiter<'s, 'b>>,
delimiter: &'s Syntax<'s>,
) -> Stack<EnteredDelimiter<'s>> {
alloc: &'b Bump,
) -> Stack<'b, EnteredDelimiter<'s, 'b>> {
match entered.peek() {
Some(EnteredDelimiter::PopEither((lhs_delims, rhs_delims))) => entered.pop().unwrap().push(
EnteredDelimiter::PopEither((lhs_delims.clone(), rhs_delims.push(delimiter))),
EnteredDelimiter::PopEither((lhs_delims.clone(), rhs_delims.push(delimiter, alloc))),
alloc,
),
_ => entered.push(
EnteredDelimiter::PopEither((Stack::new(), Stack::new().push(delimiter, alloc))),
alloc,
),
_ => entered.push(EnteredDelimiter::PopEither((
Stack::new(),
Stack::new().push(delimiter),
))),
}
}
@ -414,18 +425,19 @@ fn looks_like_punctuation(node: &Syntax) -> bool {
/// Pop as many parents of `lhs_node` and `rhs_node` as
/// possible. Return the new syntax nodes and parents.
fn pop_all_parents<'s>(
fn pop_all_parents<'s, 'b>(
lhs_node: Option<&'s Syntax<'s>>,
rhs_node: Option<&'s Syntax<'s>>,
lhs_parent_id: Option<SyntaxId>,
rhs_parent_id: Option<SyntaxId>,
parents: &Stack<EnteredDelimiter<'s>>,
parents: &Stack<'b, EnteredDelimiter<'s, 'b>>,
alloc: &'b Bump,
) -> (
Option<&'s Syntax<'s>>,
Option<&'s Syntax<'s>>,
Option<SyntaxId>,
Option<SyntaxId>,
Stack<EnteredDelimiter<'s>>,
Stack<'b, EnteredDelimiter<'s, 'b>>,
) {
let mut lhs_node = lhs_node;
let mut rhs_node = rhs_node;
@ -435,7 +447,7 @@ fn pop_all_parents<'s>(
loop {
if lhs_node.is_none() {
if let Some((lhs_parent, parents_next)) = try_pop_lhs(&parents) {
if let Some((lhs_parent, parents_next)) = try_pop_lhs(&parents, alloc) {
// Move to next after LHS parent.
// Continue from sibling of parent.
@ -447,7 +459,7 @@ fn pop_all_parents<'s>(
}
if rhs_node.is_none() {
if let Some((rhs_parent, parents_next)) = try_pop_rhs(&parents) {
if let Some((rhs_parent, parents_next)) = try_pop_rhs(&parents, alloc) {
// Move to next after RHS parent.
// Continue from sibling of parent.
@ -508,6 +520,7 @@ pub(crate) fn set_neighbours<'s, 'b>(
v.lhs_parent_id,
v.rhs_parent_id,
&v.parents,
alloc,
);
neighbours.push((
@ -552,7 +565,7 @@ pub(crate) fn set_neighbours<'s, 'b>(
let rhs_next = rhs_children.first().copied();
// TODO: be consistent between parents_next and next_parents.
let parents_next = push_both_delimiters(&v.parents, lhs_syntax, rhs_syntax);
let parents_next = push_both_delimiters(&v.parents, lhs_syntax, rhs_syntax, alloc);
let depth_difference = (lhs_syntax.num_ancestors() as i32
- rhs_syntax.num_ancestors() as i32)
@ -567,6 +580,7 @@ pub(crate) fn set_neighbours<'s, 'b>(
Some(lhs_syntax.id()),
Some(rhs_syntax.id()),
&parents_next,
alloc,
);
neighbours.push((
@ -619,6 +633,7 @@ pub(crate) fn set_neighbours<'s, 'b>(
v.lhs_parent_id,
v.rhs_parent_id,
&v.parents,
alloc,
);
neighbours.push((
edge,
@ -651,6 +666,7 @@ pub(crate) fn set_neighbours<'s, 'b>(
v.lhs_parent_id,
v.rhs_parent_id,
&v.parents,
alloc,
);
neighbours.push((
@ -674,7 +690,7 @@ pub(crate) fn set_neighbours<'s, 'b>(
Syntax::List { children, .. } => {
let lhs_next = children.first().copied();
let parents_next = push_lhs_delimiter(&v.parents, lhs_syntax);
let parents_next = push_lhs_delimiter(&v.parents, lhs_syntax, alloc);
let (lhs_syntax, rhs_syntax, lhs_parent_id, rhs_parent_id, parents) =
pop_all_parents(
@ -683,6 +699,7 @@ pub(crate) fn set_neighbours<'s, 'b>(
Some(lhs_syntax.id()),
v.rhs_parent_id,
&parents_next,
alloc,
);
neighbours.push((
@ -716,6 +733,7 @@ pub(crate) fn set_neighbours<'s, 'b>(
v.lhs_parent_id,
v.rhs_parent_id,
&v.parents,
alloc,
);
neighbours.push((
@ -738,7 +756,7 @@ pub(crate) fn set_neighbours<'s, 'b>(
// Step into this partially/fully novel list.
Syntax::List { children, .. } => {
let rhs_next = children.first().copied();
let parents_next = push_rhs_delimiter(&v.parents, rhs_syntax);
let parents_next = push_rhs_delimiter(&v.parents, rhs_syntax, alloc);
let (lhs_syntax, rhs_syntax, lhs_parent_id, rhs_parent_id, parents) =
pop_all_parents(
@ -747,6 +765,7 @@ pub(crate) fn set_neighbours<'s, 'b>(
v.lhs_parent_id,
Some(rhs_syntax.id()),
&parents_next,
alloc,
);
neighbours.push((

18
src/diff/stack.rs
Unescape Escape View File

@ -1,9 +1,9 @@
use std::rc::Rc;
use bumpalo::Bump;
#[derive(Debug, Clone, Default, PartialEq, Eq)]
struct Node<T> {
struct Node<'b, T> {
val: T,
next: Option<Rc<Node<T>>>,
next: Option<&'b Node<'b, T>>,
}
/// A persistent stack.
@ -11,11 +11,11 @@ struct Node<T> {
/// This is similar to `Stack` from the rpds crate, but it's faster
/// and uses less memory.
#[derive(Debug, Clone, Default, PartialEq, Eq)]
pub(crate) struct Stack<T> {
head: Option<Rc<Node<T>>>,
pub(crate) struct Stack<'b, T> {
head: Option<&'b Node<'b, T>>,
}
impl<T> Stack<T> {
impl<'b, T> Stack<'b, T> {
pub(crate) fn new() -> Self {
Self { head: None }
}
@ -24,15 +24,15 @@ impl<T> Stack<T> {
self.head.as_deref().map(|n| &n.val)
}
pub(crate) fn pop(&self) -> Option<Stack<T>> {
pub(crate) fn pop(&self) -> Option<Stack<'b, T>> {
self.head.as_deref().map(|n| Self {
head: n.next.clone(),
})
}
pub(crate) fn push(&self, v: T) -> Stack<T> {
pub(crate) fn push(&self, v: T, alloc: &'b Bump) -> Stack<'b, T> {
Self {
head: Some(Rc::new(Node {
head: Some(alloc.alloc(Node {
val: v,
next: self.head.clone(),
})),