From cb134fc7c7b093157c63947798e83889bfd392e3 Mon Sep 17 00:00:00 2001 From: Leon Zhao Date: Thu, 5 Sep 2024 16:03:47 +0800 Subject: [PATCH] feat: add `with fractional index` config (#442) * feat: add `with_fractional_index` config * fix: share config state --- Cargo.lock | 1 + crates/fractional_index/Cargo.toml | 1 + crates/fractional_index/src/lib.rs | 4 +- crates/fuzz/src/actor.rs | 3 + crates/fuzz/src/container/map.rs | 8 ++- crates/fuzz/src/container/tree.rs | 8 ++- crates/loro-internal/src/configure.rs | 13 ++++ crates/loro-internal/src/handler/tree.rs | 1 - crates/loro-internal/src/loro.rs | 6 ++ crates/loro-internal/src/state.rs | 17 ++++- crates/loro-internal/src/state/tree_state.rs | 76 +++++++++++--------- crates/loro/src/lib.rs | 6 ++ 12 files changed, 102 insertions(+), 42 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 88d9366e..a6e55158 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1349,6 +1349,7 @@ dependencies = [ "criterion 0.5.1", "fractional_index", "imbl", + "once_cell", "rand", "serde", "smallvec", diff --git a/crates/fractional_index/Cargo.toml b/crates/fractional_index/Cargo.toml index b7533e39..c0477fcd 100644 --- a/crates/fractional_index/Cargo.toml +++ b/crates/fractional_index/Cargo.toml @@ -16,6 +16,7 @@ imbl = "^3.0" smallvec = { workspace = true } serde = { workspace = true, features = ["derive", "rc"], optional = true } rand = { version = "^0.8" } +once_cell = { workspace = true } [dev-dependencies] fraction_index = { version = "^2.0", package = "fractional_index" } diff --git a/crates/fractional_index/src/lib.rs b/crates/fractional_index/src/lib.rs index d7b3835a..a76e2c3c 100644 --- a/crates/fractional_index/src/lib.rs +++ b/crates/fractional_index/src/lib.rs @@ -9,6 +9,8 @@ use serde::{Deserialize, Serialize}; mod jitter; const TERMINATOR: u8 = 128; +static DEFAULT_FRACTIONAL_INDEX: once_cell::sync::Lazy = + once_cell::sync::Lazy::new(|| FractionalIndex(Arc::new(vec![TERMINATOR]))); #[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)] #[cfg_attr(feature = "serde", derive(Serialize, Deserialize))] @@ -16,7 +18,7 @@ pub struct FractionalIndex(Arc>); impl Default for FractionalIndex { fn default() -> Self { - FractionalIndex(Arc::new(vec![TERMINATOR])) + DEFAULT_FRACTIONAL_INDEX.clone() } } diff --git a/crates/fuzz/src/actor.rs b/crates/fuzz/src/actor.rs index bc855cb9..b9c32759 100644 --- a/crates/fuzz/src/actor.rs +++ b/crates/fuzz/src/actor.rs @@ -24,6 +24,8 @@ use super::{ container::MapActor, }; +const DEFAULT_WITH_FRACTIONAL_INDEX: bool = false; + #[derive(Debug)] pub struct Undo { pub undo: UndoManager, @@ -45,6 +47,7 @@ impl Actor { pub fn new(id: PeerID) -> Self { let loro = LoroDoc::new(); loro.set_peer_id(id).unwrap(); + loro.set_with_fractional_index(DEFAULT_WITH_FRACTIONAL_INDEX); let undo = UndoManager::new(&loro); let tracker = Arc::new(Mutex::new(ContainerTracker::Map(MapTracker::empty( ContainerID::new_root("sys:root", ContainerType::Map), diff --git a/crates/fuzz/src/container/map.rs b/crates/fuzz/src/container/map.rs index c68a6553..8be43e85 100644 --- a/crates/fuzz/src/container/map.rs +++ b/crates/fuzz/src/container/map.rs @@ -7,7 +7,7 @@ use loro::{Container, ContainerID, ContainerType, LoroDoc, LoroMap, LoroValue}; use crate::{ actions::{Actionable, FromGenericAction, GenericAction}, - actor::{ActionExecutor, ActorTrait}, + actor::{assert_value_eq, ActionExecutor, ActorTrait}, crdt_fuzzer::FuzzValue, value::{ApplyDiff, ContainerTracker, MapTracker, Value}, }; @@ -66,7 +66,11 @@ impl ActorTrait for MapActor { let map = self.loro.get_map("map"); let value_a = map.get_deep_value(); let value_b = self.tracker.lock().unwrap().to_value(); - assert_eq!(&value_a, value_b.into_map().unwrap().get("map").unwrap()); + assert_value_eq( + &value_a, + value_b.into_map().unwrap().get("map").unwrap(), + None, + ); } fn container_len(&self) -> u8 { diff --git a/crates/fuzz/src/container/tree.rs b/crates/fuzz/src/container/tree.rs index b6c50dd2..5d9b39c5 100644 --- a/crates/fuzz/src/container/tree.rs +++ b/crates/fuzz/src/container/tree.rs @@ -14,7 +14,7 @@ use tracing::{debug, trace}; use crate::{ actions::{Actionable, FromGenericAction, GenericAction}, - actor::{ActionExecutor, ActorTrait}, + actor::{assert_value_eq, ActionExecutor, ActorTrait}, crdt_fuzzer::FuzzValue, value::{ApplyDiff, ContainerTracker, MapTracker, Value}, }; @@ -135,7 +135,11 @@ impl ActorTrait for TreeActor { let tree = loro.get_tree("tree"); let result = tree.get_value_with_meta(); let tracker = self.tracker.lock().unwrap().to_value(); - assert_eq!(&result, tracker.into_map().unwrap().get("tree").unwrap()); + assert_value_eq( + &result, + tracker.into_map().unwrap().get("tree").unwrap(), + None, + ); } fn add_new_container(&mut self, container: Container) { diff --git a/crates/loro-internal/src/configure.rs b/crates/loro-internal/src/configure.rs index c3ed81f3..69364823 100644 --- a/crates/loro-internal/src/configure.rs +++ b/crates/loro-internal/src/configure.rs @@ -5,6 +5,9 @@ pub struct Configure { pub(crate) text_style_config: Arc>, record_timestamp: Arc, pub(crate) merge_interval: Arc, + /// Whether the tree has fractional index. `false` by default. If false, the fractional index is always [`FractionalIndex::default`] and + /// `tree_position_jitter` is not used. + pub(crate) tree_with_fractional_index: Arc, /// do not use `jitter` by default pub(crate) tree_position_jitter: Arc, } @@ -16,6 +19,7 @@ impl Default for Configure { record_timestamp: Arc::new(AtomicBool::new(false)), merge_interval: Arc::new(AtomicI64::new(1000 * 1000)), tree_position_jitter: Arc::new(AtomicU8::new(0)), + tree_with_fractional_index: Arc::new(AtomicBool::new(false)), } } } @@ -38,6 +42,10 @@ impl Configure { self.tree_position_jitter .load(std::sync::atomic::Ordering::Relaxed), )), + tree_with_fractional_index: Arc::new(AtomicBool::new( + self.tree_with_fractional_index + .load(std::sync::atomic::Ordering::Relaxed), + )), } } @@ -55,6 +63,11 @@ impl Configure { .store(record, std::sync::atomic::Ordering::Relaxed); } + pub fn set_with_fractional_index(&self, with_fractional_index: bool) { + self.tree_with_fractional_index + .store(with_fractional_index, std::sync::atomic::Ordering::Relaxed); + } + pub fn set_fractional_index_jitter(&self, jitter: u8) { self.tree_position_jitter .store(jitter, std::sync::atomic::Ordering::Relaxed); diff --git a/crates/loro-internal/src/handler/tree.rs b/crates/loro-internal/src/handler/tree.rs index b7c6c423..4d772c8f 100644 --- a/crates/loro-internal/src/handler/tree.rs +++ b/crates/loro-internal/src/handler/tree.rs @@ -210,7 +210,6 @@ impl HandlerTrait for TreeHandler { self.inner.attached_handler() } - // TODO: fn get_value(&self) -> LoroValue { match &self.inner { MaybeDetached::Detached(t) => { diff --git a/crates/loro-internal/src/loro.rs b/crates/loro-internal/src/loro.rs index 59d47264..cde05faf 100644 --- a/crates/loro-internal/src/loro.rs +++ b/crates/loro-internal/src/loro.rs @@ -173,6 +173,12 @@ impl LoroDoc { self.config.set_merge_interval(interval); } + /// Set whether to use fractional index for Tree Position. + #[inline] + pub fn set_with_fractional_index(&self, with_fractional_index: bool) { + self.config.set_with_fractional_index(with_fractional_index); + } + /// Set the jitter of the tree position(Fractional Index). /// /// The jitter is used to avoid conflicts when multiple users are creating the node at the same position. diff --git a/crates/loro-internal/src/state.rs b/crates/loro-internal/src/state.rs index 3a562158..2b252f71 100644 --- a/crates/loro-internal/src/state.rs +++ b/crates/loro-internal/src/state.rs @@ -3,7 +3,7 @@ use std::{ collections::BTreeMap, io::Write, sync::{ - atomic::{AtomicU64, AtomicU8, Ordering}, + atomic::{AtomicBool, AtomicU64, AtomicU8, Ordering}, Arc, Mutex, RwLock, Weak, }, }; @@ -310,8 +310,18 @@ impl State { Self::RichtextState(Box::new(RichtextState::new(idx, config))) } - pub fn new_tree(idx: ContainerIdx, peer: PeerID, jitter: Arc) -> Self { - Self::TreeState(Box::new(TreeState::new(idx, peer, jitter))) + pub fn new_tree( + idx: ContainerIdx, + peer: PeerID, + jitter: Arc, + with_fractional_index: Arc, + ) -> Self { + Self::TreeState(Box::new(TreeState::new( + idx, + peer, + jitter, + with_fractional_index, + ))) } pub fn new_unknown(idx: ContainerIdx) -> Self { @@ -1476,6 +1486,7 @@ fn create_state_(idx: ContainerIdx, config: &Configure, peer: u64) -> State { idx, peer, config.tree_position_jitter.clone(), + config.tree_with_fractional_index.clone(), ))), ContainerType::MovableList => State::MovableListState(Box::new(MovableListState::new(idx))), #[cfg(feature = "counter")] diff --git a/crates/loro-internal/src/state/tree_state.rs b/crates/loro-internal/src/state/tree_state.rs index acdc88f0..45091428 100644 --- a/crates/loro-internal/src/state/tree_state.rs +++ b/crates/loro-internal/src/state/tree_state.rs @@ -13,7 +13,7 @@ use serde::Serialize; use std::collections::VecDeque; use std::fmt::Debug; use std::ops::{Deref, DerefMut}; -use std::sync::atomic::{AtomicU8, Ordering}; +use std::sync::atomic::{AtomicBool, AtomicU8, Ordering}; use std::sync::{Arc, Mutex, Weak}; use super::{ContainerState, DiffApplyContext}; @@ -41,8 +41,10 @@ pub struct TreeState { idx: ContainerIdx, trees: FxHashMap, children: TreeChildrenCache, + /// Whether the tree has fractional index. If false, the fractional index is always [`FractionalIndex::default`] + with_fractional_index: Arc, rng: Option, - jitter: u8, + jitter: Arc, } #[derive(Debug, Clone, PartialEq, Eq)] @@ -597,16 +599,22 @@ impl NodePosition { } impl TreeState { - pub fn new(idx: ContainerIdx, peer_id: PeerID, config: Arc) -> Self { - let jitter = config.load(Ordering::Relaxed); + pub fn new( + idx: ContainerIdx, + peer_id: PeerID, + jitter_config: Arc, + with_fractional_index: Arc, + ) -> Self { + let jitter = jitter_config.load(Ordering::Relaxed); let use_jitter = jitter != 1; Self { idx, trees: FxHashMap::default(), children: Default::default(), + with_fractional_index, rng: use_jitter.then_some(rand::rngs::StdRng::seed_from_u64(peer_id)), - jitter, + jitter: jitter_config, } } @@ -677,10 +685,6 @@ impl TreeState { !self.is_node_deleted(&target) } - pub fn contains_internal(&self, target: &TreeID) -> bool { - self.trees.contains_key(target) - } - /// Get the parent of the node, if the node is deleted or does not exist, return None pub fn parent(&self, target: &TreeID) -> TreeParentId { self.trees @@ -823,11 +827,15 @@ impl TreeState { parent: &TreeParentId, index: usize, ) -> FractionalIndexGenResult { + if !self.with_fractional_index.load(Ordering::Relaxed) { + return FractionalIndexGenResult::Ok(FractionalIndex::default()); + } + if let Some(rng) = self.rng.as_mut() { self.children .entry(*parent) .or_default() - .generate_fi_at_jitter(index, target, rng, self.jitter) + .generate_fi_at_jitter(index, target, rng, self.jitter.load(Ordering::Relaxed)) } else { self.children .entry(*parent) @@ -936,28 +944,26 @@ impl ContainerState for TreeState { }); } // Otherwise, it's a normal move inside deleted nodes, no event is needed + } else if was_alive { + // normal move + ans.push(TreeDiffItem { + target, + action: TreeExternalDiff::Move { + parent: parent.into_node().ok(), + index: self.get_index_by_tree_id(&target).unwrap(), + position: position.clone(), + }, + }); } else { - if was_alive { - // normal move - ans.push(TreeDiffItem { - target, - action: TreeExternalDiff::Move { - parent: parent.into_node().ok(), - index: self.get_index_by_tree_id(&target).unwrap(), - position: position.clone(), - }, - }); - } else { - // create event - ans.push(TreeDiffItem { - target, - action: TreeExternalDiff::Create { - parent: parent.into_node().ok(), - index: self.get_index_by_tree_id(&target).unwrap(), - position: position.clone(), - }, - }); - } + // create event + ans.push(TreeDiffItem { + target, + action: TreeExternalDiff::Create { + parent: parent.into_node().ok(), + index: self.get_index_by_tree_id(&target).unwrap(), + position: position.clone(), + }, + }); } } } else { @@ -1490,8 +1496,12 @@ mod snapshot { peers.push(PeerID::from_le_bytes(buf)); } - let mut tree = - TreeState::new(idx, ctx.peer, ctx.configure.tree_position_jitter.clone()); + let mut tree = TreeState::new( + idx, + ctx.peer, + ctx.configure.tree_position_jitter.clone(), + ctx.configure.tree_with_fractional_index.clone(), + ); let encoded: EncodedTree = serde_columnar::from_bytes(bytes)?; let fractional_indexes = PositionArena::decode(&encoded.fractional_indexes).unwrap(); let fractional_indexes = fractional_indexes.parse_to_positions(); diff --git a/crates/loro/src/lib.rs b/crates/loro/src/lib.rs index 0b3282cb..f9fe2758 100644 --- a/crates/loro/src/lib.rs +++ b/crates/loro/src/lib.rs @@ -155,6 +155,12 @@ impl LoroDoc { self.doc.set_change_merge_interval(interval); } + /// Set whether to use fractional index for Tree Position. + #[inline] + pub fn set_with_fractional_index(&self, with_fractional_index: bool) { + self.doc.set_with_fractional_index(with_fractional_index); + } + /// Set the jitter of the tree position(Fractional Index). /// /// The jitter is used to avoid conflicts when multiple users are creating the node at the same position.