fix: fix several iter & delete bug

most of the bugs are related to len / content_len
This commit is contained in:
Zixuan Chen 2022-10-12 00:16:38 +08:00
parent 280382c39c
commit 5104e94cd0
13 changed files with 266 additions and 111 deletions

1
.gitignore vendored
View file

@ -1 +1,2 @@
/target
*.log

View file

@ -33,7 +33,7 @@ pub mod yata;
///
#[derive(Debug)]
pub struct Tracker {
#[cfg(feature = "fuzzing")]
// #[cfg(feature = "fuzzing")]
client_id: u64,
vv: VersionVector,
content: ContentMap,
@ -49,7 +49,7 @@ impl From<ID> for u128 {
impl Tracker {
pub fn new() -> Self {
let min = ID::unknown(0);
let max = ID::unknown(Counter::MAX);
let max = ID::unknown(Counter::MAX / 2);
let len = (max.counter - min.counter) as usize;
let mut content: ContentMap = Default::default();
let mut id_to_cursor: CursorMap = Default::default();
@ -68,7 +68,6 @@ impl Tracker {
Tracker {
content,
id_to_cursor,
#[cfg(feature = "fuzzing")]
client_id: 0,
vv: Default::default(),
}
@ -111,28 +110,28 @@ impl Tracker {
}
pub fn update_spans(&mut self, spans: &RleVec<IdSpan>, change: StatusChange) {
println!("{} SPANS {:?}", self.client_id, &spans);
let mut cursors = Vec::new();
for span in spans.iter() {
let mut span_start = span.min_id();
for marker in self
.id_to_cursor
.get_range(span.min_id().into(), span.max_id().into())
{
let m = marker.as_cursor(span_start);
if m.is_none() {
println!("XXX ID {:?} {:?}", span_start, span.max_id());
let m = marker.as_cursor(span_start);
for cursor in marker.get_spans(*span) {
cursors.push(cursor);
}
let cursor = m.unwrap().unwrap();
span_start = span_start.inc(cursor.len as Counter);
cursors.push(cursor);
}
}
// if cursors.len() == 0 {
// panic!("cursors is empty");
// }
self.content.update_at_cursors(
cursors,
&mut |v| {
v.status.apply(change);
dbg!(&v);
},
&mut make_notify(&mut self.id_to_cursor),
)

View file

@ -4,7 +4,7 @@ use enum_as_inner::EnumAsInner;
use rle::{
range_map::RangeMap,
rle_tree::{node::LeafNode, Position, SafeCursor, SafeCursorMut},
rle_tree::{node::LeafNode, Position, SafeCursor, SafeCursorMut, UnsafeCursor},
HasLength, Mergable, RleVec, Sliceable,
};
@ -30,6 +30,9 @@ impl Marker {
Marker::Insert { ptr, len: _ } => {
// SAFETY: tree data is always valid
let node = unsafe { ptr.as_ref() };
if node.is_deleted() {
dbg!(&node);
}
debug_assert!(!node.is_deleted());
let position = node.children().iter().position(|x| x.contain_id(id))?;
let child = &node.children()[position];
@ -41,7 +44,7 @@ impl Marker {
*ptr,
position,
offset as usize,
Position::from_offset(offset as isize, child.len()),
Position::from_offset(offset as isize, child.content_len()),
0,
)
})
@ -50,6 +53,45 @@ impl Marker {
}
}
pub fn get_spans(&self, id_span: IdSpan) -> Vec<UnsafeCursor<'static, YSpan, YSpanTreeTrait>> {
match self {
Marker::Insert { ptr, len: _ } => {
// SAFETY: tree data is always valid
let node = unsafe { ptr.as_ref() };
debug_assert!(!node.is_deleted());
node.children()
.iter()
.enumerate()
.filter_map(|(i, child)| {
if child.overlap(id_span) {
let start_counter = child.id.counter;
let offset = std::cmp::max(id_span.counter.min() - start_counter, 0);
debug_assert!((offset as usize) < child.len);
let max_offset = std::cmp::min(
id_span.counter.max() - start_counter,
(child.len - 1) as i32,
);
let len = max_offset - offset + 1;
// SAFETY: we just checked it is valid
Some(unsafe {
std::mem::transmute(UnsafeCursor::new(
*ptr,
i,
offset as usize,
Position::from_offset(offset as isize, child.len),
len as usize,
))
})
} else {
None
}
})
.collect()
}
Marker::Delete(_) => unreachable!(),
}
}
/// # Safety
///
/// It's safe when you are sure that the returned cursor is the only reference to the target yspan tree
@ -140,7 +182,7 @@ pub(super) fn make_notify(
ptr: unsafe {
NonNull::new_unchecked(leaf as usize as *mut LeafNode<'static, _, _>)
},
len: span.len(),
len: span.content_len(),
},
)
}

View file

@ -1,4 +1,4 @@
use crate::{id::Counter, ContentType, InsertContent, ID};
use crate::{id::Counter, span::IdSpan, ContentType, InsertContent, ID};
use rle::{rle_tree::tree_trait::CumulateTreeTrait, HasLength, Mergable, Sliceable};
#[derive(Debug, Clone, PartialEq, Eq, Default)]
@ -40,13 +40,13 @@ impl Status {
}
}
#[derive(Debug, Clone)]
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct YSpan {
pub origin_left: Option<ID>,
pub origin_right: Option<ID>,
pub id: ID,
pub len: usize,
pub status: Status,
pub origin_left: Option<ID>,
pub origin_right: Option<ID>,
}
#[derive(Clone, Copy, Debug)]
@ -80,15 +80,25 @@ impl YSpan {
&& self.id.counter <= id.counter
&& self.last_id().counter >= id.counter
}
#[inline]
pub fn overlap(&self, id: IdSpan) -> bool {
if self.id.client_id != id.client_id {
return false;
}
self.id.counter < id.counter.to
&& self.id.counter + (self.len as Counter) > id.counter.min()
}
}
impl Mergable for YSpan {
fn is_mergable(&self, other: &Self, _: &()) -> bool {
other.id.client_id == self.id.client_id
&& self.id.counter + self.len() as Counter == other.id.counter
&& Some(self.id.inc(self.len as Counter - 1)) == other.origin_left
&& self.origin_right == other.origin_right
&& self.status == other.status
&& self.id.counter + self.len() as Counter == other.id.counter
&& self.origin_right == other.origin_right
&& Some(self.id.inc(self.len as Counter - 1)) == other.origin_left
}
fn merge(&mut self, other: &Self, _: &()) {
@ -146,6 +156,11 @@ impl HasLength for YSpan {
0
}
}
#[inline]
fn content_len(&self) -> usize {
self.len
}
}
#[cfg(test)]

View file

@ -81,7 +81,11 @@ impl ListCrdt for YataImpl {
fn insert_at(container: &mut Self::Container, op: Self::OpUnit, pos: usize) {
let mut notify = make_notify(&mut container.id_to_cursor);
container.content.insert_notify(pos, op, &mut notify);
if pos == 0 {
container.content.insert_at_first(op, &mut notify);
} else {
container.content.insert_notify(pos, op, &mut notify);
}
}
fn id(op: &Self::OpUnit) -> Self::OpId {
@ -178,6 +182,7 @@ pub mod fuzz {
test::{Action, TestFramework},
yata::Yata,
};
use moveit::New;
use rle::RleVec;
use crate::{
@ -191,21 +196,23 @@ pub mod fuzz {
impl TestFramework for YataImpl {
fn is_content_eq(a: &Self::Container, b: &Self::Container) -> bool {
let aa = {
let mut ans = Vec::new();
let mut ans = RleVec::new();
for iter in a.content.iter() {
ans.push((iter.id, iter.len));
ans.push(iter.as_ref().clone());
}
ans
};
let bb = {
let mut ans = Vec::new();
let mut ans = RleVec::new();
for iter in b.content.iter() {
ans.push((iter.id, iter.len));
ans.push(iter.as_ref().clone());
}
ans
};
if aa != bb {
dbg!(aa.vec());
dbg!(bb.vec());
dbg!(a);
dbg!(b);
}
@ -251,7 +258,7 @@ pub mod fuzz {
}
pos %= container.content.len();
len = std::cmp::min(len % 10 + 1, container.content.len() - pos);
len = std::cmp::min(len % 10, container.content.len() - pos);
if len == 0 {
return RleVec::new();
}
@ -262,6 +269,7 @@ pub mod fuzz {
fn integrate_delete_op(container: &mut Self::Container, op: Self::DeleteOp) {
container.update_spans(&op, StatusChange::Delete);
// dbg!(&container);
}
fn can_apply_del_op(container: &Self::Container, op: &Self::DeleteOp) -> bool {
@ -276,16 +284,13 @@ pub mod fuzz {
5,
&[
NewOp {
client_id: 16573246628723425271,
pos: 16565899579919523301,
client_id: 0,
pos: 0,
},
NewOp {
client_id: 16504256534250120677,
pos: 16565899579919523301,
},
NewOp {
client_id: 16565899579910645221,
pos: 182786533,
Delete {
client_id: 1,
pos: 0,
len: 4,
},
],
)
@ -297,48 +302,99 @@ pub mod fuzz {
5,
&[
NewOp {
client_id: 72057319153112726,
pos: 18446743116487664383,
client_id: 11719107999768421014,
pos: 11719107999768421026,
},
Delete {
client_id: 18446742978492891135,
client_id: 10851025925718409122,
pos: 531712649396118,
len: 18446504380166307839,
},
Delete {
client_id: 10880696699727118335,
pos: 18374967954648334335,
len: 18446744069414584321,
},
Delete {
client_id: 11719210655348162559,
pos: 18446641418129810082,
len: 10873349650923257855,
},
Delete {
client_id: 11719107999768421119,
pos: 11719107999768421026,
len: 11719107999768421026,
},
NewOp {
client_id: 16835197176461304482,
pos: 11719107999768421026,
},
NewOp {
client_id: 11719107999768421026,
pos: 10851025927479993010,
},
NewOp {
client_id: 9223370937374840575,
pos: 18446744073695264767,
},
Delete {
client_id: 18446743622737985535,
pos: 2194745065471,
len: 18446742974197924096,
},
Delete {
client_id: 11745387828182253567,
pos: 11719107999768421026,
len: 11719107999768421026,
},
NewOp {
client_id: 11719107999768421026,
pos: 11719107999768421026,
},
NewOp {
client_id: 11719107999768421026,
pos: 11719107999768421026,
},
NewOp {
client_id: 11719107999768421026,
pos: 11719107999768421026,
},
NewOp {
client_id: 11719107999768421026,
pos: 11719107999768421026,
},
NewOp {
client_id: 10850930719374615202,
pos: 18446628623220119190,
},
Delete {
client_id: 18446744073709551615,
pos: 18446743206884093439,
len: 15914838024376868095,
},
Delete {
client_id: 15863046628662107356,
pos: 98784247772,
len: 18446744069414584320,
},
Delete {
client_id: 18446744073709551615,
pos: 18446744073709551615,
len: 18446744073695461375,
len: 18446744073709551615,
},
Delete {
client_id: 65535,
pos: 281178623508480,
len: 18446742974197923840,
client_id: 16777471,
pos: 2954361355538333696,
len: 18446744073709551615,
},
Delete {
client_id: 13107135066100727807,
pos: 532050712311190,
len: 18446744073701163007,
client_id: 11745387828182253567,
pos: 11719107999768421026,
len: 11719107999768421026,
},
NewOp {
client_id: 35184372089087,
pos: 18446462598732840960,
},
Sync {
from: 18446744073692774400,
to: 16565899692026626047,
},
Delete {
client_id: 18446462606851290549,
pos: 18446744073709551487,
len: 9910603680803979263,
},
NewOp {
client_id: 9910603678816504201,
pos: 9910603678816504201,
},
NewOp {
client_id: 9910603678816504201,
pos: 9910603678816504201,
},
NewOp {
client_id: 9910603678816504201,
pos: 18446744073701788041,
client_id: 11719107999768421026,
pos: 11719107999768421026,
},
],
)

View file

@ -40,7 +40,7 @@ impl CounterSpan {
if self.from > self.to {
self.from
} else {
self.to
self.to - 1
}
}
}

View file

@ -41,6 +41,10 @@ pub trait HasLength {
}
fn len(&self) -> usize;
fn content_len(&self) -> usize {
self.len()
}
}
pub trait Rle<Cfg = ()>: HasLength + Sliceable + Mergable<Cfg> + Debug + Clone {}

View file

@ -36,6 +36,25 @@ impl<T: Rle + 'static, A: RleTreeTrait<T> + 'static> Default for RleTree<T, A> {
}
impl<T: Rle, A: RleTreeTrait<T>> RleTree<T, A> {
pub fn insert_at_first<F>(&mut self, value: T, notify: &mut F)
where
F: FnMut(&T, *mut LeafNode<'_, T, A>),
{
if let Some(value) = self.with_node_mut(|node| {
let leaf = node.get_first_leaf();
if let Some(leaf) = leaf {
// SAFETY: we have exclusive ref to the tree
let cursor = unsafe { SafeCursorMut::new(leaf.into(), 0, 0, Position::Start, 0) };
cursor.insert_before_notify(value, notify);
None
} else {
Some(value)
}
}) {
self.insert_notify(A::Int::from_u8(0).unwrap(), value, notify);
}
}
#[inline]
pub fn insert(&mut self, index: A::Int, value: T) {
self.with_node_mut(|node| {
@ -238,6 +257,7 @@ impl<T: Rle, A: RleTreeTrait<T>> RleTree<T, A> {
U: FnMut(&mut T),
F: FnMut(&T, *mut LeafNode<T, A>),
{
dbg!(&cursors);
let mut updates_map: HashMap<NonNull<_>, Vec<(usize, Vec<T>)>> = Default::default();
for cursor in cursors {
// SAFETY: we has the exclusive reference to the tree and the cursor is valid
@ -287,20 +307,17 @@ impl<T: Rle, A: RleTreeTrait<T>> RleTree<T, A> {
.entry(node.parent.unwrap())
.or_default()
.push((node.get_index_in_parent().unwrap(), new));
} else {
} else if node.parent.is_some() {
// insert empty value to trigger cache update
internal_updates_map.insert(node.parent.unwrap(), Default::default());
} else {
A::update_cache_internal(node);
}
}
}
#[cfg(test)]
{
self.debug_check();
}
}
pub fn iter_update<U, F>(
pub fn update_range<U, F>(
&mut self,
start: A::Int,
end: Option<A::Int>,

View file

@ -50,7 +50,7 @@ impl<'tree, T: Rle, A: RleTreeTrait<T>> Copy for SafeCursor<'tree, T, A> {}
impl<'tree, T: Rle, A: RleTreeTrait<T>> UnsafeCursor<'tree, T, A> {
#[inline]
pub(crate) fn new(
pub fn new(
leaf: NonNull<LeafNode<'tree, T, A>>,
index: usize,
offset: usize,
@ -169,7 +169,7 @@ impl<'tree, T: Rle, A: RleTreeTrait<T>> UnsafeCursor<'tree, T, A> {
let mut leaf = self.leaf.as_ref();
while shift > 0 {
let diff = leaf.children[self.index].len() - self.offset;
let diff = leaf.children[self.index].content_len() - self.offset;
#[cfg(test)]
{
leaf.check();
@ -181,7 +181,7 @@ impl<'tree, T: Rle, A: RleTreeTrait<T>> UnsafeCursor<'tree, T, A> {
return Some(self);
}
std::cmp::Ordering::Equal => {
self.offset = leaf.children[self.index].len();
self.offset = leaf.children[self.index].content_len();
self.pos = Position::End;
return Some(self);
}

View file

@ -142,7 +142,7 @@ impl<'tree, T: Rle, A: RleTreeTrait<T>> Iterator for Iter<'tree, T, A> {
cursor.offset,
Position::from_offset(
cursor.offset as isize,
node.children[cursor.index].len(),
node.children[cursor.index].content_len(),
),
end.offset - cursor.offset,
)
@ -154,7 +154,7 @@ impl<'tree, T: Rle, A: RleTreeTrait<T>> Iterator for Iter<'tree, T, A> {
}
}
let child_len = node.children[cursor.index].len();
let child_len = node.children[cursor.index].content_len();
// SAFETY: we just checked that the child exists
let ans = Some(unsafe {
SafeCursor::new(
@ -217,7 +217,7 @@ impl<'tree, T: Rle, A: RleTreeTrait<T>> Iterator for IterMut<'tree, T, A> {
cursor.offset,
Position::from_offset(
cursor.offset as isize,
node.children[cursor.index].len(),
node.children[cursor.index].content_len(),
),
end.offset - cursor.offset,
)
@ -229,7 +229,7 @@ impl<'tree, T: Rle, A: RleTreeTrait<T>> Iterator for IterMut<'tree, T, A> {
}
}
let child_len = node.children[cursor.index].len();
let child_len = node.children[cursor.index].content_len();
// SAFETY: we just checked that the child exists
let ans = Some(unsafe {
SafeCursorMut::new(

View file

@ -227,20 +227,19 @@ impl<'a, T: Rle, A: RleTreeTrait<T>> InternalNode<'a, T, A> {
let mut self_children = std::mem::replace(&mut self.children, BumpVec::new_in(self.bump));
let mut last_end = 0;
for (index, replace) in updates {
let should_pop = index - last_end < self_children.len();
for child in self_children.drain(0..index - last_end + 1) {
new_children.push(child);
}
if should_pop {
new_children.pop();
}
for element in replace {
new_children.push(element);
}
last_end = index + 1;
last_end = index;
}
for child in self_children.drain(..) {
new_children.push(child);
}
let self_ptr: NonNull<_> = self.into();
@ -253,7 +252,9 @@ impl<'a, T: Rle, A: RleTreeTrait<T>> InternalNode<'a, T, A> {
A::update_cache_internal(self);
Ok(())
} else {
for child in new_children.drain(0..A::MAX_CHILDREN_NUM) {
for child in
new_children.drain(0..(std::cmp::min(A::MAX_CHILDREN_NUM, new_children.len())))
{
child.set_parent(self_ptr);
self.children.push(child);
}
@ -265,7 +266,9 @@ impl<'a, T: Rle, A: RleTreeTrait<T>> InternalNode<'a, T, A> {
.bump
.alloc(Node::Internal(InternalNode::new(self.bump, self.parent)));
let new_internal = new_internal_node.as_internal_mut().unwrap();
for child in new_children.drain(0..A::MAX_CHILDREN_NUM) {
for child in
new_children.drain(0..(std::cmp::min(A::MAX_CHILDREN_NUM, new_children.len())))
{
child.set_parent(new_internal.into());
new_internal.children.push(child);
}
@ -368,7 +371,7 @@ impl<'a, T: Rle, A: RleTreeTrait<T>> InternalNode<'a, T, A> {
}
left_inner.parent = Some(NonNull::new(self).unwrap());
new.as_internal_mut().unwrap().parent = Some(NonNull::new(self).unwrap());
new.as_internal_mut().unwrap().parent = Some(self.into());
self.children.push(left);
self.children.push(new);
A::update_cache_internal(self);

View file

@ -267,7 +267,10 @@ impl<'bump, T: Rle, A: RleTreeTrait<T>> LeafNode<'bump, T, A> {
let right = if self.children[child_index].len() == offset + len {
None
} else {
Some(self.children[child_index].slice(offset + len, self.children[child_index].len()))
Some(
self.children[child_index]
.slice(offset + len, self.children[child_index].content_len()),
)
};
let mut target = self.children[child_index].slice(offset, offset + len);
@ -356,8 +359,8 @@ impl<'bump, T: Rle, A: RleTreeTrait<T>> LeafNode<'bump, T, A> {
}
let child = &self.children[child_index];
if offset == 0 && child.len() == len {
let mut element = child.slice(0, len);
if offset == 0 && child.content_len() == len {
let mut element = (**child).clone();
update_fn(&mut element);
ans.push(element);
return Some(ans);
@ -374,10 +377,12 @@ impl<'bump, T: Rle, A: RleTreeTrait<T>> LeafNode<'bump, T, A> {
} else {
ans.push(target);
}
} else {
ans.push(target);
}
if offset + len < child.len() {
let right = child.slice(offset + len, child.len());
if offset + len < child.content_len() {
let right = child.slice(offset + len, child.content_len());
let mut merged = false;
if let Some(last) = ans.last_mut() {
if last.is_mergable(&right, &()) {
@ -425,15 +430,11 @@ impl<'bump, T: Rle, A: RleTreeTrait<T>> LeafNode<'bump, T, A> {
let mut last_end = 0;
// append element to the new_children list
for (index, replace) in updates {
let should_pop = index - last_end < self_children.len();
for child in self_children.drain(0..index - last_end + 1) {
for child in self_children.drain(0..index + 1 - last_end) {
new_children.push(child);
}
if should_pop {
// ignore original element at index
new_children.pop();
}
new_children.pop();
for element in replace {
let mut merged = false;
@ -451,6 +452,10 @@ impl<'bump, T: Rle, A: RleTreeTrait<T>> LeafNode<'bump, T, A> {
last_end = index + 1;
}
for child in self_children.drain(..) {
new_children.push(child);
}
if new_children.len() <= A::MAX_CHILDREN_NUM {
for child in new_children {
notify(child, self);
@ -460,7 +465,9 @@ impl<'bump, T: Rle, A: RleTreeTrait<T>> LeafNode<'bump, T, A> {
A::update_cache_leaf(self);
Ok(())
} else {
for child in new_children.drain(0..A::MAX_CHILDREN_NUM) {
for child in
new_children.drain(0..std::cmp::min(A::MAX_CHILDREN_NUM, new_children.len()))
{
notify(child, self);
self.children.push(child);
}
@ -472,7 +479,9 @@ impl<'bump, T: Rle, A: RleTreeTrait<T>> LeafNode<'bump, T, A> {
.bump
.alloc(Node::Leaf(LeafNode::new(self.bump, self.parent)));
let new_leaf = new_leaf_node.as_leaf_mut().unwrap();
for child in new_children.drain(0..A::MAX_CHILDREN_NUM) {
for child in
new_children.drain(0..std::cmp::min(A::MAX_CHILDREN_NUM, new_children.len()))
{
notify(child, new_leaf);
new_leaf.children.push(child);
}
@ -540,7 +549,7 @@ impl<'bump, T: Rle, A: RleTreeTrait<T>> LeafNode<'bump, T, A> {
}
// need to split child
let a = self.children[child_index].slice(0, offset);
let b = self.children[child_index].slice(offset, self.children[child_index].len());
let b = self.children[child_index].slice(offset, self.children[child_index].content_len());
self.children[child_index] = self.bump.alloc(a);
if self.children.len() >= A::MAX_CHILDREN_NUM - 1 {
let next_node = self._split(notify);
@ -635,7 +644,7 @@ impl<'a, T: Rle, A: RleTreeTrait<T>> LeafNode<'a, T, A> {
let end = &mut self.children[del_end];
let (left, right) = (
end.slice(0, del_relative_from),
end.slice(del_relative_to, end.len()),
end.slice(del_relative_to, end.content_len()),
);
*end = self.bump.alloc(left);
@ -653,7 +662,9 @@ impl<'a, T: Rle, A: RleTreeTrait<T>> LeafNode<'a, T, A> {
if let Some(del_relative_to) = del_relative_to {
let self_ptr = self as *mut _;
let end = &mut self.children[del_end];
*end = self.bump.alloc(end.slice(del_relative_to, end.len()));
*end = self
.bump
.alloc(end.slice(del_relative_to, end.content_len()));
notify(end, self_ptr);
}
}

View file

@ -1,5 +1,4 @@
use std::ops::{Range};
use std::ops::Range;
use crate::{HasLength, Mergable, Slice, Sliceable};
@ -29,6 +28,14 @@ pub struct SearchResult<'a, T> {
pub offset: usize,
}
impl<T: Eq + PartialEq> PartialEq for RleVec<T> {
fn eq(&self, other: &Self) -> bool {
self.vec == other.vec
}
}
impl<T: Eq + PartialEq> Eq for RleVec<T> {}
impl<T: Mergable<Cfg> + HasLength, Cfg> RleVec<T, Cfg> {
/// push a new element to the end of the array. It may be merged with last element.
pub fn push(&mut self, value: T) {