From 8dc788e40400c58522879dffef090c5b7188662d Mon Sep 17 00:00:00 2001 From: Zixuan Chen Date: Mon, 7 Nov 2022 23:19:13 +0800 Subject: [PATCH] refactor: remove isomophic and parallel feature features should be additive in Rust --- crates/loro-core/Cargo.toml | 1 - crates/loro-core/src/container/manager.rs | 27 +++---- .../src/container/map/map_container.rs | 2 +- crates/loro-core/src/container/map/tests.rs | 6 +- .../src/container/text/text_container.rs | 4 +- crates/loro-core/src/isomorph.rs | 79 ------------------- crates/loro-core/src/lib.rs | 1 - crates/loro-core/src/log_store.rs | 27 ++++--- crates/loro-core/src/loro.rs | 45 ++++++----- 9 files changed, 60 insertions(+), 132 deletions(-) delete mode 100644 crates/loro-core/src/isomorph.rs diff --git a/crates/loro-core/Cargo.toml b/crates/loro-core/Cargo.toml index d897e29f..5ef9217a 100644 --- a/crates/loro-core/Cargo.toml +++ b/crates/loro-core/Cargo.toml @@ -48,7 +48,6 @@ doctest = false [features] wasm = ["wasm-bindgen", "js-sys"] -parallel = [] mem-prof = [] # whether to use list slice instead of raw str in text container fuzzing = ["crdt-list/fuzzing", "rand", "arbitrary", "tabled"] diff --git a/crates/loro-core/src/container/manager.rs b/crates/loro-core/src/container/manager.rs index 2f4cda0d..5f887ec0 100644 --- a/crates/loro-core/src/container/manager.rs +++ b/crates/loro-core/src/container/manager.rs @@ -1,16 +1,13 @@ -use std::ops::{Deref, DerefMut}; +use std::{ + ops::{Deref, DerefMut}, + sync::{RwLockReadGuard, RwLockWriteGuard}, +}; use enum_as_inner::EnumAsInner; use fxhash::FxHashMap; use owning_ref::{OwningRef, OwningRefMut}; -use crate::{ - isomorph::{IsoRef, IsoRefMut}, - log_store::LogStoreWeakRef, - op::RemoteOp, - span::IdSpan, - LogStore, LoroError, -}; +use crate::{log_store::LogStoreWeakRef, op::RemoteOp, span::IdSpan, LogStore, LoroError}; use super::{ map::MapContainer, text::text_container::TextContainer, Container, ContainerID, ContainerType, @@ -153,21 +150,23 @@ impl ContainerManager { } pub struct ContainerRefMut<'a, T> { - value: OwningRefMut, Box>, + value: OwningRefMut, Box>, } pub struct ContainerRef<'a, T> { - value: OwningRef, Box>, + value: OwningRef, Box>, } -impl<'a, T> From, Box>> for ContainerRefMut<'a, T> { - fn from(value: OwningRefMut, Box>) -> Self { +impl<'a, T> From, Box>> + for ContainerRefMut<'a, T> +{ + fn from(value: OwningRefMut, Box>) -> Self { ContainerRefMut { value } } } -impl<'a, T> From, Box>> for ContainerRef<'a, T> { - fn from(value: OwningRef, Box>) -> Self { +impl<'a, T> From, Box>> for ContainerRef<'a, T> { + fn from(value: OwningRef, Box>) -> Self { ContainerRef { value } } } diff --git a/crates/loro-core/src/container/map/map_container.rs b/crates/loro-core/src/container/map/map_container.rs index 0d47dffc..a17d43cc 100644 --- a/crates/loro-core/src/container/map/map_container.rs +++ b/crates/loro-core/src/container/map/map_container.rs @@ -46,7 +46,7 @@ impl MapContainer { pub fn insert(&mut self, key: InternalString, value: InsertValue) { let self_id = &self.id; let m = self.store.upgrade().unwrap(); - let mut store = m.write(); + let mut store = m.write().unwrap(); let client_id = store.this_client_id; let order = TotalOrderStamp { client_id, diff --git a/crates/loro-core/src/container/map/tests.rs b/crates/loro-core/src/container/map/tests.rs index 99180631..04bb4f52 100644 --- a/crates/loro-core/src/container/map/tests.rs +++ b/crates/loro-core/src/container/map/tests.rs @@ -1,12 +1,12 @@ #![cfg(test)] use std::collections::HashMap; +use std::sync::Arc; use fxhash::FxHashMap; use proptest::prelude::*; use proptest::proptest; -use crate::isomorph::Irc; use crate::value::proptest::gen_insert_value; use crate::Container; @@ -15,7 +15,7 @@ use crate::{fx_map, value::InsertValue, LoroCore, LoroValue}; #[test] fn basic() { let mut loro = LoroCore::default(); - let _weak = Irc::downgrade(&loro.log_store); + let _weak = Arc::downgrade(&loro.log_store); let mut container = loro.get_or_create_root_map("map").unwrap(); container.insert("haha".into(), InsertValue::Int32(1)); let ans = fx_map!( @@ -37,7 +37,7 @@ mod map_proptest { value in prop::collection::vec(gen_insert_value(), 0..10 * PROPTEST_FACTOR_10) ) { let mut loro = LoroCore::default(); - let _weak = Irc::downgrade(&loro.log_store); + let _weak = Arc::downgrade(&loro.log_store); let mut container = loro.get_or_create_root_map("map").unwrap(); let mut map: HashMap = HashMap::new(); for (k, v) in key.iter().zip(value.iter()) { diff --git a/crates/loro-core/src/container/text/text_container.rs b/crates/loro-core/src/container/text/text_container.rs index 7ab370d9..06aa43bf 100644 --- a/crates/loro-core/src/container/text/text_container.rs +++ b/crates/loro-core/src/container/text/text_container.rs @@ -62,7 +62,7 @@ impl TextContainer { } let s = self.log_store.upgrade().unwrap(); - let mut store = s.write(); + let mut store = s.write().unwrap(); let id = store.next_id(); let slice = self.raw_str.alloc(text); self.state.insert(pos, slice.clone()); @@ -93,7 +93,7 @@ impl TextContainer { } let s = self.log_store.upgrade().unwrap(); - let mut store = s.write(); + let mut store = s.write().unwrap(); let id = store.next_id(); let op = Op::new( id, diff --git a/crates/loro-core/src/isomorph.rs b/crates/loro-core/src/isomorph.rs deleted file mode 100644 index 69a90b59..00000000 --- a/crates/loro-core/src/isomorph.rs +++ /dev/null @@ -1,79 +0,0 @@ -#![allow(unused)] -use std::{ - cell::{Ref, RefCell, RefMut}, - rc::{Rc, Weak as RcWeak}, - sync::{Arc, RwLock, RwLockReadGuard, RwLockWriteGuard, Weak as ArcWeak}, -}; - -#[cfg(feature = "parallel")] -pub(crate) type Irc = Arc; -#[cfg(not(feature = "parallel"))] -pub(crate) type Irc = Rc; - -#[cfg(feature = "parallel")] -pub(crate) type IsoWeak = ArcWeak; -#[cfg(not(feature = "parallel"))] -pub(crate) type IsoWeak = RcWeak; - -#[cfg(feature = "parallel")] -#[derive(Debug)] -pub(crate) struct IsoRw(RwLock); -#[cfg(not(feature = "parallel"))] -#[derive(Debug)] -pub(crate) struct IsoRw(RefCell); - -#[cfg(feature = "parallel")] -pub(crate) type IsoRef<'a, T> = RwLockReadGuard<'a, T>; -#[cfg(not(feature = "parallel"))] -pub(crate) type IsoRef<'a, T> = Ref<'a, T>; - -#[cfg(feature = "parallel")] -pub(crate) type IsoRefMut<'a, T> = RwLockWriteGuard<'a, T>; -#[cfg(not(feature = "parallel"))] -pub(crate) type IsoRefMut<'a, T> = RefMut<'a, T>; - -#[cfg(feature = "parallel")] -mod rw_parallel { - use super::*; - - impl IsoRw { - #[inline(always)] - pub fn new(t: T) -> Self { - Self(RwLock::new(t)) - } - - #[inline(always)] - pub fn read(&self) -> std::sync::RwLockReadGuard { - self.0.read().unwrap() - } - - #[inline(always)] - pub fn write(&self) -> std::sync::RwLockWriteGuard { - self.0.write().unwrap() - } - } -} - -#[cfg(not(feature = "parallel"))] -mod rw_single { - use std::{cell::RefCell, ops::Deref}; - - use super::IsoRw; - - impl IsoRw { - #[inline(always)] - pub fn new(t: T) -> Self { - IsoRw(RefCell::new(t)) - } - - #[inline(always)] - pub fn read(&self) -> std::cell::Ref { - self.0.borrow() - } - - #[inline(always)] - pub fn write(&self) -> std::cell::RefMut { - self.0.borrow_mut() - } - } -} diff --git a/crates/loro-core/src/lib.rs b/crates/loro-core/src/lib.rs index a8398b61..757fd3da 100644 --- a/crates/loro-core/src/lib.rs +++ b/crates/loro-core/src/lib.rs @@ -18,7 +18,6 @@ pub mod version; mod error; #[cfg(feature = "fuzzing")] pub mod fuzz; -mod isomorph; mod loro; mod smstring; mod snapshot; diff --git a/crates/loro-core/src/log_store.rs b/crates/loro-core/src/log_store.rs index 609196de..b50c18b5 100644 --- a/crates/loro-core/src/log_store.rs +++ b/crates/loro-core/src/log_store.rs @@ -2,7 +2,11 @@ //! //! mod iter; -use std::{marker::PhantomPinned, ops::Range}; +use std::{ + marker::PhantomPinned, + ops::Range, + sync::{Arc, RwLock, Weak}, +}; use fxhash::{FxHashMap, FxHashSet}; @@ -20,7 +24,6 @@ use crate::{ dag::Dag, debug_log, id::{ClientID, ContainerIdx, Counter}, - isomorph::{Irc, IsoRw, IsoWeak}, op::RemoteOp, span::{HasCounterSpan, HasIdSpan, HasLamportSpan, IdSpan}, Lamport, Op, Timestamp, VersionVector, ID, @@ -44,8 +47,8 @@ impl Default for GcConfig { } } -pub(crate) type LogStoreRef = Irc>; -pub(crate) type LogStoreWeakRef = IsoWeak>; +pub(crate) type LogStoreRef = Arc>; +pub(crate) type LogStoreWeakRef = Weak>; #[derive(Debug)] /// LogStore stores the full history of Loro @@ -64,8 +67,8 @@ pub struct LogStore { pub(crate) this_client_id: ClientID, frontier: SmallVec<[ID; 2]>, /// CRDT container manager - pub(crate) container: IsoWeak>, - to_self: IsoWeak>, + pub(crate) container: Weak>, + to_self: Weak>, container_to_idx: FxHashMap, idx_to_container: Vec, _pin: PhantomPinned, @@ -75,11 +78,11 @@ impl LogStore { pub(crate) fn new( mut cfg: Configure, client_id: Option, - container: IsoWeak>, - ) -> Irc> { + container: Weak>, + ) -> Arc> { let this_client_id = client_id.unwrap_or_else(|| cfg.rand.next_u64()); - Irc::new_cyclic(|x| { - IsoRw::new(Self { + Arc::new_cyclic(|x| { + RwLock::new(Self { cfg, this_client_id, changes: FxHashMap::default(), @@ -175,7 +178,7 @@ impl LogStore { fn change_to_export_format(&self, change: Change) -> Change { let upgraded = self.container.upgrade().unwrap(); - let container_manager = upgraded.read(); + let container_manager = upgraded.read().unwrap(); let mut ops = RleVec::new(); for mut op in change.ops.into_iter() { let container = container_manager @@ -296,7 +299,7 @@ impl LogStore { // TODO: find a way to remove this clone? we don't need change in apply method actually let upgraded = self.container.upgrade().unwrap(); - let mut container_manager = upgraded.write(); + let mut container_manager = upgraded.write().unwrap(); let change = self.change_to_imported_format(&mut container_manager, change); let v = self .changes diff --git a/crates/loro-core/src/loro.rs b/crates/loro-core/src/loro.rs index da03b84f..19588d34 100644 --- a/crates/loro-core/src/loro.rs +++ b/crates/loro-core/src/loro.rs @@ -1,3 +1,5 @@ +use std::sync::{Arc, RwLock}; + use owning_ref::{OwningRef, OwningRefMut}; use crate::{ @@ -10,14 +12,13 @@ use crate::{ ContainerID, ContainerType, }, id::ClientID, - isomorph::{Irc, IsoRw}, op::RemoteOp, LogStore, LoroError, VersionVector, }; pub struct LoroCore { - pub(crate) log_store: Irc>, - pub(crate) container: Irc>, + pub(crate) log_store: Arc>, + pub(crate) container: Arc>, } impl Default for LoroCore { @@ -28,8 +29,8 @@ impl Default for LoroCore { impl LoroCore { pub fn new(cfg: Configure, client_id: Option) -> Self { - let container = Irc::new(IsoRw::new(ContainerManager::new())); - let weak = Irc::downgrade(&container); + let container = Arc::new(RwLock::new(ContainerManager::new())); + let weak = Arc::downgrade(&container); Self { log_store: LogStore::new(cfg, client_id, weak), container, @@ -37,7 +38,7 @@ impl LoroCore { } pub fn vv(&self) -> VersionVector { - self.log_store.read().get_vv().clone() + self.log_store.read().unwrap().get_vv().clone() } #[inline(always)] @@ -45,10 +46,13 @@ impl LoroCore { &mut self, name: &str, ) -> Result, LoroError> { - let mut a = OwningRefMut::new(self.container.write()); + let mut a = OwningRefMut::new(self.container.write().unwrap()); let id = ContainerID::new_root(name, ContainerType::Map); - self.log_store.write().get_or_create_container_idx(&id); - let ptr = Irc::downgrade(&self.log_store); + self.log_store + .write() + .unwrap() + .get_or_create_container_idx(&id); + let ptr = Arc::downgrade(&self.log_store); a.get_or_create(&id, ptr)?; Ok( a.map_mut(move |x| x.get_mut(&id).unwrap().as_map_mut().unwrap()) @@ -61,10 +65,13 @@ impl LoroCore { &mut self, name: &str, ) -> Result, LoroError> { - let mut a = OwningRefMut::new(self.container.write()); + let mut a = OwningRefMut::new(self.container.write().unwrap()); let id = ContainerID::new_root(name, ContainerType::Text); - self.log_store.write().get_or_create_container_idx(&id); - let ptr = Irc::downgrade(&self.log_store); + self.log_store + .write() + .unwrap() + .get_or_create_container_idx(&id); + let ptr = Arc::downgrade(&self.log_store); a.get_or_create(&id, ptr)?; Ok( a.map_mut(move |x| x.get_mut(&id).unwrap().as_text_mut().unwrap()) @@ -77,7 +84,7 @@ impl LoroCore { &mut self, id: &ContainerID, ) -> Result, LoroError> { - let a = OwningRefMut::new(self.container.write()); + let a = OwningRefMut::new(self.container.write().unwrap()); Ok( a.map_mut(move |x| x.get_mut(id).unwrap().as_map_mut().unwrap()) .into(), @@ -89,7 +96,7 @@ impl LoroCore { &mut self, id: &ContainerID, ) -> Result, LoroError> { - let a = OwningRefMut::new(self.container.write()); + let a = OwningRefMut::new(self.container.write().unwrap()); Ok( a.map_mut(move |x| x.get_mut(id).unwrap().as_text_mut().unwrap()) .into(), @@ -101,23 +108,23 @@ impl LoroCore { &self, id: &ContainerID, ) -> Result, LoroError> { - let a = OwningRef::new(self.container.read()); + let a = OwningRef::new(self.container.read().unwrap()); Ok(a.map(move |x| x.get(id).unwrap().as_text().unwrap()).into()) } pub fn export(&self, remote_vv: VersionVector) -> Vec> { - let store = self.log_store.read(); + let store = self.log_store.read().unwrap(); store.export(&remote_vv) } pub fn import(&mut self, changes: Vec>) { - let mut store = self.log_store.write(); + let mut store = self.log_store.write().unwrap(); store.import(changes) } #[cfg(feature = "fuzzing")] pub fn debug_inspect(&self) { - self.log_store.write().debug_inspect(); - self.container.write().debug_inspect(); + self.log_store.write().unwrap().debug_inspect(); + self.container.write().unwrap().debug_inspect(); } }