refactor: re-implement map type (#26)

y-octo/benches/map_ops_benchmarks.rs

@@ -16,7 +16,7 @@ fn operations(c: &mut Criterion) {
             let doc = Doc::default();
             let mut map = doc.get_or_create_map("test").unwrap();
             for (idx, key) in base_text.iter().enumerate() {
-                map.insert(key, idx).unwrap();
+                map.insert(key.to_string(), idx).unwrap();
             }
         });
     });
@@ -49,7 +49,7 @@ fn operations(c: &mut Criterion) {
         let doc = Doc::default();
         let mut map = doc.get_or_create_map("test").unwrap();
         for (idx, key) in base_text.iter().enumerate() {
-            map.insert(key, idx).unwrap();
+            map.insert(key.to_string(), idx).unwrap();
         }
 
         b.iter(|| {
@@ -93,7 +93,7 @@ fn operations(c: &mut Criterion) {
             let doc = Doc::default();
             let mut map = doc.get_or_create_map("test").unwrap();
             for (idx, key) in base_text.iter().enumerate() {
-                map.insert(key, idx).unwrap();
+                map.insert(key.to_string(), idx).unwrap();
             }
             for key in &base_text {
                 map.remove(key);

y-octo/bin/memory_leak_test.rs

@@ -63,7 +63,7 @@ fn run_map_test() {
         let doc = Doc::default();
         let mut map = doc.get_or_create_map("test").unwrap();
         for (idx, key) in base_text.iter().enumerate() {
-            map.insert(key, idx).unwrap();
+            map.insert(key.to_string(), idx).unwrap();
         }
     }
 }

y-octo/src/doc/codec/item.rs

@@ -1,5 +1,5 @@
 use super::*;
-use crate::sync::{Arc, AtomicU8, Ordering};
+use crate::sync::{AtomicU8, Ordering};
 
 #[derive(Debug, Clone)]
 #[cfg_attr(test, derive(proptest_derive::Arbitrary))]
@@ -118,16 +118,13 @@ pub(crate) struct Item {
     pub id: Id,
     pub origin_left_id: Option<Id>,
     pub origin_right_id: Option<Id>,
-    #[cfg_attr(all(test, not(loom)), proptest(value = "Somr::default()"))]
-    pub left: Somr<Item>,
-    #[cfg_attr(all(test, not(loom)), proptest(value = "Somr::default()"))]
-    pub right: Somr<Item>,
+    #[cfg_attr(all(test, not(loom)), proptest(value = "Somr::none()"))]
+    pub left: ItemRef,
+    #[cfg_attr(all(test, not(loom)), proptest(value = "Somr::none()"))]
+    pub right: ItemRef,
     pub parent: Option<Parent>,
     pub parent_sub: Option<String>,
-    // make content Arc, so we can share the content between items
-    // and item can be readonly and cloned fast.
-    // TODO: considering using Cow
-    pub content: Arc<Content>,
+    pub content: Content,
     #[cfg_attr(all(test, not(loom)), proptest(value = "ItemFlags::default()"))]
     pub flags: ItemFlags,
 }
@@ -187,7 +184,7 @@ impl Default for Item {
             right: Somr::none(),
             parent: None,
             parent_sub: None,
-            content: Arc::new(Content::Deleted(0)),
+            content: Content::Deleted(0),
             flags: ItemFlags::from(0),
         }
     }
@@ -216,7 +213,7 @@ impl Item {
             right,
             parent,
             parent_sub,
-            content: Arc::new(content),
+            content,
             flags,
         }
     }
@@ -369,7 +366,7 @@ impl Item {
                 // tag must not GC or Skip, this must process in parse_struct
                 debug_assert_ne!(first_5_bit, 0);
                 debug_assert_ne!(first_5_bit, 10);
-                Arc::new(Content::read(decoder, first_5_bit)?)
+                Content::read(decoder, first_5_bit)?
             },
             left: Somr::none(),
             right: Somr::none(),
@@ -380,7 +377,7 @@ impl Item {
             item.flags.set_countable();
         }
 
-        if matches!(item.content.as_ref(), Content::Deleted(_)) {
+        if matches!(item.content, Content::Deleted(_)) {
             item.flags.set_deleted();
         }
 

y-octo/src/doc/codec/refs.rs

@@ -1,5 +1,3 @@
-use sync::Arc;
-
 use super::*;
 
 // make fields Copy + Clone without much effort
@@ -81,7 +79,7 @@ impl Node {
             _ => {
                 let item = Somr::new(Item::read(decoder, id, info, first_5_bit)?);
 
-                if let Content::Type(ty) = item.get().unwrap().content.as_ref() {
+                if let Content::Type(ty) = &item.get().unwrap().content {
                     if let Some(mut ty) = ty.ty_mut() {
                         ty.item = item.clone();
                     }
@@ -271,7 +269,7 @@ impl Node {
                     return false;
                 }
 
-                match (Arc::make_mut(&mut litem.content), Arc::make_mut(&mut ritem.content)) {
+                match (&mut litem.content, &mut ritem.content) {
                     (Content::Deleted(l), Content::Deleted(r)) => {
                         *l += *r;
                     }

y-octo/src/doc/codec/utils/items.rs

@@ -1,5 +1,4 @@
 use super::{item::item_flags, *};
-use crate::sync::Arc;
 
 pub(crate) struct ItemBuilder {
     item: Item,
@@ -54,7 +53,7 @@ impl ItemBuilder {
     }
 
     pub fn content(mut self, content: Content) -> ItemBuilder {
-        self.item.content = Arc::new(content);
+        self.item.content = content;
         self
     }
 
@@ -92,7 +91,7 @@ mod tests {
             assert_eq!(item.origin_right_id, Some(Id::new(4, 5)));
             assert!(matches!(item.parent, Some(Parent::String(text)) if text == "test"));
             assert_eq!(item.parent_sub, None);
-            assert_eq!(item.content, Arc::new(Content::Any(vec![Any::String("Hello".into())])));
+            assert_eq!(item.content, Content::Any(vec![Any::String("Hello".into())]));
         });
     }
 }

y-octo/src/doc/document.rs

@@ -416,7 +416,7 @@ mod tests {
                 let doc = Doc::new();
 
                 let mut map = doc.get_or_create_map("abc").unwrap();
-                map.insert("a", 1).unwrap();
+                map.insert("a".to_string(), 1).unwrap();
                 let binary = doc.encode_update_v1().unwrap();
 
                 let doc_new = Doc::new();

y-octo/src/doc/history.rs

@@ -119,9 +119,7 @@ impl StoreHistory {
             histories.push(History {
                 id: item.id.to_string(),
                 parent: Self::parse_path(item, &parents),
-                content: Value::try_from(item.content.as_ref())
-                    .map(|v| v.to_string())
-                    .unwrap_or("unknown".to_owned()),
+                content: Value::from(&item.content).to_string(),
             })
         }
 
@@ -243,8 +241,8 @@ mod test {
             let doc = Doc::default();
             let mut map = doc.get_or_create_map("map").unwrap();
             let mut sub_map = doc.create_map().unwrap();
-            map.insert("sub_map", sub_map.clone()).unwrap();
-            sub_map.insert("key", "value").unwrap();
+            map.insert("sub_map".to_string(), sub_map.clone()).unwrap();
+            sub_map.insert("key".to_string(), "value").unwrap();
 
             assert_eq!(doc.clients()[0], doc.client());
         });
@@ -256,8 +254,8 @@ mod test {
             let doc = Doc::default();
             let mut map = doc.get_or_create_map("map").unwrap();
             let mut sub_map = doc.create_map().unwrap();
-            map.insert("sub_map", sub_map.clone()).unwrap();
-            sub_map.insert("key", "value").unwrap();
+            map.insert("sub_map".to_string(), sub_map.clone()).unwrap();
+            sub_map.insert("key".to_string(), "value").unwrap();
 
             let history = StoreHistory::new(&doc.store);
 

y-octo/src/doc/publisher.rs

@@ -201,12 +201,12 @@ mod tests {
             });
 
             let mut map = doc.get_or_create_map("test").unwrap();
-            map.insert("key1", "val1").unwrap();
+            map.insert("key1".to_string(), "val1").unwrap();
 
             sleep(Duration::from_secs(1));
 
-            map.insert("key2", "val2").unwrap();
-            map.insert("key3", "val3").unwrap();
+            map.insert("key2".to_string(), "val2").unwrap();
+            map.insert("key3".to_string(), "val3").unwrap();
             sleep(Duration::from_secs(1));
 
             let mut array = doc.get_or_create_array("array").unwrap();

y-octo/src/doc/store.rs

@@ -174,7 +174,7 @@ impl DocStore {
         let id = (self.client(), self.get_state(self.client())).into();
         let item = Somr::new(Item::new(id, content, left, right, parent, parent_sub));
 
-        if let Content::Type(ty) = item.get().unwrap().content.as_ref() {
+        if let Content::Type(ty) = &item.get().unwrap().content {
             if let Some(mut ty) = ty.ty_mut() {
                 ty.item = item.clone();
             }
@@ -375,15 +375,12 @@ impl DocStore {
             Some(Parent::Id(parent_id)) => {
                 match self.get_node(*parent_id) {
                     Some(Node::Item(parent_item)) => {
-                        match &parent_item.get().unwrap().content.as_ref() {
-                            Content::Type(ty) => {
-                                item.parent.replace(Parent::Type(ty.clone()));
-                            }
-                            _ => {
-                                // invalid parent, take it.
-                                item.parent.take();
-                                // return Err(JwstCodecError::InvalidParent);
-                            }
+                        if let Content::Type(ty) = &parent_item.get().unwrap().content {
+                            item.parent.replace(Parent::Type(ty.clone()));
+                        } else {
+                            // invalid parent, take it.
+                            item.parent.take();
+                            // return Err(JwstCodecError::InvalidParent);
                         }
                     }
                     _ => {
@@ -406,7 +403,7 @@ impl DocStore {
         };
 
         // assign store in ytype to ensure store exists if a ytype not has any children
-        if let Content::Type(ty) = Arc::make_mut(&mut item.content) {
+        if let Content::Type(ty) = &mut item.content {
             ty.store = Arc::downgrade(&store_ref);
 
             // we keep ty owner in dangling_types so the delete of any type will not make it
@@ -450,7 +447,7 @@ impl DocStore {
                         this.origin_left_id = left_ref.get().map(|left| left.last_id());
                         this.left = left_ref;
                     }
-                    this.content = Arc::new(this.content.split(offset)?.1);
+                    this.content = this.content.split(offset)?.1;
                 }
 
                 if let Some(Parent::Type(ty)) = &this.parent {
@@ -618,7 +615,7 @@ impl DocStore {
             }
         }
 
-        match item.content.as_ref() {
+        match &item.content {
             Content::Type(ty) => {
                 if let Some(mut ty) = ty.ty_mut() {
                     // items in ty are all refs, not owned
@@ -851,7 +848,7 @@ impl DocStore {
                 let _ = mem::replace(&mut items[idx], Node::new_gc(item.id, item.len()));
             } else {
                 let mut item = unsafe { item_ref.get_mut_unchecked() };
-                item.content = Arc::new(Content::Deleted(item.len()));
+                item.content = Content::Deleted(item.len());
                 item.flags.clear_countable();
                 debug_assert!(!item.flags.countable());
             }
@@ -1141,14 +1138,7 @@ mod tests {
             store.gc_delete_set().unwrap();
 
             assert_eq!(
-                store
-                    .get_node((1, 0))
-                    .unwrap()
-                    .as_item()
-                    .get()
-                    .unwrap()
-                    .content
-                    .as_ref(),
+                &store.get_node((1, 0)).unwrap().as_item().get().unwrap().content,
                 &Content::Deleted(4)
             );
         });
@@ -1173,14 +1163,7 @@ mod tests {
 
             assert_eq!(arr.len(), 0);
             assert_eq!(
-                store
-                    .get_node((1, 0))
-                    .unwrap()
-                    .as_item()
-                    .get()
-                    .unwrap()
-                    .content
-                    .as_ref(),
+                &store.get_node((1, 0)).unwrap().as_item().get().unwrap().content,
                 &Content::Deleted(1)
             );
 

y-octo/src/doc/types/array.rs

@@ -4,16 +4,16 @@ impl_type!(Array);
 
 impl ListType for Array {}
 
-pub struct ArrayIter(ListIterator);
+pub struct ArrayIter<'a>(ListIterator<'a>);
 
-impl Iterator for ArrayIter {
+impl Iterator for ArrayIter<'_> {
     type Item = Value;
 
     fn next(&mut self) -> Option<Self::Item> {
         for item in self.0.by_ref() {
             if let Some(item) = item.get() {
                 if item.countable() {
-                    return Some(item.content.as_ref().try_into().unwrap());
+                    return Some(Value::try_from(&item.content).unwrap());
                 }
             }
         }
@@ -39,9 +39,9 @@ impl Array {
         debug_assert!(offset == 0);
         if let Some(item) = item.get() {
             // TODO: rewrite to content.read(&mut [Any])
-            return match item.content.as_ref() {
+            return match &item.content {
                 Content::Any(any) => return any.first().map(|any| Value::Any(any.clone())),
-                _ => item.content.as_ref().try_into().map_or_else(|_| None, Some),
+                _ => Value::try_from(&item.content).map_or_else(|_| None, Some),
             };
         }
 
@@ -83,8 +83,6 @@ impl serde::Serialize for Array {
 
 #[cfg(test)]
 mod tests {
-    use yrs::{Array, Options, Text, Transact};
-
     use super::*;
 
     #[test]
@@ -108,6 +106,7 @@ mod tests {
     #[test]
     #[cfg_attr(miri, ignore)]
     fn test_ytext_equal() {
+        use yrs::{Options, Text, Transact};
         let options = DocOptions::default();
         let yrs_options = Options::default();
 
@@ -166,6 +165,7 @@ mod tests {
     #[test]
     #[cfg_attr(miri, ignore)]
     fn test_yrs_array_decode() {
+        use yrs::{Array, Transact};
         let update = {
             let doc = yrs::Doc::new();
             let array = doc.get_or_insert_array("abc");

y-octo/src/doc/types/list/iterator.rs

@@ -1,10 +1,11 @@
 use super::*;
 
-pub(crate) struct ListIterator {
+pub(crate) struct ListIterator<'a> {
+    pub(super) _lock: RwLockReadGuard<'a, YType>,
     pub(super) cur: Somr<Item>,
 }
 
-impl Iterator for ListIterator {
+impl Iterator for ListIterator<'_> {
     type Item = Somr<Item>;
 
     fn next(&mut self) -> Option<Self::Item> {
@@ -20,9 +21,3 @@ impl Iterator for ListIterator {
         None
     }
 }
-
-impl ListIterator {
-    pub fn new(start: Somr<Item>) -> Self {
-        Self { cur: start }
-    }
-}

y-octo/src/doc/types/list/mod.rs

@@ -62,7 +62,10 @@ pub(crate) trait ListType: AsInner<Inner = YTypeRef> {
 
     fn iter_item(&self) -> ListIterator {
         let inner = self.as_inner().ty().unwrap();
-        ListIterator::new(inner.start.clone())
+        ListIterator {
+            cur: inner.start.clone(),
+            _lock: inner,
+        }
     }
 
     fn find_pos(&self, inner: &YType, index: u64) -> Option<ItemPosition> {