From 3400a4e4b7e5190bea721eac18d8311985f668fb Mon Sep 17 00:00:00 2001 From: Zixuan Chen Date: Mon, 17 Oct 2022 16:46:18 +0800 Subject: [PATCH] test: refine test, dag node may be merged --- crates/loro-core/src/dag.rs | 34 +++------ crates/loro-core/src/dag/iter.rs | 4 +- crates/loro-core/src/dag/test.rs | 119 ++++++++++++++++++++++--------- crates/loro-core/src/span.rs | 9 +++ crates/loro-core/src/version.rs | 2 +- 5 files changed, 105 insertions(+), 63 deletions(-) diff --git a/crates/loro-core/src/dag.rs b/crates/loro-core/src/dag.rs index 510f9cd6..97ead18e 100644 --- a/crates/loro-core/src/dag.rs +++ b/crates/loro-core/src/dag.rs @@ -6,10 +6,13 @@ //! - Each node has its ID (client_id, counter). //! - We use ID to refer to node rather than content addressing (hash) //! -use std::collections::{BinaryHeap, HashMap}; +use std::{ + collections::{BinaryHeap, HashMap}, + fmt::Debug, +}; use fxhash::{FxHashMap, FxHashSet}; -use rle::rle_tree::node::Node; +use rle::{rle_tree::node::Node, HasLength}; use smallvec::SmallVec; mod iter; mod mermaid; @@ -19,7 +22,7 @@ mod test; use crate::{ change::Lamport, id::{ClientID, Counter, ID}, - span::{CounterSpan, IdSpan}, + span::{CounterSpan, HasId, IdSpan}, version::VersionVector, }; @@ -29,10 +32,8 @@ use self::{ }; // TODO: use HasId, HasLength -pub(crate) trait DagNode { - fn id_start(&self) -> ID; +pub(crate) trait DagNode: HasId + HasLength + Debug { fn lamport_start(&self) -> Lamport; - fn len(&self) -> usize; fn deps(&self) -> &[ID]; #[inline] @@ -40,25 +41,6 @@ pub(crate) trait DagNode { self.len() == 0 } - #[inline] - fn id_span(&self) -> IdSpan { - let id = self.id_start(); - IdSpan { - client_id: id.client_id, - counter: CounterSpan::new(id.counter, id.counter + self.len() as Counter), - } - } - - /// inclusive end - #[inline] - fn id_end(&self) -> ID { - let id = self.id_start(); - ID { - client_id: id.client_id, - counter: id.counter + self.len() as Counter - 1, - } - } - #[inline] fn get_offset(&self, c: Counter) -> Counter { c - self.id_start().counter @@ -250,7 +232,7 @@ where let node = get(node_id).unwrap(); let node_id_start = node.id_start(); if !visited.contains(&node_id_start) { - vv.try_update_end(node_id); + vv.try_update_last(node_id); for dep in node.deps() { if !visited.contains(dep) { stack.push(dep); diff --git a/crates/loro-core/src/dag/iter.rs b/crates/loro-core/src/dag/iter.rs index 91b1113c..1f5de35e 100644 --- a/crates/loro-core/src/dag/iter.rs +++ b/crates/loro-core/src/dag/iter.rs @@ -71,8 +71,6 @@ impl<'a, T: DagNode> Iterator for DagIterator<'a, T> { let item = self.heap.pop().unwrap(); let id = item.id; let node = self.dag.get(id).unwrap(); - debug_assert_eq!(id, node.id_start()); - // push next node from the same client to the heap let next_id = id.inc(node.len() as i32); if self.dag.contains(next_id) { @@ -154,7 +152,7 @@ impl<'a, T: DagNode> Iterator for DagIteratorVV<'a, T> { vv.unwrap_or_else(VersionVector::new) }; - vv.try_update_end(id); + vv.try_update_last(id); self.vv_map.insert(node.id_start(), vv.clone()); // push next node from the same client to the heap diff --git a/crates/loro-core/src/dag/test.rs b/crates/loro-core/src/dag/test.rs index 19377021..13e88337 100644 --- a/crates/loro-core/src/dag/test.rs +++ b/crates/loro-core/src/dag/test.rs @@ -1,11 +1,13 @@ #![cfg(test)] +use std::cmp::Ordering; + use super::*; use crate::{ array_mut_ref, change::Lamport, id::{ClientID, Counter, ID}, - span::IdSpan, + span::{HasIdSpan, IdSpan}, }; #[derive(Debug, PartialEq, Eq, Clone)] @@ -28,20 +30,26 @@ impl TestNode { } impl DagNode for TestNode { - fn id_start(&self) -> ID { - self.id - } fn lamport_start(&self) -> Lamport { self.lamport } - fn len(&self) -> usize { - self.len - } fn deps(&self) -> &[ID] { &self.deps } } +impl HasId for TestNode { + fn id_start(&self) -> ID { + self.id + } +} + +impl HasLength for TestNode { + fn len(&self) -> usize { + self.len + } +} + #[derive(Debug, PartialEq, Eq)] struct TestDag { nodes: FxHashMap>, @@ -61,9 +69,17 @@ impl Dag for TestDag { type Node = TestNode; fn get(&self, id: ID) -> Option<&Self::Node> { - self.nodes.get(&id.client_id)?.iter().find(|node| { - id.counter >= node.id.counter && id.counter < node.id.counter + node.len as Counter + let arr = self.nodes.get(&id.client_id)?; + arr.binary_search_by(|node| { + if node.id.counter > id.counter { + Ordering::Greater + } else if node.id.counter + node.len as i32 <= id.counter { + Ordering::Less + } else { + Ordering::Equal + } }) + .map_or(None, |x| Some(&arr[x])) } fn frontier(&self) -> &[ID] { @@ -75,10 +91,7 @@ impl Dag for TestDag { } fn contains(&self, id: ID) -> bool { - self.version_vec - .get(&id.client_id) - .and_then(|x| if *x > id.counter { Some(()) } else { None }) - .is_some() + self.version_vec.includes_id(id) } fn vv(&self) -> VersionVector { @@ -111,12 +124,19 @@ impl TestDag { let id = ID::new(client_id, *counter); *counter += len as Counter; let deps = std::mem::replace(&mut self.frontier, vec![id]); - self.nodes.entry(client_id).or_default().push(TestNode::new( - id, - self.next_lamport, - deps, - len, - )); + if deps.len() == 1 && deps[0].client_id == client_id { + // can merge two op + let arr = self.nodes.get_mut(&client_id).unwrap(); + let mut last = arr.last_mut().unwrap(); + last.len += len; + } else { + self.nodes.entry(client_id).or_default().push(TestNode::new( + id, + self.next_lamport, + deps, + len, + )); + } self.next_lamport += len as u32; } @@ -156,21 +176,23 @@ impl TestDag { i: usize, ) -> bool { let client_id = node.id.client_id; - if self.contains(node.id) { + if self.contains(node.id_last()) { return false; } if node.deps.iter().any(|dep| !self.contains(*dep)) { pending.push((client_id, i)); return true; } - update_frontier( - &mut self.frontier, - node.id.inc((node.len() - 1) as Counter), - &node.deps, - ); - self.nodes.entry(client_id).or_default().push(node.clone()); - self.version_vec - .insert(client_id, node.id.counter + node.len as Counter); + update_frontier(&mut self.frontier, node.id_last(), &node.deps); + let contains_start = self.contains(node.id_start()); + let mut arr = self.nodes.entry(client_id).or_default(); + if contains_start { + arr.pop(); + arr.push(node.clone()); + } else { + arr.push(node.clone()); + } + self.version_vec.set_end(node.id_end()); self.next_lamport = self.next_lamport.max(node.lamport + node.len as u32); false } @@ -292,6 +314,8 @@ mod iter { mod mermaid { + use rand::{rngs::StdRng, SeedableRng}; + use super::*; #[test] @@ -334,7 +358,7 @@ mod mermaid { #[test] fn gen() { let num = 5; - let mut rng = rand::thread_rng(); + let mut rng = StdRng::seed_from_u64(100); let mut dags = (0..num).map(TestDag::new).collect::>(); for _ in 0..100 { Interaction::gen(&mut rng, num as usize).apply(&mut dags); @@ -436,6 +460,17 @@ mod find_path { mod find_common_ancestors { use super::*; + #[test] + fn siblings() { + let mut a = TestDag::new(0); + a.push(5); + let actual = a + .find_common_ancestor(ID::new(0, 2), ID::new(0, 4)) + .first() + .copied(); + assert_eq!(actual, Some(ID::new(0, 2))); + } + #[test] fn no_common_ancestors() { let mut a = TestDag::new(0); @@ -474,7 +509,7 @@ mod find_common_ancestors { b.merge(&a); b.frontier.retain(|x| x.client_id == 1); let k = b.nodes.get_mut(&1).unwrap(); - k[1].deps.push(ID::new(0, 2)); + k[0].deps.push(ID::new(0, 2)); assert_eq!( b.find_common_ancestor(ID::new(0, 3), ID::new(1, 8)) .first() @@ -520,9 +555,11 @@ mod find_common_ancestors { } mod find_common_ancestors_proptest { + use std::thread::panicking; + use proptest::prelude::*; - use crate::{array_mut_ref, tests::PROPTEST_FACTOR_10, unsafe_array_mut_ref}; + use crate::{array_mut_ref, span::HasIdSpan, tests::PROPTEST_FACTOR_10, unsafe_array_mut_ref}; use super::*; @@ -603,6 +640,22 @@ mod find_common_ancestors_proptest { } } + #[test] + fn issue() { + if let Err(err) = test_single_common_ancestor( + 4, + vec![Interaction { + dag_idx: 0, + merge_with: None, + len: 1, + }], + vec![], + ) { + println!("{}", err); + panic!(); + } + } + fn test_single_common_ancestor( dag_num: i32, mut before_merge_insertion: Vec, @@ -644,8 +697,8 @@ mod find_common_ancestors_proptest { let (dag0, dag1) = array_mut_ref!(&mut dags, [0, 1]); dag1.push(1); dag0.merge(dag1); - let a = dags[0].nodes.get(&0).unwrap().last().unwrap().id; - let b = dags[1].nodes.get(&1).unwrap().last().unwrap().id; + let a = dags[0].nodes.get(&0).unwrap().last().unwrap().id_last(); + let b = dags[1].nodes.get(&1).unwrap().last().unwrap().id_last(); let actual = dags[0].find_common_ancestor(a, b); prop_assert_eq!(actual.first().copied().unwrap(), expected); Ok(()) diff --git a/crates/loro-core/src/span.rs b/crates/loro-core/src/span.rs index 54493490..6b6eeeb1 100644 --- a/crates/loro-core/src/span.rs +++ b/crates/loro-core/src/span.rs @@ -175,6 +175,15 @@ pub trait HasIdSpan: HasId + HasLength { fn id_last(&self) -> ID { self.id_start().inc(self.len() as i32 - 1) } + + fn contains_id(&self, id: ID) -> bool { + let id_start = self.id_start(); + if id.client_id != id_start.client_id { + return false; + } + + id_start.counter <= id.counter && id.counter < id_start.counter + self.len() as Counter + } } impl HasIdSpan for T {} diff --git a/crates/loro-core/src/version.rs b/crates/loro-core/src/version.rs index 716771f4..2a80dfbf 100644 --- a/crates/loro-core/src/version.rs +++ b/crates/loro-core/src/version.rs @@ -125,7 +125,7 @@ impl VersionVector { /// update the end counter of the given client, if the end is greater /// return whether updated #[inline] - pub fn try_update_end(&mut self, id: ID) -> bool { + pub fn try_update_last(&mut self, id: ID) -> bool { if let Some(end) = self.0.get_mut(&id.client_id) { if *end < id.counter + 1 { *end = id.counter + 1;