perf: skip useless unmark op

.changeset/gorgeous-pets-rhyme.md

@@ -0,0 +1,6 @@
+---
+"loro-crdt": patch
+"loro-crdt-map": patch
+---
+
+perf: skip useless unmark op #878

crates/loro-internal/src/container/richtext/richtext_state.rs

@@ -31,7 +31,7 @@ use self::query::{
 
 use super::{
     style_range_map::{IterAnchorItem, StyleRangeMap, Styles},
-    AnchorType, RichtextSpan, StyleOp,
+    AnchorType, RichtextSpan, StyleKey, StyleOp,
 };
 
 pub(crate) use crate::cursor::PosType;
@@ -1295,13 +1295,29 @@ impl RichtextState {
         result
     }
 
-    fn has_styles(&self) -> bool {
+    pub(crate) fn has_styles(&self) -> bool {
         self.style_ranges
             .as_ref()
             .map(|x| x.has_style())
             .unwrap_or(false)
     }
 
+    pub(crate) fn range_has_style_key(
+        &mut self,
+        range: Range<usize>,
+        key: &StyleKey,
+    ) -> bool {
+        self.check_cache();
+        let result = match self.style_ranges.as_ref() {
+            Some(s) => s.range_contains_key(range, key),
+            None => false,
+        };
+        self.check_cache();
+        result
+    }
+
+    /// Return the entity range and text styles at the given range.
+    /// If in the target range the leaves are not in the same span, the returned styles would be None
     pub(crate) fn get_entity_range_and_text_styles_at_range(
         &mut self,
         range: Range<usize>,
@@ -1960,16 +1976,15 @@ impl RichtextState {
                 match &self.style_ranges {
                     Some(s) => {
                         let mut idx = current_entity_index;
-                        Box::new(
-                            s.iter_range(current_entity_index..end_entity_index)
-                                .map(move |elem_slice| {
-                                    let len = elem_slice.end.unwrap_or(elem_slice.elem.len)
-                                        - elem_slice.start.unwrap_or(0);
-                                    let range = idx..idx + len;
-                                    idx += len;
-                                    (range, &elem_slice.elem.styles)
-                                }),
-                        )
+                        Box::new(s.iter_range(current_entity_index..end_entity_index).map(
+                            move |elem_slice| {
+                                let len = elem_slice.end.unwrap_or(elem_slice.elem.len)
+                                    - elem_slice.start.unwrap_or(0);
+                                let range = idx..idx + len;
+                                idx += len;
+                                (range, &elem_slice.elem.styles)
+                            },
+                        ))
                     }
                     None => Box::new(Some((0..usize::MAX / 2, &*EMPTY_STYLES)).into_iter()),
                 };
@@ -1984,8 +1999,8 @@ impl RichtextState {
             for span in self.tree.iter_range(start_cursor..end_cursor) {
                 match &span.elem {
                     RichtextStateChunk::Text(t) => {
-                        let chunk_len = span.end.unwrap_or(span.elem.rle_len())
-                            - span.start.unwrap_or(0); // length in rle_len (unicode_len)
+                        let chunk_len =
+                            span.end.unwrap_or(span.elem.rle_len()) - span.start.unwrap_or(0); // length in rle_len (unicode_len)
                         let mut processed_len = 0;
 
                         while processed_len < chunk_len {
@@ -2014,13 +2029,12 @@ impl RichtextState {
                             let slice_start = span.start.unwrap_or(0) + processed_len;
                             let slice_end = slice_start + take_len;
 
-                            let text_content =
-                                unicode_slice(t.as_str(), slice_start, slice_end)
-                                    .map_err(|_| LoroError::OutOfBound {
-                                        pos: slice_end,
-                                        len: t.unicode_len() as usize,
-                                        info: "Slice delta out of bound".into(),
-                                    })?;
+                            let text_content = unicode_slice(t.as_str(), slice_start, slice_end)
+                                .map_err(|_| LoroError::OutOfBound {
+                                    pos: slice_end,
+                                    len: t.unicode_len() as usize,
+                                    info: "Slice delta out of bound".into(),
+                                })?;
 
                             let styles = cur_styles.as_ref().unwrap();
                             if let Some(last) = ans.last_mut() {

crates/loro-internal/src/container/richtext/style_range_map.rs

@@ -188,6 +188,37 @@ impl StyleRangeMap {
         }
     }
 
+    pub(crate) fn range_contains_key(&self, range: Range<usize>, key: &StyleKey) -> bool {
+        if range.is_empty() || !self.has_style {
+            return false;
+        }
+
+        let mut query = self.tree.query::<LengthFinder>(&range.start).unwrap();
+        let mut pos = range.start;
+        loop {
+            let elem = self.tree.get_elem(query.cursor.leaf).unwrap();
+            if elem.styles.contains_key(key) {
+                return true;
+            }
+
+            let remaining_in_elem = elem.len - query.cursor.offset;
+            let next_pos = pos + remaining_in_elem;
+            if next_pos >= range.end {
+                break;
+            }
+
+            match self.tree.next_elem(query.cursor) {
+                Some(next_cursor) => {
+                    pos = next_pos;
+                    query.cursor = next_cursor;
+                }
+                None => break,
+            }
+        }
+
+        false
+    }
+
     /// Insert entities at `pos` with length of `len`
     ///
     /// # Internal

crates/loro-internal/src/handler.rs

@@ -4,7 +4,7 @@ use crate::{
     container::{
         idx::ContainerIdx,
         list::list_op::{DeleteSpan, DeleteSpanWithId, ListOp},
-        richtext::{richtext_state::PosType, RichtextState, StyleOp, TextStyleInfoFlag},
+        richtext::{richtext_state::PosType, RichtextState, StyleKey, StyleOp, TextStyleInfoFlag},
     },
     cursor::{Cursor, Side},
     delta::{DeltaItem, Meta, StyleMeta, TreeExternalDiff},
@@ -1968,10 +1968,10 @@ impl TextHandler {
         match &self.inner {
             MaybeDetached::Detached(t) => {
                 let mut g = t.lock().unwrap();
-                self.mark_for_detached(&mut g.value, key, &value, start, end, pos_type, false)
+                self.mark_for_detached(&mut g.value, key, &value, start, end, pos_type)
             }
             MaybeDetached::Attached(a) => {
-                a.with_txn(|txn| self.mark_with_txn(txn, start, end, key, value, pos_type, false))
+                a.with_txn(|txn| self.mark_with_txn(txn, start, end, key, value, pos_type))
             }
         }
     }
@@ -1984,9 +1984,9 @@ impl TextHandler {
         start: usize,
         end: usize,
         pos_type: PosType,
-        is_delete: bool,
     ) -> Result<(), LoroError> {
         let key: InternalString = key.into();
+        let is_delete = matches!(value, &LoroValue::Null);
         if start >= end {
             return Err(loro_common::LoroError::ArgErr(
                 "Start must be less than end".to_string().into_boxed_str(),
@@ -2005,11 +2005,18 @@ impl TextHandler {
             state.get_entity_range_and_text_styles_at_range(start..end, pos_type);
         if let Some(styles) = styles {
             if styles.has_key_value(&key, value) {
-                // already has the same style, skip
                 return Ok(());
             }
         }
 
+        let has_target_style =
+            state.range_has_style_key(entity_range.clone(), &StyleKey::Key(key.clone()));
+        let missing_style_key = is_delete && !has_target_style;
+
+        if missing_style_key {
+            return Ok(());
+        }
+
         let style_op = Arc::new(StyleOp {
             lamport: 0,
             peer: 0,
@@ -2043,10 +2050,9 @@ impl TextHandler {
                 start,
                 end,
                 pos_type,
-                true,
             ),
             MaybeDetached::Attached(a) => a.with_txn(|txn| {
-                self.mark_with_txn(txn, start, end, key, LoroValue::Null, pos_type, true)
+                self.mark_with_txn(txn, start, end, key, LoroValue::Null, pos_type)
             }),
         }
     }
@@ -2060,7 +2066,6 @@ impl TextHandler {
         key: impl Into<InternalString>,
         value: LoroValue,
         pos_type: PosType,
-        is_delete: bool,
     ) -> LoroResult<()> {
         if start >= end {
             return Err(loro_common::LoroError::ArgErr(
@@ -2070,6 +2075,7 @@ impl TextHandler {
 
         let inner = self.inner.try_attached_state()?;
         let key: InternalString = key.into();
+        let is_delete = matches!(&value, &LoroValue::Null);
 
         let mut doc_state = inner.doc.state.lock().unwrap();
         let len = doc_state.with_state_mut(inner.container_idx, |state| {
@@ -2084,26 +2090,34 @@ impl TextHandler {
             });
         }
 
-        let (entity_range, skip, event_start, event_end) =
-            doc_state.with_state_mut(inner.container_idx, |state| {
+        let (entity_range, skip, missing_style_key, event_start, event_end) = doc_state
+            .with_state_mut(inner.container_idx, |state| {
                 let state = state.as_richtext_state_mut().unwrap();
                 let event_start = state.index_to_event_index(start, pos_type);
                 let event_end = state.index_to_event_index(end, pos_type);
                 let (entity_range, styles) =
                     state.get_entity_range_and_styles_at_range(start..end, pos_type);
 
-                let skip = match styles {
-                    Some(styles) if styles.has_key_value(&key, &value) => {
-                        // already has the same style, skip
-                        true
-                    }
-                    _ => false,
-                };
-
-                (entity_range, skip, event_start, event_end)
+                let skip = styles
+                    .as_ref()
+                    .map(|styles| styles.has_key_value(&key, &value))
+                    .unwrap_or(false);
+                let has_target_style = state.has_style_key_in_entity_range(
+                    entity_range.clone(),
+                    &StyleKey::Key(key.clone()),
+                );
+                let missing_style_key = is_delete && !has_target_style;
+
+                (
+                    entity_range,
+                    skip,
+                    missing_style_key,
+                    event_start,
+                    event_end,
+                )
             });
 
-        if skip {
+        if skip || missing_style_key {
             return Ok(());
         }
 
@@ -2269,7 +2283,6 @@ impl TextHandler {
                     key.deref(),
                     value,
                     PosType::Event,
-                    false,
                 )?;
             }
         }
@@ -4449,7 +4462,7 @@ mod test {
             .insert_with_txn(&mut txn, 0, "hello world", PosType::Unicode)
             .unwrap();
         handler
-            .mark_with_txn(&mut txn, 0, 5, "bold", true.into(), PosType::Event, false)
+            .mark_with_txn(&mut txn, 0, 5, "bold", true.into(), PosType::Event)
             .unwrap();
         txn.commit().unwrap();
 

crates/loro-internal/src/state/richtext_state.rs

@@ -15,7 +15,7 @@ use crate::{
             richtext_state::{
                 DrainInfo, EntityRangeInfo, IterRangeItem, PosType, RichtextStateChunk,
             },
-            AnchorType, RichtextState as InnerState, StyleOp, Styles,
+            AnchorType, RichtextState as InnerState, StyleKey, StyleOp, Styles,
         },
     },
     delta::{StyleMeta, StyleMetaItem},
@@ -283,6 +283,74 @@ impl RichtextState {
     }
 }
 
+#[cfg(test)]
+mod tests {
+    use crate::{container::richtext::StyleKey, cursor::PosType, handler::HandlerTrait, LoroDoc};
+
+    #[test]
+    fn has_style_key_in_entity_range_basic() {
+        let loro = LoroDoc::new_auto_commit();
+        let text = loro.get_text("text");
+        text.insert(0, "abcdef", PosType::Unicode).unwrap();
+        text.mark(1, 3, "bold", true.into(), PosType::Unicode)
+            .unwrap();
+
+        let bold_key = StyleKey::Key("bold".into());
+        let has_style = text
+            .with_state(|state| {
+                let st = state.as_richtext_state_mut().unwrap();
+                let (entity_range, _) =
+                    st.get_entity_range_and_styles_at_range(1..3, PosType::Unicode);
+                Ok(st.has_style_key_in_entity_range(entity_range, &bold_key))
+            })
+            .unwrap();
+        assert!(has_style);
+
+        let missing = text
+            .with_state(|state| {
+                let st = state.as_richtext_state_mut().unwrap();
+                let (entity_range, _) =
+                    st.get_entity_range_and_styles_at_range(4..5, PosType::Unicode);
+                Ok(st.has_style_key_in_entity_range(entity_range, &bold_key))
+            })
+            .unwrap();
+        assert!(!missing);
+    }
+
+    #[test]
+    fn has_style_key_in_entity_range_spans_elements() {
+        let loro = LoroDoc::new_auto_commit();
+        let text = loro.get_text("text");
+        text.insert(0, "abcdefgh", PosType::Unicode).unwrap();
+        text.mark(0, 2, "bold", true.into(), PosType::Unicode)
+            .unwrap();
+        text.mark(3, 5, "bold", true.into(), PosType::Unicode)
+            .unwrap();
+
+        let bold_key = StyleKey::Key("bold".into());
+
+        let has_style_across_segments = text
+            .with_state(|state| {
+                let st = state.as_richtext_state_mut().unwrap();
+                let (entity_range, _) =
+                    st.get_entity_range_and_styles_at_range(0..5, PosType::Unicode);
+                Ok(st.has_style_key_in_entity_range(entity_range, &bold_key))
+            })
+            .unwrap();
+        assert!(has_style_across_segments);
+
+        let gap_has_style = text
+            .with_state(|state| {
+                let st = state.as_richtext_state_mut().unwrap();
+                let (entity_range, _) =
+                    st.get_entity_range_and_styles_at_range(6..7, PosType::Unicode);
+                Ok(st.has_style_key_in_entity_range(entity_range, &bold_key))
+            })
+            .unwrap();
+        assert!(!gap_has_style);
+    }
+}
+
 impl Clone for RichtextState {
     fn clone(&self) -> Self {
         Self {
@@ -725,6 +793,19 @@ impl RichtextState {
         }
     }
 
+    #[inline]
+    pub(crate) fn has_styles(&mut self) -> bool {
+        self.state.get_mut().has_styles()
+    }
+
+    pub(crate) fn has_style_key_in_entity_range(
+        &mut self,
+        range: Range<usize>,
+        key: &StyleKey,
+    ) -> bool {
+        self.state.get_mut().range_has_style_key(range, key)
+    }
+
     /// Check if the content and style ranges are consistent.
     ///
     /// Panic if inconsistent.