diff --git a/.changeset/gorgeous-pets-rhyme.md b/.changeset/gorgeous-pets-rhyme.md new file mode 100644 index 000000000..d4d836269 --- /dev/null +++ b/.changeset/gorgeous-pets-rhyme.md @@ -0,0 +1,6 @@ +--- +"loro-crdt": patch +"loro-crdt-map": patch +--- + +perf: skip useless unmark op #878 diff --git a/Cargo.lock b/Cargo.lock index f0f093007..0853d8e49 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1836,7 +1836,7 @@ checksum = "3f3d053a135388e6b1df14e8af1212af5064746e9b87a06a345a7a779ee9695a" [[package]] name = "loro-wasm" -version = "1.10.1" +version = "1.10.2" dependencies = [ "console_error_panic_hook", "js-sys", diff --git a/crates/loro-internal/src/container/richtext/richtext_state.rs b/crates/loro-internal/src/container/richtext/richtext_state.rs index cac5a9c4c..a3ea92be8 100644 --- a/crates/loro-internal/src/container/richtext/richtext_state.rs +++ b/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, + 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, @@ -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() { diff --git a/crates/loro-internal/src/container/richtext/style_range_map.rs b/crates/loro-internal/src/container/richtext/style_range_map.rs index 8018b16ef..f7487d4db 100644 --- a/crates/loro-internal/src/container/richtext/style_range_map.rs +++ b/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, key: &StyleKey) -> bool { + if range.is_empty() || !self.has_style { + return false; + } + + let mut query = self.tree.query::(&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 diff --git a/crates/loro-internal/src/handler.rs b/crates/loro-internal/src/handler.rs index 7f8e48ae1..63205c279 100644 --- a/crates/loro-internal/src/handler.rs +++ b/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, 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(); diff --git a/crates/loro-internal/src/state/richtext_state.rs b/crates/loro-internal/src/state/richtext_state.rs index b8d43119c..a5c5ed1a7 100644 --- a/crates/loro-internal/src/state/richtext_state.rs +++ b/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, + 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. diff --git a/crates/loro-wasm/tests/richtext.test.ts b/crates/loro-wasm/tests/richtext.test.ts index 2fccce297..04fca0051 100644 --- a/crates/loro-wasm/tests/richtext.test.ts +++ b/crates/loro-wasm/tests/richtext.test.ts @@ -25,6 +25,27 @@ describe("richtext", () => { ] as Delta[]); }); + it("unmark noop when style key missing in span", () => { + const doc = new LoroDoc(); + doc.configTextStyle({ + bold: { expand: "after" }, + italic: { expand: "after" }, + }); + const text = doc.getText("text"); + text.insert(0, "Hello"); + text.mark({ start: 1, end: 4 }, "bold", true); + doc.commit(); + const beforeDelta = text.toDelta(); + const beforeVersion = doc.version().toJSON(); + + // Unmark a key that doesn't exist in the span; should be noop + text.unmark({ start: 0, end: 5 }, "italic"); + doc.commit(); + + expect(text.toDelta()).toStrictEqual(beforeDelta); + expect(doc.version().toJSON()).toStrictEqual(beforeVersion); + }); + it("insert after emoji", () => { const doc = new LoroDoc(); const text = doc.getText("text");