fix: add importGreaterUpdates variants to diffmode

This commit is contained in:
Zixuan Chen 2024-08-08 14:57:11 +08:00
parent de78cf9636
commit 62d4c9ca5d
No known key found for this signature in database
6 changed files with 102 additions and 24 deletions

View file

@ -8102,7 +8102,7 @@ fn unknown_fuzz_err_1() {
}
#[test]
fn text_fuzz_err_1() {
fn diff_calc_fuzz_err_1() {
test_multi_sites(
5,
vec![FuzzTarget::All],
@ -8190,6 +8190,56 @@ fn text_fuzz_err_1() {
)
}
#[test]
fn diff_calc_fuzz_err_2() {
test_multi_sites(
5,
vec![FuzzTarget::All],
&mut [
Handle {
site: 4,
target: 0,
container: 0,
action: Generic(GenericAction {
value: I32(-2105409536),
bool: false,
key: 4294967295,
pos: 137975431167,
length: 458752,
prop: 360287970189639680,
}),
},
Handle {
site: 255,
target: 255,
container: 255,
action: Generic(GenericAction {
value: Container(Text),
bool: false,
key: 0,
pos: 12948890936913428480,
length: 12948890938015724467,
prop: 12948890938015724467,
}),
},
Sync { from: 179, to: 179 },
Handle {
site: 0,
target: 0,
container: 0,
action: Generic(GenericAction {
value: I32(825307392),
bool: true,
key: 825307441,
pos: 17361641481138352433,
length: 18302628885800892209,
prop: 65534,
}),
},
],
)
}
#[test]
fn minify() {
minify_error(

View file

@ -313,10 +313,6 @@ where
D: DagNode + 'a,
F: Fn(ID) -> Option<&'a D>,
{
if a_id.is_empty() {
return (Default::default(), DiffMode::Import);
}
if b_id.is_empty() {
return (Default::default(), DiffMode::Checkout);
}
@ -486,6 +482,27 @@ where
D: DagNode + 'a,
F: Fn(ID) -> Option<&'a D>,
{
if right.is_empty() {
return (Default::default(), DiffMode::Checkout);
}
if left.is_empty() {
if right.len() == 1 {
let mut node_id = right[0];
let mut node = get(node_id).unwrap();
while node.deps().len() == 1 {
node_id = node.deps()[0];
node = get(node_id).unwrap();
}
if node.deps().is_empty() {
return (Default::default(), DiffMode::Linear);
}
}
return (Default::default(), DiffMode::ImportGreaterUpdates);
}
if left.len() == 1 && right.len() == 1 {
let left = left[0];
let right = right[0];
@ -510,7 +527,7 @@ where
}
}
let mut is_linear = left.len() == 1 && right.len() == 1;
let mut is_linear = left.len() <= 1 && right.len() == 1;
let mut is_right_greater = true;
let mut ans: Frontiers = Default::default();
let mut queue: BinaryHeap<(SmallVec<[OrdIdSpan; 1]>, NodeType)> = BinaryHeap::new();
@ -609,7 +626,7 @@ where
debug_assert!(ans.len() <= 1);
DiffMode::Linear
} else {
DiffMode::Import
DiffMode::ImportGreaterUpdates
}
} else {
DiffMode::Checkout

View file

@ -12,8 +12,7 @@ use itertools::Itertools;
use enum_dispatch::enum_dispatch;
use fxhash::{FxHashMap, FxHashSet};
use loro_common::{
CompactIdLp, ContainerID, Counter, HasCounterSpan, HasIdSpan, IdFull, IdLp, IdSpan, LoroValue,
PeerID, ID,
CompactIdLp, ContainerID, Counter, HasCounterSpan, IdFull, IdLp, IdSpan, LoroValue, PeerID, ID,
};
use loro_delta::DeltaRope;
use smallvec::SmallVec;
@ -84,14 +83,21 @@ pub(crate) enum DiffMode {
/// For example, we may need to compare the current register's lamport with the update's lamport to decide
/// what's the new value.
///
/// It is faster than the checkout mode, but it is still slower than the linear mode.
/// It has stricter requirements than `Checkout`:
///
/// - The difference between the import mode and the checkout mode: in import mode, target version > current version.
/// So when calculating the `DiffCalculator` doesn't need to rely on `ContainerHistoryCache`, except for the Tree container.
/// - The difference between the import mode and the linear mode: in linear mode, all the imported updates are ordered, no concurrent update exists.
/// So there is no need to build CRDTs for the calculation
/// - The target version vector must be greater than the current version vector.
Import,
/// This mode is used when the user imports new updates and all the updates are guaranteed to greater than the current version.
///
/// It has stricter requirements than `Import`.
/// - All the updates are greater than the current version. No update is concurrent to the current version.
/// - So LCA is always the `from` version
ImportGreaterUpdates,
/// This mode is used when we don't need to build CRDTs to calculate the difference. It is the fastest mode.
///
/// It has stricter requirements than `ImportGreaterUpdates`.
/// - In `ImportGreaterUpdates`, all the updates are guaranteed to be greater than the current version.
/// - In `Linear`, all the updates are ordered, no concurrent update exists.
Linear,
}
@ -322,7 +328,6 @@ impl DiffCalculator {
if d != *depth {
*depth = d;
all.push((*depth, container_idx));
calc.finish_this_round();
continue;
}
}
@ -530,7 +535,7 @@ impl DiffCalculatorTrait for MapDiffCalculator {
mut on_new_container: impl FnMut(&ContainerID),
) -> (InternalDiff, DiffMode) {
match self.current_mode {
DiffMode::Checkout => {
DiffMode::Checkout | DiffMode::Import => {
let mut changed = Vec::new();
let group = oplog
.history_cache
@ -580,7 +585,7 @@ impl DiffCalculatorTrait for MapDiffCalculator {
(InternalDiff::Map(MapDelta { updated }), DiffMode::Checkout)
}
DiffMode::Import | DiffMode::Linear => {
DiffMode::ImportGreaterUpdates | DiffMode::Linear => {
let changed = std::mem::take(&mut self.changed);
let mode = self.current_mode;
// Reset this field to avoid we use `has_all` to cache the diff calc and use it next round
@ -1331,7 +1336,7 @@ impl DiffCalculatorTrait for MovableListDiffCalculator {
};
assert_eq!(diff_mode, DiffMode::Checkout);
let is_checkout = matches!(self.current_mode, DiffMode::Checkout);
let is_checkout = matches!(self.current_mode, DiffMode::Checkout | DiffMode::Import);
let mut element_changes: FxHashMap<CompactIdLp, ElementDelta> = if is_checkout {
FxHashMap::default()
} else {

View file

@ -32,7 +32,7 @@ use std::{
sync::{Arc, Mutex, Weak},
};
use tracing::{debug, error, info, instrument};
use tracing::{debug, error, info, instrument, trace};
mod tree;
pub use tree::TreeHandler;
@ -1066,6 +1066,7 @@ impl Handler {
diff: Diff,
container_remap: &mut FxHashMap<ContainerID, ContainerID>,
) -> LoroResult<()> {
trace!("apply_diff: {:?}", &diff);
let on_container_remap = &mut |old_id, new_id| {
container_remap.insert(old_id, new_id);
};
@ -1689,10 +1690,10 @@ impl TextHandler {
}
PosType::Bytes => {
if pos + len > self.len_utf8() {
error!("pos={} len={} len_event={}", pos, len, self.len_event());
error!("pos={} len={} len_bytes={}", pos, len, self.len_utf8());
return Err(LoroError::OutOfBound {
pos: pos + len,
len: self.len_event(),
len: self.len_utf8(),
info: format!("Position: {}:{}", file!(), line!()).into_boxed_str(),
});
}
@ -3793,7 +3794,7 @@ pub mod counter {
mod test {
use super::{HandlerTrait, TextDelta};
use crate::loro::LoroDoc;
use crate::version::Frontiers;
use crate::{fx_map, ToJson};

View file

@ -554,8 +554,12 @@ impl OpLog {
}
};
let (common_ancestors, diff_mode) =
let (common_ancestors, mut diff_mode) =
self.dag.find_common_ancestor(from_frontiers, to_frontiers);
if diff_mode == DiffMode::Checkout && to > from {
diff_mode = DiffMode::Import;
}
let common_ancestors_vv = self.dag.frontiers_to_vv(&common_ancestors).unwrap();
// go from lca to merged_vv
let diff = common_ancestors_vv.diff(&merged_vv).right;

View file

@ -1097,7 +1097,8 @@ impl ContainerState for MovableListState {
match self.inner.elements().get(&elem_id).cloned() {
Some(elem) => {
// Update value if needed
if elem.value != value
if value_id.is_some()
&& elem.value != value
&& (!need_compare || elem.value_id < value_id.unwrap())
{
maybe_moved.remove(&elem_id);