diff --git a/crates/loro-internal/src/container/richtext/richtext_state.rs b/crates/loro-internal/src/container/richtext/richtext_state.rs index db9f02f01..1bbf24a30 100644 --- a/crates/loro-internal/src/container/richtext/richtext_state.rs +++ b/crates/loro-internal/src/container/richtext/richtext_state.rs @@ -1477,6 +1477,23 @@ impl RichtextState { result } + /// Whether every position in the entity range already resolves `key` to + /// `value`, in which case a mark with that key and value can be skipped. + pub(crate) fn range_has_style_key_value( + &mut self, + range: Range, + key: &str, + value: &LoroValue, + ) -> bool { + self.check_cache(); + let result = match self.style_ranges.as_ref() { + Some(s) => s.range_has_key_value(range, key, value), + 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( 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 f7487d4db..350fae0ed 100644 --- a/crates/loro-internal/src/container/richtext/style_range_map.rs +++ b/crates/loro-internal/src/container/richtext/style_range_map.rs @@ -219,6 +219,48 @@ impl StyleRangeMap { false } + /// Whether every position in the range already resolves `key` to `value`. + /// + /// Unlike [`Self::get_styles_of_range`], this works when the range spans + /// multiple style ranges, so callers can skip marks that would not change + /// anything (each redundant mark op leaves a pair of style anchors in the + /// state forever). + pub(crate) fn range_has_key_value( + &self, + range: Range, + key: &str, + value: &loro_common::LoroValue, + ) -> 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(); + let remaining_in_elem = elem.len - query.cursor.offset; + if remaining_in_elem > 0 && !elem.styles.has_key_value(key, value) { + return false; + } + + 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, + } + } + + true + } + /// 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 001a36483..4ba41fea7 100644 --- a/crates/loro-internal/src/handler.rs +++ b/crates/loro-internal/src/handler.rs @@ -2340,10 +2340,16 @@ impl TextHandler { } let (entity_range, styles) = 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) { - return Ok(()); - } + // `styles` is None when the range spans multiple style ranges; fall + // back to scanning them so redundant marks are still skipped instead + // of accumulating style anchors. + let already_applied = styles.map(|styles| styles.has_key_value(&key, value)); + let already_applied = match already_applied { + Some(applied) => applied, + None => state.range_has_style_key_value(entity_range.clone(), &key, value), + }; + if already_applied { + return Ok(()); } let has_target_style = @@ -2436,10 +2442,20 @@ impl TextHandler { let (entity_range, styles) = state.get_entity_range_and_styles_at_range(start..end, pos_type); - let skip = styles + // `styles` is None when the range spans multiple style + // ranges; fall back to scanning them so redundant marks are + // still skipped instead of accumulating style anchors. + let skip = match styles .as_ref() .map(|styles| styles.has_key_value(&key, &value)) - .unwrap_or(false); + { + Some(skip) => skip, + None => state.has_style_key_value_in_entity_range( + entity_range.clone(), + &key, + &value, + ), + }; let has_target_style = state.has_style_key_in_entity_range( entity_range.clone(), &StyleKey::Key(key.clone()), diff --git a/crates/loro-internal/src/state/richtext_state.rs b/crates/loro-internal/src/state/richtext_state.rs index 0060669fa..61706f856 100644 --- a/crates/loro-internal/src/state/richtext_state.rs +++ b/crates/loro-internal/src/state/richtext_state.rs @@ -961,6 +961,17 @@ impl RichtextState { self.state.get_mut().range_has_style_key(range, key) } + pub(crate) fn has_style_key_value_in_entity_range( + &mut self, + range: Range, + key: &str, + value: &LoroValue, + ) -> bool { + self.state + .get_mut() + .range_has_style_key_value(range, key, value) + } + /// Check if the content and style ranges are consistent. /// /// Panic if inconsistent. diff --git a/crates/loro-internal/tests/richtext.rs b/crates/loro-internal/tests/richtext.rs index 553c8e594..5347357d6 100644 --- a/crates/loro-internal/tests/richtext.rs +++ b/crates/loro-internal/tests/richtext.rs @@ -361,3 +361,66 @@ fn insert_after_link() { doc_a.check_state_diff_calc_consistency_slow(); doc_b.check_state_diff_calc_consistency_slow(); } + +#[test] +fn redundant_mark_spanning_style_boundaries_creates_no_ops() { + let doc = init("Hello World"); + mark(&doc, 0..5, Kind::Bold); + // This range spans a style boundary, so it genuinely changes 5..11. + mark(&doc, 0..11, Kind::Bold); + doc.commit_then_renew(); + let vv = doc.oplog_vv(); + + // The whole text is already bold; re-asserting the mark must not record + // new ops, even though the range spans multiple style ranges. Every + // redundant mark op leaves a pair of style anchors in the container state + // forever, permanently slowing down styled reads. + mark(&doc, 0..11, Kind::Bold); + doc.commit_then_renew(); + assert_eq!(doc.oplog_vv(), vv); + expect_result( + &doc, + serde_json::json!([ + {"insert": "Hello World", "attributes": {"bold": true}}, + ]), + ); +} + +#[test] +fn redundant_unmark_spanning_style_boundaries_creates_no_ops() { + let doc = init("Hello World"); + mark(&doc, 0..5, Kind::Bold); + mark(&doc, 6..11, Kind::Bold); + unmark(&doc, 0..11, Kind::Bold); + doc.commit_then_renew(); + let vv = doc.oplog_vv(); + + unmark(&doc, 0..11, Kind::Bold); + doc.commit_then_renew(); + assert_eq!(doc.oplog_vv(), vv); + expect_result( + &doc, + serde_json::json!([ + {"insert": "Hello World", "attributes": {"bold": false}}, + ]), + ); +} + +#[test] +fn mark_that_changes_part_of_the_range_still_applies() { + let doc = init("Hello World"); + mark(&doc, 0..5, Kind::Bold); + doc.commit_then_renew(); + let vv = doc.oplog_vv(); + + // 0..5 is already bold but 5..11 is not, so this must not be skipped. + mark(&doc, 0..11, Kind::Bold); + doc.commit_then_renew(); + assert_ne!(doc.oplog_vv(), vv); + expect_result( + &doc, + serde_json::json!([ + {"insert": "Hello World", "attributes": {"bold": true}}, + ]), + ); +} diff --git a/crates/loro/tests/perf_redundant_marks.rs b/crates/loro/tests/perf_redundant_marks.rs new file mode 100644 index 000000000..4bd8434ea --- /dev/null +++ b/crates/loro/tests/perf_redundant_marks.rs @@ -0,0 +1,48 @@ +use loro::LoroDoc; +use std::time::Instant; + +/// Regression guard for style-anchor accumulation from redundant marks. +/// +/// The skip-redundant-marks check in `mark_with_txn` used to work only when +/// the marked range fell inside a single style-range leaf, so on any styled +/// document a caller re-asserting a mark that changes nothing (e.g. an editor +/// binding syncing mark state) recorded a new op every time. Each of those +/// ops leaves a pair of style anchors in the container state forever — they +/// survive snapshots and are never consolidated — and every styled read pays +/// for all of them, so reads degraded without bound. After the fix redundant +/// marks are skipped, so read time should stay flat as `n` grows. +/// +/// Run with: +/// cargo test -p loro perf_redundant_marks_do_not_degrade_styled_reads -- --ignored --nocapture +#[test] +#[ignore] +fn perf_redundant_marks_do_not_degrade_styled_reads() { + fn bench(n: usize) -> std::time::Duration { + let doc = LoroDoc::new(); + let text = doc.get_text("text"); + text.insert(0, &"x".repeat(724)).unwrap(); + // Fragment the style ranges so the redundant marks below take the + // spans-multiple-leaves path. + text.mark(0..100, "bold", true).unwrap(); + text.mark(0..724, "bold", true).unwrap(); + for _ in 0..n { + text.mark(0..724, "bold", true).unwrap(); + } + doc.commit(); + + let start = Instant::now(); + for _ in 0..100 { + std::hint::black_box(text.get_richtext_value()); + } + start.elapsed() / 100 + } + + let mut prev = 0f64; + for &n in &[0usize, 1000, 2000, 4000] { + let d = bench(n); + let us = d.as_secs_f64() * 1e6; + let ratio = if prev > 0.0 { us / prev } else { 0.0 }; + println!("redundant_marks={n:>5} read={us:>9.2} us x_vs_prev={ratio:.2}"); + prev = us; + } +}