Merge pull request #82 from zed-industries/faster-undo

Antonio Scandurra created

Batch undo

Change summary

zed/src/editor/buffer.rs | 252 ++++++++++++++++++++++++++++-------------
1 file changed, 171 insertions(+), 81 deletions(-)

Detailed changes

zed/src/editor/buffer.rs 🔗

@@ -35,8 +35,6 @@ use std::{
     time::{Duration, Instant, SystemTime, UNIX_EPOCH},
 };
 
-const UNDO_GROUP_INTERVAL: Duration = Duration::from_millis(300);
-
 #[derive(Clone, Default)]
 struct DeterministicState;
 
@@ -134,17 +132,67 @@ struct SyntaxTree {
     version: time::Global,
 }
 
-#[derive(Clone)]
+#[derive(Clone, Debug)]
 struct Transaction {
     start: time::Global,
+    end: time::Global,
     buffer_was_dirty: bool,
     edits: Vec<time::Local>,
+    ranges: Vec<Range<usize>>,
     selections_before: Option<(SelectionSetId, Arc<[Selection]>)>,
     selections_after: Option<(SelectionSetId, Arc<[Selection]>)>,
     first_edit_at: Instant,
     last_edit_at: Instant,
 }
 
+impl Transaction {
+    fn push_edit(&mut self, edit: &EditOperation) {
+        self.edits.push(edit.timestamp.local());
+        self.end.observe(edit.timestamp.local());
+
+        let mut other_ranges = edit.ranges.iter().peekable();
+        let mut new_ranges: Vec<Range<usize>> = Vec::new();
+        let insertion_len = edit.new_text.as_ref().map_or(0, |t| t.len());
+        let mut delta = 0;
+
+        for mut self_range in self.ranges.iter().cloned() {
+            self_range.start += delta;
+            self_range.end += delta;
+
+            while let Some(other_range) = other_ranges.peek() {
+                let mut other_range = (*other_range).clone();
+                other_range.start += delta;
+                other_range.end += delta;
+
+                if other_range.start <= self_range.end {
+                    other_ranges.next().unwrap();
+                    delta += insertion_len;
+
+                    if other_range.end < self_range.start {
+                        new_ranges.push(other_range.start..other_range.end + insertion_len);
+                        self_range.start += insertion_len;
+                        self_range.end += insertion_len;
+                    } else {
+                        self_range.start = cmp::min(self_range.start, other_range.start);
+                        self_range.end = cmp::max(self_range.end, other_range.end) + insertion_len;
+                    }
+                } else {
+                    break;
+                }
+            }
+
+            new_ranges.push(self_range);
+        }
+
+        for other_range in other_ranges {
+            new_ranges.push(other_range.start + delta..other_range.end + delta + insertion_len);
+            delta += insertion_len;
+        }
+
+        self.ranges = new_ranges;
+    }
+}
+
 #[derive(Clone)]
 pub struct History {
     // TODO: Turn this into a String or Rope, maybe.
@@ -164,7 +212,7 @@ impl History {
             undo_stack: Vec::new(),
             redo_stack: Vec::new(),
             transaction_depth: 0,
-            group_interval: UNDO_GROUP_INTERVAL,
+            group_interval: Duration::from_millis(300),
         }
     }
 
@@ -182,9 +230,11 @@ impl History {
         self.transaction_depth += 1;
         if self.transaction_depth == 1 {
             self.undo_stack.push(Transaction {
-                start,
+                start: start.clone(),
+                end: start,
                 buffer_was_dirty,
                 edits: Vec::new(),
+                ranges: Vec::new(),
                 selections_before: selections,
                 selections_after: None,
                 first_edit_at: now,
@@ -215,12 +265,10 @@ impl History {
         let mut transactions = self.undo_stack.iter_mut();
 
         if let Some(mut transaction) = transactions.next_back() {
-            for prev_transaction in transactions.next_back() {
+            while let Some(prev_transaction) = transactions.next_back() {
                 if transaction.first_edit_at - prev_transaction.last_edit_at <= self.group_interval
+                    && transaction.start == prev_transaction.end
                 {
-                    prev_transaction.edits.append(&mut transaction.edits);
-                    prev_transaction.last_edit_at = transaction.last_edit_at;
-                    prev_transaction.selections_after = transaction.selections_after.take();
                     transaction = prev_transaction;
                     new_len -= 1;
                 } else {
@@ -229,12 +277,28 @@ impl History {
             }
         }
 
+        let (transactions_to_keep, transactions_to_merge) = self.undo_stack.split_at_mut(new_len);
+        if let Some(last_transaction) = transactions_to_keep.last_mut() {
+            for transaction in &*transactions_to_merge {
+                for edit_id in &transaction.edits {
+                    last_transaction.push_edit(&self.ops[edit_id]);
+                }
+            }
+
+            if let Some(transaction) = transactions_to_merge.last_mut() {
+                last_transaction.last_edit_at = transaction.last_edit_at;
+                last_transaction.selections_after = transaction.selections_after.take();
+                last_transaction.end = transaction.end.clone();
+            }
+        }
+
         self.undo_stack.truncate(new_len);
     }
 
     fn push_undo(&mut self, edit_id: time::Local) {
         assert_ne!(self.transaction_depth, 0);
-        self.undo_stack.last_mut().unwrap().edits.push(edit_id);
+        let last_transaction = self.undo_stack.last_mut().unwrap();
+        last_transaction.push_edit(&self.ops[&edit_id]);
     }
 
     fn pop_undo(&mut self) -> Option<&Transaction> {
@@ -259,11 +323,13 @@ impl History {
 }
 
 #[derive(Clone, Default, Debug)]
-struct UndoMap(HashMap<time::Local, Vec<UndoOperation>>);
+struct UndoMap(HashMap<time::Local, Vec<(time::Local, u32)>>);
 
 impl UndoMap {
-    fn insert(&mut self, undo: UndoOperation) {
-        self.0.entry(undo.edit_id).or_default().push(undo);
+    fn insert(&mut self, undo: &UndoOperation) {
+        for (edit_id, count) in &undo.counts {
+            self.0.entry(*edit_id).or_default().push((undo.id, *count));
+        }
     }
 
     fn is_undone(&self, edit_id: time::Local) -> bool {
@@ -276,8 +342,8 @@ impl UndoMap {
             .get(&edit_id)
             .unwrap_or(&Vec::new())
             .iter()
-            .filter(|undo| version.observed(undo.id))
-            .map(|undo| undo.count)
+            .filter(|(undo_id, _)| version.observed(*undo_id))
+            .map(|(_, undo_count)| *undo_count)
             .max()
             .unwrap_or(0);
         undo_count % 2 == 1
@@ -288,7 +354,7 @@ impl UndoMap {
             .get(&edit_id)
             .unwrap_or(&Vec::new())
             .iter()
-            .map(|undo| undo.count)
+            .map(|(_, undo_count)| *undo_count)
             .max()
             .unwrap_or(0)
     }
@@ -401,11 +467,12 @@ pub struct EditOperation {
     new_text: Option<String>,
 }
 
-#[derive(Copy, Clone, Debug, Eq, PartialEq)]
+#[derive(Clone, Debug, Eq, PartialEq)]
 pub struct UndoOperation {
     id: time::Local,
-    edit_id: time::Local,
-    count: u32,
+    counts: HashMap<time::Local, u32>,
+    ranges: Vec<Range<usize>>,
+    version: time::Global,
 }
 
 impl Buffer {
@@ -1102,7 +1169,7 @@ impl Buffer {
                 lamport_timestamp,
             } => {
                 if !self.version.observed(undo.id) {
-                    self.apply_undo(undo)?;
+                    self.apply_undo(&undo)?;
                     self.version.observe(undo.id);
                     self.lamport_clock.observe(lamport_timestamp);
                 }
@@ -1274,12 +1341,9 @@ impl Buffer {
         let old_version = self.version.clone();
 
         let mut ops = Vec::new();
-        if let Some(transaction) = self.history.pop_undo() {
+        if let Some(transaction) = self.history.pop_undo().cloned() {
             let selections = transaction.selections_before.clone();
-            for edit_id in transaction.edits.clone() {
-                ops.push(self.undo_or_redo(edit_id).unwrap());
-            }
-
+            ops.push(self.undo_or_redo(transaction).unwrap());
             if let Some((set_id, selections)) = selections {
                 let _ = self.update_selection_set(set_id, selections, cx.as_deref_mut());
             }
@@ -1301,12 +1365,9 @@ impl Buffer {
         let old_version = self.version.clone();
 
         let mut ops = Vec::new();
-        if let Some(transaction) = self.history.pop_redo() {
+        if let Some(transaction) = self.history.pop_redo().cloned() {
             let selections = transaction.selections_after.clone();
-            for edit_id in transaction.edits.clone() {
-                ops.push(self.undo_or_redo(edit_id).unwrap());
-            }
-
+            ops.push(self.undo_or_redo(transaction).unwrap());
             if let Some((set_id, selections)) = selections {
                 let _ = self.update_selection_set(set_id, selections, cx.as_deref_mut());
             }
@@ -1323,13 +1384,19 @@ impl Buffer {
         ops
     }
 
-    fn undo_or_redo(&mut self, edit_id: time::Local) -> Result<Operation> {
+    fn undo_or_redo(&mut self, transaction: Transaction) -> Result<Operation> {
+        let mut counts = HashMap::default();
+        for edit_id in transaction.edits {
+            counts.insert(edit_id, self.undo_map.undo_count(edit_id) + 1);
+        }
+
         let undo = UndoOperation {
             id: self.local_clock.tick(),
-            edit_id,
-            count: self.undo_map.undo_count(edit_id) + 1,
+            counts,
+            ranges: transaction.ranges,
+            version: transaction.start.clone(),
         };
-        self.apply_undo(undo)?;
+        self.apply_undo(&undo)?;
         self.version.observe(undo.id);
 
         Ok(Operation::Undo {
@@ -1338,27 +1405,31 @@ impl Buffer {
         })
     }
 
-    fn apply_undo(&mut self, undo: UndoOperation) -> Result<()> {
+    fn apply_undo(&mut self, undo: &UndoOperation) -> Result<()> {
         self.undo_map.insert(undo);
-        let edit = &self.history.ops[&undo.edit_id];
-        let version = Some(edit.version.clone());
 
-        let mut old_fragments = self.fragments.cursor::<VersionedOffset, VersionedOffset>();
-        old_fragments.seek(&VersionedOffset::Offset(0), Bias::Left, &version);
+        let mut cx = undo.version.clone();
+        for edit_id in undo.counts.keys().copied() {
+            cx.observe(edit_id);
+        }
+        let cx = Some(cx);
 
-        let mut new_fragments = SumTree::new();
+        let mut old_fragments = self.fragments.cursor::<VersionedOffset, VersionedOffset>();
+        let mut new_fragments = old_fragments.slice(
+            &VersionedOffset::Offset(undo.ranges[0].start),
+            Bias::Right,
+            &cx,
+        );
         let mut new_ropes =
             RopeBuilder::new(self.visible_text.cursor(0), self.deleted_text.cursor(0));
+        new_ropes.push_tree(new_fragments.summary().text);
 
-        for range in &edit.ranges {
-            let mut end_offset = old_fragments.end(&version).offset();
+        for range in &undo.ranges {
+            let mut end_offset = old_fragments.end(&cx).offset();
 
             if end_offset < range.start {
-                let preceding_fragments = old_fragments.slice(
-                    &VersionedOffset::Offset(range.start),
-                    Bias::Left,
-                    &version,
-                );
+                let preceding_fragments =
+                    old_fragments.slice(&VersionedOffset::Offset(range.start), Bias::Right, &cx);
                 new_ropes.push_tree(preceding_fragments.summary().text);
                 new_fragments.push_tree(preceding_fragments, &None);
             }
@@ -1367,8 +1438,9 @@ impl Buffer {
                 if let Some(fragment) = old_fragments.item() {
                     let mut fragment = fragment.clone();
                     let fragment_was_visible = fragment.visible;
-                    if fragment.was_visible(&edit.version, &self.undo_map)
-                        || fragment.timestamp.local() == edit.timestamp.local()
+
+                    if fragment.was_visible(&undo.version, &self.undo_map)
+                        || undo.counts.contains_key(&fragment.timestamp.local())
                     {
                         fragment.visible = fragment.is_visible(&self.undo_map);
                         fragment.max_undos.observe(undo.id);
@@ -1376,15 +1448,24 @@ impl Buffer {
                     new_ropes.push_fragment(&fragment, fragment_was_visible);
                     new_fragments.push(fragment, &None);
 
-                    old_fragments.next(&version);
-                    end_offset = old_fragments.end(&version).offset();
+                    old_fragments.next(&cx);
+                    if end_offset == old_fragments.end(&cx).offset() {
+                        let unseen_fragments = old_fragments.slice(
+                            &VersionedOffset::Offset(end_offset),
+                            Bias::Right,
+                            &cx,
+                        );
+                        new_ropes.push_tree(unseen_fragments.summary().text);
+                        new_fragments.push_tree(unseen_fragments, &None);
+                    }
+                    end_offset = old_fragments.end(&cx).offset();
                 } else {
                     break;
                 }
             }
         }
 
-        let suffix = old_fragments.suffix(&version);
+        let suffix = old_fragments.suffix(&cx);
         new_ropes.push_tree(suffix.summary().text);
         new_fragments.push_tree(suffix, &None);
 
@@ -1417,7 +1498,7 @@ impl Buffer {
         } else {
             match op {
                 Operation::Edit(edit) => self.version >= edit.version,
-                Operation::Undo { undo, .. } => self.version.observed(undo.edit_id),
+                Operation::Undo { undo, .. } => self.version >= undo.version,
                 Operation::UpdateSelections { selections, .. } => {
                     if let Some(selections) = selections {
                         selections.iter().all(|selection| {
@@ -2321,6 +2402,7 @@ mod tests {
                 .collect::<String>();
             cx.add_model(|cx| {
                 let mut buffer = Buffer::new(0, reference_string.as_str(), cx);
+                buffer.history.group_interval = Duration::from_millis(rng.gen_range(0..=200));
                 let mut buffer_versions = Vec::new();
                 log::info!(
                     "buffer text {:?}, version: {:?}",
@@ -2343,6 +2425,11 @@ mod tests {
                     if rng.gen_bool(0.25) {
                         buffer.randomly_undo_redo(rng);
                         reference_string = buffer.text();
+                        log::info!(
+                            "buffer text {:?}, version: {:?}",
+                            buffer.text(),
+                            buffer.version()
+                        );
                     }
 
                     let range = buffer.random_byte_range(0, rng);
@@ -2899,31 +2986,36 @@ mod tests {
     fn test_undo_redo(cx: &mut gpui::MutableAppContext) {
         cx.add_model(|cx| {
             let mut buffer = Buffer::new(0, "1234", cx);
+            // Set group interval to zero so as to not group edits in the undo stack.
+            buffer.history.group_interval = Duration::from_secs(0);
 
-            let edit1 = buffer.edit(vec![1..1], "abx", None).unwrap();
-            let edit2 = buffer.edit(vec![3..4], "yzef", None).unwrap();
-            let edit3 = buffer.edit(vec![3..5], "cd", None).unwrap();
+            buffer.edit(vec![1..1], "abx", None).unwrap();
+            buffer.edit(vec![3..4], "yzef", None).unwrap();
+            buffer.edit(vec![3..5], "cd", None).unwrap();
             assert_eq!(buffer.text(), "1abcdef234");
 
-            buffer.undo_or_redo(edit1.edit_id().unwrap()).unwrap();
+            let transactions = buffer.history.undo_stack.clone();
+            assert_eq!(transactions.len(), 3);
+
+            buffer.undo_or_redo(transactions[0].clone()).unwrap();
             assert_eq!(buffer.text(), "1cdef234");
-            buffer.undo_or_redo(edit1.edit_id().unwrap()).unwrap();
+            buffer.undo_or_redo(transactions[0].clone()).unwrap();
             assert_eq!(buffer.text(), "1abcdef234");
 
-            buffer.undo_or_redo(edit2.edit_id().unwrap()).unwrap();
+            buffer.undo_or_redo(transactions[1].clone()).unwrap();
             assert_eq!(buffer.text(), "1abcdx234");
-            buffer.undo_or_redo(edit3.edit_id().unwrap()).unwrap();
+            buffer.undo_or_redo(transactions[2].clone()).unwrap();
             assert_eq!(buffer.text(), "1abx234");
-            buffer.undo_or_redo(edit2.edit_id().unwrap()).unwrap();
+            buffer.undo_or_redo(transactions[1].clone()).unwrap();
             assert_eq!(buffer.text(), "1abyzef234");
-            buffer.undo_or_redo(edit3.edit_id().unwrap()).unwrap();
+            buffer.undo_or_redo(transactions[2].clone()).unwrap();
             assert_eq!(buffer.text(), "1abcdef234");
 
-            buffer.undo_or_redo(edit3.edit_id().unwrap()).unwrap();
+            buffer.undo_or_redo(transactions[2].clone()).unwrap();
             assert_eq!(buffer.text(), "1abyzef234");
-            buffer.undo_or_redo(edit1.edit_id().unwrap()).unwrap();
+            buffer.undo_or_redo(transactions[0].clone()).unwrap();
             assert_eq!(buffer.text(), "1yzef234");
-            buffer.undo_or_redo(edit2.edit_id().unwrap()).unwrap();
+            buffer.undo_or_redo(transactions[1].clone()).unwrap();
             assert_eq!(buffer.text(), "1234");
 
             buffer
@@ -2957,7 +3049,7 @@ mod tests {
             assert_eq!(buffer.text(), "12cde6");
             assert_eq!(buffer.selection_ranges(set_id).unwrap(), vec![1..3]);
 
-            now += UNDO_GROUP_INTERVAL + Duration::from_millis(1);
+            now += buffer.history.group_interval + Duration::from_millis(1);
             buffer.start_transaction_at(Some(set_id), now).unwrap();
             buffer
                 .update_selection_set(
@@ -3068,7 +3160,11 @@ mod tests {
             let mut buffers = Vec::new();
             let mut network = Network::new();
             for i in 0..peers {
-                let buffer = cx.add_model(|cx| Buffer::new(i as ReplicaId, base_text.as_str(), cx));
+                let buffer = cx.add_model(|cx| {
+                    let mut buf = Buffer::new(i as ReplicaId, base_text.as_str(), cx);
+                    buf.history.group_interval = Duration::from_millis(rng.gen_range(0..=200));
+                    buf
+                });
                 buffers.push(buffer);
                 replica_ids.push(i as u16);
                 network.add_peer(i as u16);
@@ -3365,7 +3461,7 @@ mod tests {
         where
             T: Rng,
         {
-            let (old_ranges, new_text, operation) = self.randomly_edit(rng, 5, cx.as_deref_mut());
+            let (old_ranges, new_text, operation) = self.randomly_edit(rng, 2, cx.as_deref_mut());
             let mut operations = Vec::from_iter(operation);
 
             // Randomly add, remove or mutate selection sets.
@@ -3400,9 +3496,13 @@ mod tests {
         pub fn randomly_undo_redo(&mut self, rng: &mut impl Rng) -> Vec<Operation> {
             let mut ops = Vec::new();
             for _ in 0..rng.gen_range(1..=5) {
-                if let Some(edit_id) = self.history.ops.keys().choose(rng).copied() {
-                    log::info!("undoing buffer {} operation {:?}", self.replica_id, edit_id);
-                    ops.push(self.undo_or_redo(edit_id).unwrap());
+                if let Some(transaction) = self.history.undo_stack.choose(rng).cloned() {
+                    log::info!(
+                        "undoing buffer {} transaction {:?}",
+                        self.replica_id,
+                        transaction
+                    );
+                    ops.push(self.undo_or_redo(transaction).unwrap());
                 }
             }
             ops
@@ -3481,14 +3581,4 @@ mod tests {
             })
         }
     }
-
-    impl Operation {
-        fn edit_id(&self) -> Option<time::Local> {
-            match self {
-                Operation::Edit(edit) => Some(edit.timestamp.local()),
-                Operation::Undo { undo, .. } => Some(undo.edit_id),
-                Operation::UpdateSelections { .. } => None,
-            }
-        }
-    }
 }