Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions crates/loro-internal/src/container/richtext/richtext_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<usize>,
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(
Expand Down
42 changes: 42 additions & 0 deletions crates/loro-internal/src/container/richtext/style_range_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<usize>,
key: &str,
value: &loro_common::LoroValue,
) -> 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();
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
Expand Down
28 changes: 22 additions & 6 deletions crates/loro-internal/src/handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down Expand Up @@ -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()),
Expand Down
11 changes: 11 additions & 0 deletions crates/loro-internal/src/state/richtext_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<usize>,
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.
Expand Down
63 changes: 63 additions & 0 deletions crates/loro-internal/tests/richtext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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}},
]),
);
}
48 changes: 48 additions & 0 deletions crates/loro/tests/perf_redundant_marks.rs
Original file line number Diff line number Diff line change
@@ -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;
}
}