From d34abeef929c232b6fce858ee6a4314116ccf202 Mon Sep 17 00:00:00 2001 From: Zixuan Chen Date: Wed, 12 Oct 2022 14:55:59 +0800 Subject: [PATCH] fix: insert map logic bug --- crates/loro-core/fuzz/fuzz_targets/yata.rs | 2 +- .../loro-core/src/container/text/tracker.rs | 32 ++++- .../src/container/text/tracker/yata.rs | 121 ++++++++++++++++-- crates/rle/src/range_map.rs | 5 + crates/rle/src/rle_tree.rs | 6 +- crates/rle/src/rle_tree/cursor.rs | 4 + crates/rle/src/rle_tree/node/internal_impl.rs | 3 - 7 files changed, 151 insertions(+), 22 deletions(-) diff --git a/crates/loro-core/fuzz/fuzz_targets/yata.rs b/crates/loro-core/fuzz/fuzz_targets/yata.rs index 3e8af6b3..5eafdc21 100644 --- a/crates/loro-core/fuzz/fuzz_targets/yata.rs +++ b/crates/loro-core/fuzz/fuzz_targets/yata.rs @@ -3,4 +3,4 @@ use crdt_list::{test, test::Action}; use libfuzzer_sys::fuzz_target; use loro_core::container::text::tracker::yata::YataImpl; -fuzz_target!(|data: Vec| { test::test_with_actions::(5, &data) }); +fuzz_target!(|data: Vec| { test::test_with_actions::(2, 5, data) }); diff --git a/crates/loro-core/src/container/text/tracker.rs b/crates/loro-core/src/container/text/tracker.rs index 31a07e1d..567b327d 100644 --- a/crates/loro-core/src/container/text/tracker.rs +++ b/crates/loro-core/src/container/text/tracker.rs @@ -75,8 +75,29 @@ impl Tracker { } /// check whether id_to_cursor correctly reflect the status of the content - fn check_consistency(&self) { - todo!() + fn check_consistency(&mut self) { + for span in self.content.iter() { + let yspan = span.as_ref(); + let id_span = IdSpan::new( + yspan.id.client_id, + yspan.id.counter, + yspan.len as Counter + yspan.id.counter, + ); + let mut len = 0; + for marker in self + .id_to_cursor + .get_range(id_span.min_id().into(), id_span.end_id().into()) + { + for span in marker.get_spans(id_span) { + len += span.len; + } + } + + assert_eq!(len, yspan.len); + } + + self.content.debug_check(); + self.id_to_cursor.debug_check(); } fn turn_on(&mut self, _id: IdSpan) {} @@ -131,6 +152,13 @@ impl Tracker { debug_assert!( cursors.iter().map(|x| x.len).sum::() == spans.iter().map(|x| x.len()).sum() ); + // println!("SPANS {:?}", spans); + // for cursor in cursors.iter() { + // // SAFETY: Test + // println!(" LEAF {:?}", unsafe { cursor.leaf.as_ref() }); + // println!(" CURSOR {:?}", cursor); + // } + // dbg!(&self); self.content.update_at_cursors( cursors, &mut |v| { diff --git a/crates/loro-core/src/container/text/tracker/yata.rs b/crates/loro-core/src/container/text/tracker/yata.rs index 505a1570..f970524e 100644 --- a/crates/loro-core/src/container/text/tracker/yata.rs +++ b/crates/loro-core/src/container/text/tracker/yata.rs @@ -93,7 +93,7 @@ impl ListCrdt for YataImpl { } fn cmp_id(op_a: &Self::OpUnit, op_b: &Self::OpUnit) -> std::cmp::Ordering { - op_a.id.cmp(&op_b.id) + op_a.id.client_id.cmp(&op_b.id.client_id) } fn contains(op: &Self::OpUnit, id: Self::OpId) -> bool { @@ -104,6 +104,7 @@ impl ListCrdt for YataImpl { container.vv.set_end(op.id.inc(op.len as i32)); // SAFETY: we know this is safe because in [YataImpl::insert_after] there is no access to shared elements unsafe { crdt_list::yata::integrate::(container, op) }; + container.check_consistency(); } fn can_integrate(container: &Self::Container, op: &Self::OpUnit) -> bool { @@ -278,6 +279,7 @@ pub mod fuzz { fn integrate_delete_op(container: &mut Self::Container, op: Self::DeleteOp) { container.update_spans(&op, StatusChange::Delete); + container.check_consistency(); } } @@ -286,7 +288,8 @@ pub mod fuzz { fn issue_0() { crdt_list::test::test_with_actions::( 5, - &[ + 5, + vec![ NewOp { client_id: 1, pos: 0, @@ -309,26 +312,116 @@ pub mod fuzz { fn issue_1() { crdt_list::test::test_with_actions::( 3, - &[ + 5, + vec![ + Delete { + client_id: 1, + pos: 3, + len: 3, + }, + Delete { + client_id: 1, + pos: 1, + len: 4, + }, + NewOp { + client_id: 0, + pos: 4, + }, + NewOp { + client_id: 0, + pos: 4, + }, + NewOp { + client_id: 0, + pos: 3, + }, + NewOp { + client_id: 0, + pos: 4, + }, + NewOp { + client_id: 0, + pos: 0, + }, + Delete { + client_id: 1, + pos: 4, + len: 4, + }, + NewOp { + client_id: 1, + pos: 0, + }, Delete { client_id: 1, pos: 0, - len: 1, + len: 2, }, Delete { client_id: 0, - pos: 1, - len: 1, - }, - NewOp { - client_id: 1, - pos: 1, - }, - NewOp { - client_id: 0, - pos: 1, + pos: 0, + len: 4, }, ], ) } + + #[test] + fn normalize() { + let mut actions = vec![ + Delete { + client_id: 18446744073709551615, + pos: 18446462602589896703, + len: 18374687467077894143, + }, + Delete { + client_id: 18374939255676862463, + pos: 64710657328087551, + len: 11429747308416114334, + }, + NewOp { + client_id: 4872331909652192926, + pos: 11429747308416114334, + }, + NewOp { + client_id: 11429747308416114334, + pos: 11429747308416114334, + }, + NewOp { + client_id: 11429738512323092126, + pos: 11429747306828660733, + }, + NewOp { + client_id: 18446744073709524638, + pos: 10876193100099747839, + }, + NewOp { + client_id: 18374687126443038358, + pos: 18446744073709551615, + }, + Delete { + client_id: 7451037802331897855, + pos: 7451037802321897319, + len: 7451037802321897319, + }, + NewOp { + client_id: 7451037802321897319, + pos: 16529055059114682215, + }, + Delete { + client_id: 648103854079, + pos: 7133702213043879935, + len: 18446744069565579237, + }, + Delete { + client_id: 16638239752757634710, + pos: 18446744073288476390, + len: 7133701809771642879, + }, + ]; + + crdt_list::test::normalize_actions(&mut actions, 2, 5); + dbg!(actions); + } } diff --git a/crates/rle/src/range_map.rs b/crates/rle/src/range_map.rs index 3ddd51ec..8d280f75 100644 --- a/crates/rle/src/range_map.rs +++ b/crates/rle/src/range_map.rs @@ -76,6 +76,11 @@ impl RangeMap ); } + #[inline] + pub fn debug_check(&mut self) { + self.tree.debug_check() + } + #[inline] pub fn delete(&mut self, start: Option, end: Option) { self.tree.delete_range(start, end); diff --git a/crates/rle/src/rle_tree.rs b/crates/rle/src/rle_tree.rs index 1931be85..8d8a02c5 100644 --- a/crates/rle/src/rle_tree.rs +++ b/crates/rle/src/rle_tree.rs @@ -287,7 +287,7 @@ impl> RleTree { .push((leaf.get_index_in_parent().unwrap(), new)); } else { // insert empty value to trigger cache update - internal_updates_map.insert(leaf.parent, Default::default()); + internal_updates_map.entry(leaf.parent).or_default(); } } @@ -308,7 +308,9 @@ impl> RleTree { .push((node.get_index_in_parent().unwrap(), new)); } else if node.parent.is_some() { // insert empty value to trigger cache update - internal_updates_map.insert(node.parent.unwrap(), Default::default()); + internal_updates_map + .entry(node.parent.unwrap()) + .or_default(); } else { A::update_cache_internal(node); } diff --git a/crates/rle/src/rle_tree/cursor.rs b/crates/rle/src/rle_tree/cursor.rs index f1b50042..819e9918 100644 --- a/crates/rle/src/rle_tree/cursor.rs +++ b/crates/rle/src/rle_tree/cursor.rs @@ -126,7 +126,11 @@ impl<'tree, T: Rle, A: RleTreeTrait> UnsafeCursor<'tree, T, A> { F: FnMut(&T, *mut LeafNode<'_, T, A>), { let leaf = self.leaf.as_mut(); + // println!("insert cursor {:?}", self); + // println!("insert value {:?}", value); + // dbg!(&leaf); let result = leaf.insert_at_pos(self.pos, self.index, self.offset, value, notify); + // dbg!(&leaf); let mut node = leaf.parent.as_mut(); if let Err(new) = result { let mut result = node.insert_at_pos(leaf.get_index_in_parent().unwrap() + 1, new); diff --git a/crates/rle/src/rle_tree/node/internal_impl.rs b/crates/rle/src/rle_tree/node/internal_impl.rs index 69ba9d41..f4df2863 100644 --- a/crates/rle/src/rle_tree/node/internal_impl.rs +++ b/crates/rle/src/rle_tree/node/internal_impl.rs @@ -513,9 +513,6 @@ impl<'a, T: Rle, A: RleTreeTrait> InternalNode<'a, T, A> { continue; } - dbg!(self); - dbg!(node.parent()); - dbg!(node); unreachable!(); }