From 4fc987f42a1c940adff0b3e7de9dd224b56d1d70 Mon Sep 17 00:00:00 2001 From: Zixuan Chen Date: Wed, 19 Oct 2022 15:21:08 +0800 Subject: [PATCH] fix: use better structure for log store access --- crates/loro-core/src/change.rs | 1 + crates/loro-core/src/configure.rs | 12 ++++ .../src/container/text/text_container.rs | 7 +-- crates/loro-core/src/log_store.rs | 60 +++---------------- crates/loro-core/src/loro.rs | 14 +++-- 5 files changed, 34 insertions(+), 60 deletions(-) diff --git a/crates/loro-core/src/change.rs b/crates/loro-core/src/change.rs index 947eb275..5f7fbb27 100644 --- a/crates/loro-core/src/change.rs +++ b/crates/loro-core/src/change.rs @@ -78,6 +78,7 @@ impl HasLength for Change { } } +#[derive(Debug)] pub struct ChangeMergeCfg { pub max_change_length: usize, pub max_change_interval: usize, diff --git a/crates/loro-core/src/configure.rs b/crates/loro-core/src/configure.rs index 7abaf6aa..fc2aceaa 100644 --- a/crates/loro-core/src/configure.rs +++ b/crates/loro-core/src/configure.rs @@ -1,3 +1,5 @@ +use std::fmt::Debug; + use crate::{change::ChangeMergeCfg, log_store::GcConfig, Timestamp}; use ring::rand::{SecureRandom, SystemRandom}; @@ -8,6 +10,16 @@ pub struct Configure { pub(crate) rand: Box, } +impl Debug for Configure { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("Configure") + .field("change", &self.change) + .field("gc", &self.gc) + .field("get_time", &self.get_time) + .finish() + } +} + pub trait SecureRandomGenerator { fn fill_byte(&mut self, dest: &mut [u8]); fn next_u64(&mut self) -> u64 { diff --git a/crates/loro-core/src/container/text/text_container.rs b/crates/loro-core/src/container/text/text_container.rs index f439ce67..b6ec27e9 100644 --- a/crates/loro-core/src/container/text/text_container.rs +++ b/crates/loro-core/src/container/text/text_container.rs @@ -1,5 +1,3 @@ -use std::ptr::NonNull; - use fxhash::FxHashMap; use rle::RleVec; @@ -8,10 +6,11 @@ use smallvec::SmallVec; use crate::{ container::{Container, ContainerID, ContainerType}, id::ID, + log_store::LogStoreWeakRef, op::OpProxy, span::IdSpan, value::LoroValue, - ClientID, LogStore, + ClientID, }; #[derive(Clone, Debug)] @@ -24,7 +23,7 @@ struct DagNode { pub struct TextContainer { id: ContainerID, sub_dag: FxHashMap>, - log_store: NonNull, + log_store: LogStoreWeakRef, } impl TextContainer { diff --git a/crates/loro-core/src/log_store.rs b/crates/loro-core/src/log_store.rs index 24d9ff00..d6b9d158 100644 --- a/crates/loro-core/src/log_store.rs +++ b/crates/loro-core/src/log_store.rs @@ -7,7 +7,7 @@ use std::{ marker::PhantomPinned, pin::Pin, ptr::NonNull, - sync::{Arc, RwLock}, + sync::{Arc, RwLock, Weak}, }; use fxhash::FxHashMap; @@ -27,6 +27,7 @@ use crate::{ const _YEAR: u64 = 365 * 24 * 60 * 60; const MONTH: u64 = 30 * 24 * 60 * 60; +#[derive(Debug)] pub struct GcConfig { pub gc: bool, pub interval: u64, @@ -41,6 +42,10 @@ impl Default for GcConfig { } } +pub type LogStoreRef = Arc>; +pub type LogStoreWeakRef = Weak>; + +#[derive(Debug)] /// LogStore stores the full history of Loro /// /// This is a self-referential structure. So it need to be pinned. @@ -48,7 +53,6 @@ impl Default for GcConfig { /// `frontier`s are the Changes without children in the DAG (there is no dep pointing to them) /// /// TODO: Refactor we need to move the things about the current state out of LogStore (container, latest_lamport, ..) -#[pin_project] pub struct LogStore { changes: FxHashMap>, cfg: Configure, @@ -56,22 +60,16 @@ pub struct LogStore { latest_timestamp: Timestamp, pub(crate) this_client_id: ClientID, frontier: SmallVec<[ID; 2]>, - accessor: Option>, - /// CRDT container manager pub(crate) container: ContainerManager, _pin: PhantomPinned, } -pub struct LogAccessor { - store: RwLock>, -} - impl LogStore { - pub fn new(mut cfg: Configure, client_id: Option) -> Pin> { + pub fn new(mut cfg: Configure, client_id: Option) -> Arc> { let this_client_id = client_id.unwrap_or_else(|| cfg.rand.next_u64()); - let mut this = Box::pin(Self { + let mut this = Arc::new(RwLock::new(Self { cfg, this_client_id, changes: FxHashMap::default(), @@ -81,33 +79,13 @@ impl LogStore { containers: Default::default(), store: NonNull::dangling(), }, - accessor: None, frontier: Default::default(), _pin: PhantomPinned, - }); + })); - let p = this.as_ref().get_ref(); - let p = p as *const _ as *mut LogStore; - // SAFETY: we just created this - this.container.store = unsafe { NonNull::new_unchecked(p) }; this } - pub fn get_accessor(&mut self) -> Arc { - if let Some(accessor) = self.accessor.as_ref() { - return accessor.clone(); - } - - let p = self as *const _ as *mut LogStore; - // SAFETY: this is self, which is non-null fore sure - let p = unsafe { NonNull::new_unchecked(p) }; - let accessor = Arc::new(LogAccessor { - store: RwLock::new(p), - }); - self.accessor = Some(accessor.clone()); - accessor - } - #[inline] pub fn lookup_change(&self, id: ID) -> Option<&Change> { self.changes @@ -237,23 +215,3 @@ impl LogStore { iter::OpIter::new(&self.changes) } } - -impl LogAccessor { - pub fn with_ref(&self, f: F) -> R - where - F: FnOnce(&LogStore) -> R, - { - let store = self.store.read().unwrap(); - // SAFETY: //FIXME: this is not guaranteed to be correct - f(unsafe { store.as_ref() }) - } - - pub fn with_mut(&self, f: F) -> R - where - F: FnOnce(&mut LogStore) -> R, - { - let mut store = self.store.write().unwrap(); - // SAFETY: //FIXME: this is not guaranteed to be correct - f(unsafe { store.as_mut() }) - } -} diff --git a/crates/loro-core/src/loro.rs b/crates/loro-core/src/loro.rs index bb402da0..134905f2 100644 --- a/crates/loro-core/src/loro.rs +++ b/crates/loro-core/src/loro.rs @@ -1,4 +1,4 @@ -use std::{pin::Pin}; +use std::sync::{Arc, RwLock}; use crate::{ configure::Configure, @@ -8,7 +8,7 @@ use crate::{ }; pub struct LoroCore { - pub store: Pin>, + pub store: Arc>, } impl Default for LoroCore { @@ -29,9 +29,13 @@ impl LoroCore { name: InternalString, container: ContainerType, ) -> &mut dyn Container { - self.store - .container - .get_or_create(&ContainerID::new_root(name, container)) + if let Ok(store) = self.store.get_mut() { + store + .container + .get_or_create(&ContainerID::new_root(name, container)) + } else { + todo!() + } } pub fn get_map_container(&mut self, name: InternalString) -> Option<&mut MapContainer> {