From 3584fbe6a986b84702212f56fb97a4a5c8db6406 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Even=20Olsson=20Rogstadkj=C3=A6rnet?= Date: Sun, 8 Oct 2023 13:05:01 +0200 Subject: [PATCH 1/2] Use Slab instead of BTreeMap for branch container --- Cargo.toml | 2 + src/history.rs | 107 ++++++++++++++++++++++---------------- src/history/checkpoint.rs | 3 +- src/history/display.rs | 5 +- src/lib.rs | 2 + 5 files changed, 70 insertions(+), 49 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 5790913..fc922d7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,6 +14,7 @@ edition = "2021" [dependencies] colored = { version = "2", optional = true } serde = { version = "1", optional = true, default-features = false, features = ["derive"] } +slab = { version = "0.4", default-features = false } [dev-dependencies] chrono = "0.4" @@ -22,6 +23,7 @@ chrono = "0.4" default = ["std"] std = ["alloc", "serde?/std"] alloc = ["serde?/alloc"] +serde = ["dep:serde", "slab/serde"] [badges] maintenance = { status = "actively-developed" } diff --git a/src/history.rs b/src/history.rs index d901bba..a17b936 100644 --- a/src/history.rs +++ b/src/history.rs @@ -12,12 +12,14 @@ pub use queue::Queue; use crate::socket::Slot; use crate::{At, Edit, Entry, Event, Record}; -use alloc::collections::{BTreeMap, VecDeque}; +use alloc::collections::VecDeque; use alloc::string::String; use alloc::vec::Vec; use core::fmt; +use core::mem; #[cfg(feature = "serde")] use serde::{Deserialize, Serialize}; +use slab::Slab; /// A history tree of [`Edit`] commands. /// @@ -55,10 +57,9 @@ use serde::{Deserialize, Serialize}; #[derive(Clone, Debug)] pub struct History { root: usize, - next: usize, saved: Option, record: Record, - branches: BTreeMap>, + branches: Slab>, } impl History { @@ -151,9 +152,9 @@ impl History { /// This can be used in combination with [`History::go_to`] to go to the next branch. pub fn next_branch_head(&self) -> Option { self.branches - .range(self.root + 1..) - .next() - .map(|(&id, branch)| At::new(id, branch.parent.index + 1)) + .iter() + .find(|&(id, _)| id > self.root) + .map(|(id, branch)| At::new(id, branch.parent.index + 1)) } /// Returns the head of the previous branch in the history. @@ -162,9 +163,9 @@ impl History { /// This can be used in combination with [`History::go_to`] to go to the previous branch. pub fn prev_branch_head(&self) -> Option { self.branches - .range(..self.root) - .next_back() - .map(|(&id, branch)| At::new(id, branch.parent.index + 1)) + .iter() + .rfind(|&(id, _)| id < self.root) + .map(|(id, branch)| At::new(id, branch.parent.index + 1)) } /// Returns the entry at the index in the current root branch. @@ -181,14 +182,14 @@ impl History { /// Returns the branch with the given id. pub fn get_branch(&self, id: usize) -> Option<&Branch> { - self.branches.get(&id) + self.branches.get(id) } /// Returns an iterator over the branches in the history. /// /// This does not include the current root branch. pub fn branches(&self) -> impl Iterator)> { - self.branches.iter().map(|(&k, v)| (k, v)) + self.branches.iter() } /// Returns a queue. @@ -213,14 +214,14 @@ impl History { .filter(|&(_, child)| child.parent == at) .map(|(id, _)| id) .collect(); - while let Some(parent) = dead.pop() { + while let Some(id) = dead.pop() { // Remove the dead branch. - self.branches.remove(&parent).unwrap(); - self.saved = self.saved.filter(|saved| saved.root != parent); + self.branches.remove(id); + self.saved = self.saved.filter(|s| s.root != id); // Add the children of the dead branch so they are removed too. dead.extend( self.branches() - .filter(|&(_, child)| child.parent.root == parent) + .filter(|&(_, child)| child.parent.root == id) .map(|(id, _)| id), ) } @@ -228,11 +229,12 @@ impl History { fn mk_path(&mut self, mut to: usize) -> Option)>> { debug_assert_ne!(self.root, to); - let mut dest = self.branches.remove(&to)?; + let mut dest = self.nil_replace(to)?; + let mut i = dest.parent.root; let mut path = alloc::vec![(to, dest)]; while i != self.root { - dest = self.branches.remove(&i).unwrap(); + dest = self.nil_replace(i).unwrap(); to = i; i = dest.parent.root; path.push((to, dest)); @@ -240,6 +242,12 @@ impl History { Some(path.into_iter().rev()) } + + fn nil_replace(&mut self, id: usize) -> Option> { + let dest = self.branches.get_mut(id)?; + let dest = mem::replace(dest, Branch::NIL); + Some(dest) + } } impl History { @@ -258,12 +266,13 @@ impl History { /// Removes all edits from the history without undoing them. pub fn clear(&mut self) { let old_root = self.root; - self.root = 0; - self.next = 1; self.saved = None; self.record.clear(); self.branches.clear(); - self.record.socket.emit_if(old_root != 0, || Event::Root(0)); + self.root = self.branches.insert(Branch::NIL); + self.record + .socket + .emit_if(old_root != self.root, || Event::Root(self.root)); } fn set_root(&mut self, new: At, rm_saved: Option) { @@ -283,9 +292,9 @@ impl History { // be children of the new root. All branches that are above should still be // children of the old root. self.branches - .values_mut() - .filter(|child| child.parent.root == self.root && child.parent.index <= new.index) - .for_each(|child| child.parent.root = new.root); + .iter_mut() + .filter(|(_, child)| child.parent.root == self.root && child.parent.index <= new.index) + .for_each(|(_, child)| child.parent.root = new.root); match (self.saved, rm_saved) { (Some(saved), None) if saved.root == new.root => { @@ -304,14 +313,13 @@ impl History { self.record.socket.emit(|| Event::Root(new.root)); } - fn jump_to(&mut self, root: usize) { - let mut branch = self.branches.remove(&root).unwrap(); + fn jump_to_and_discard(&mut self, root: usize) { + let mut branch = self.branches.remove(root); debug_assert_eq!(branch.parent, self.head()); let new = At::new(root, self.record.head()); - let (tail, rm_saved) = self.record.rm_tail(); + let (_, rm_saved) = self.record.rm_tail(); self.record.entries.append(&mut branch.entries); - self.branches.insert(self.root, Branch::new(new, tail)); self.set_root(new, rm_saved); } } @@ -327,17 +335,20 @@ impl History { let root = self.root; self.rm_child_of(At::new(root, 0)); self.branches - .values_mut() - .filter(|child| child.parent.root == root) - .for_each(|child| child.parent.index -= 1); + .iter_mut() + .filter(|(_, child)| child.parent.root == root) + .for_each(|(_, child)| child.parent.index -= 1); } - // Handle new branch. + // Handle new branch by putting the tail into the empty root branch + // before we swap the root with the new branch. if !tail.is_empty() { - let new_root = self.next; - self.next += 1; - let new = At::new(new_root, head.index); - self.branches.insert(head.root, Branch::new(new, tail)); + let next = self.branches.insert(Branch::NIL); + let new = At::new(next, head.index); + let root = self.branches.get_mut(head.root).unwrap(); + debug_assert!(root.entries.is_empty()); + root.parent = new; + root.entries = tail; self.set_root(new, rm_saved); } @@ -386,7 +397,10 @@ impl History { let (_, _, entries, rm_saved) = self.record.redo_and_push(target, entry); if !entries.is_empty() { let new = At::new(id, index); - self.branches.insert(self.root, Branch::new(new, entries)); + let root = self.branches.get_mut(self.root).unwrap(); + debug_assert!(root.entries.is_empty()); + root.parent = new; + root.entries = entries; self.set_root(new, rm_saved); } } @@ -420,12 +434,13 @@ impl Default for History { impl From> for History { fn from(record: Record) -> Self { + let mut branches = Slab::new(); + let root = branches.insert(Branch::NIL); History { - root: 0, - next: 1, + root, saved: None, record, - branches: BTreeMap::new(), + branches, } } } @@ -445,10 +460,10 @@ pub struct Branch { } impl Branch { - fn new(parent: At, entries: VecDeque>) -> Branch { - debug_assert!(!entries.is_empty()); - Branch { parent, entries } - } + const NIL: Branch = Branch { + parent: At::NIL, + entries: VecDeque::new(), + }; /// Returns the parent edit of the branch. pub fn parent(&self) -> At { @@ -456,11 +471,15 @@ impl Branch { } /// Returns the number of edits in the branch. - #[allow(clippy::len_without_is_empty)] pub fn len(&self) -> usize { self.entries.len() } + /// Returns `true` if the branch is empty. + pub fn is_empty(&self) -> bool { + self.entries.is_empty() + } + /// Returns the edit at the index. pub fn get_entry(&self, index: usize) -> Option<&Entry> { self.entries.get(index) diff --git a/src/history/checkpoint.rs b/src/history/checkpoint.rs index fa8f791..a19b2c0 100644 --- a/src/history/checkpoint.rs +++ b/src/history/checkpoint.rs @@ -60,8 +60,7 @@ impl Checkpoint<'_, E, S> { if root == branch { self.history.record.entries.pop_back(); } else { - self.history.jump_to(branch); - self.history.branches.remove(&root).unwrap(); + self.history.jump_to_and_discard(branch); } Some(output) } diff --git a/src/history/display.rs b/src/history/display.rs index 2566067..dfb514c 100644 --- a/src/history/display.rs +++ b/src/history/display.rs @@ -98,10 +98,9 @@ impl Display<'_, E, S> { level: usize, #[cfg(feature = "std")] now: SystemTime, ) -> fmt::Result { - for (&i, branch) in self + for (i, branch) in self .history - .branches - .iter() + .branches() .filter(|(_, branch)| branch.parent == at) { for (j, entry) in branch.entries.iter().enumerate().rev() { diff --git a/src/lib.rs b/src/lib.rs index 5538589..d3ec174 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -135,6 +135,8 @@ pub struct At { #[cfg(feature = "alloc")] impl At { + const NIL: At = At::new(0, 0); + /// Creates a new `At` with the provided root and index. pub const fn new(root: usize, index: usize) -> At { At { root, index } From cf77dca6bd856da7514454a0e0a246f77694ef2c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Even=20Olsson=20Rogstadkj=C3=A6rnet?= Date: Tue, 10 Oct 2023 18:07:36 +0200 Subject: [PATCH 2/2] Clean up checkpoint code and move jump method --- src/history.rs | 12 ------------ src/history/checkpoint.rs | 20 +++++++++++++------- src/record/checkpoint.rs | 16 +++++++++------- 3 files changed, 22 insertions(+), 26 deletions(-) diff --git a/src/history.rs b/src/history.rs index a17b936..54eeb8f 100644 --- a/src/history.rs +++ b/src/history.rs @@ -186,8 +186,6 @@ impl History { } /// Returns an iterator over the branches in the history. - /// - /// This does not include the current root branch. pub fn branches(&self) -> impl Iterator)> { self.branches.iter() } @@ -312,16 +310,6 @@ impl History { self.root = new.root; self.record.socket.emit(|| Event::Root(new.root)); } - - fn jump_to_and_discard(&mut self, root: usize) { - let mut branch = self.branches.remove(root); - debug_assert_eq!(branch.parent, self.head()); - - let new = At::new(root, self.record.head()); - let (_, rm_saved) = self.record.rm_tail(); - self.record.entries.append(&mut branch.entries); - self.set_root(new, rm_saved); - } } impl History { diff --git a/src/history/checkpoint.rs b/src/history/checkpoint.rs index a19b2c0..0b4c62b 100644 --- a/src/history/checkpoint.rs +++ b/src/history/checkpoint.rs @@ -1,4 +1,4 @@ -use crate::{Edit, History, Slot}; +use crate::{At, Edit, History, Slot}; use alloc::vec::Vec; #[derive(Debug)] @@ -31,8 +31,7 @@ impl Checkpoint<'_, E, S> { impl Checkpoint<'_, E, S> { /// Calls the [`History::edit`] method. pub fn edit(&mut self, target: &mut E::Target, edit: E) -> E::Output { - let root = self.history.root; - self.entries.push(CheckpointEntry::Edit(root)); + self.entries.push(CheckpointEntry::Edit(self.history.root)); self.history.edit(target, edit) } @@ -54,13 +53,20 @@ impl Checkpoint<'_, E, S> { .into_iter() .rev() .filter_map(|entry| match entry { - CheckpointEntry::Edit(branch) => { + CheckpointEntry::Edit(root) => { let output = self.history.undo(target)?; - let root = self.history.root; - if root == branch { + if self.history.root == root { self.history.record.entries.pop_back(); } else { - self.history.jump_to_and_discard(branch); + // If a new root was created when we edited earlier, + // we remove it and append the entries to the previous root. + let mut branch = self.history.branches.remove(root); + debug_assert_eq!(branch.parent, self.history.head()); + + let new = At::new(root, self.history.record.head()); + let (_, rm_saved) = self.history.record.rm_tail(); + self.history.record.entries.append(&mut branch.entries); + self.history.set_root(new, rm_saved); } Some(output) } diff --git a/src/record/checkpoint.rs b/src/record/checkpoint.rs index 6a06f95..96cf79a 100644 --- a/src/record/checkpoint.rs +++ b/src/record/checkpoint.rs @@ -4,7 +4,10 @@ use alloc::vec::Vec; #[derive(Debug)] enum CheckpointEntry { - Edit(Option, VecDeque>), + Edit { + saved: Option, + tail: VecDeque>, + }, Undo, Redo, } @@ -32,9 +35,8 @@ impl Checkpoint<'_, E, S> { impl Checkpoint<'_, E, S> { /// Calls the `apply` method. pub fn edit(&mut self, target: &mut E::Target, edit: E) -> E::Output { - let saved = self.record.saved; - let (output, _, tail, _) = self.record.edit_and_push(target, Entry::new(edit)); - self.entries.push(CheckpointEntry::Edit(saved, tail)); + let (output, _, tail, saved) = self.record.edit_and_push(target, Entry::new(edit)); + self.entries.push(CheckpointEntry::Edit { saved, tail }); output } @@ -58,11 +60,11 @@ impl Checkpoint<'_, E, S> { .into_iter() .rev() .filter_map(|entry| match entry { - CheckpointEntry::Edit(saved, mut entries) => { + CheckpointEntry::Edit { saved, mut tail } => { let output = self.record.undo(target)?; self.record.entries.pop_back(); - self.record.entries.append(&mut entries); - self.record.saved = saved; + self.record.entries.append(&mut tail); + self.record.saved = self.record.saved.or(saved); Some(output) } CheckpointEntry::Undo => self.record.redo(target),