Skip to content

Commit 52d8168

Browse files
zxch3nclaude
andauthored
fix: recover two per-op editing slowdowns regressed since 1.11 (#1021)
* fix: recover two per-op editing slowdowns regressed since 1.11 Both are constant-factor regressions on the per-op (auto-commit) editing path introduced by the lazy-snapshot work in #985, found while bisecting current HEAD against the 1.11.1 release. 1. map/list/movable-list insert: ensure_no_regular_container_value heap-allocated a Vec on every insert, even for scalar values (the common case). Add a scalar fast-path that skips the allocation and traversal. `map create 10^4 key`: ~19.4ms -> ~10.7ms. 2. text insert/delete: the per-op bounds-check len()/len_unicode()/ len_utf16() took two DocState locks (decoded-check, then query). Consolidate into one DocState::get_text_len taking a single lock + container-store lookup, preserving the lazy-snapshot memory behavior (a still-lazy container reads cached length metadata without materializing the richtext state). `bench_text B4 apply`: ~389ms -> ~352ms. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore: sync Cargo.lock with loro-wasm 1.13.4 version bump Lockfile update to match the crates/loro-wasm Cargo.toml version bump that landed via the main merge (chore: version packages). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix: read cached byte length for PosType::Bytes text len Addresses a review finding on the per-op text length consolidation: the PosType::Bytes branch of DocState::get_text_len built the full plain string via get_value_by_idx().as_string().len(), so insert_utf8/delete_utf8 bounds checks on an already-decoded text became O(text length) per op (the old code read cached byte-length metadata in the decoded case). Add a text_utf8_len store helper mirroring text_unicode_len/text_utf16_len: - decoded state reads the O(1) cached byte length (RichtextState::len_utf8 = root_cache().bytes) - a still-lazy container reads the byte length from the already-materialized text value string (O(1) str::len), preserving the no-force-decode behavior Also route the public TextHandler::len_utf8 through the same single-lock helper; it had the same has_decoded_state double-lock + string-construction pattern. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 1727258 commit 52d8168

5 files changed

Lines changed: 103 additions & 24 deletions

File tree

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
---
2+
"loro-crdt": patch
3+
"loro-crdt-map": patch
4+
---
5+
6+
Recover two per-operation editing slowdowns regressed since 1.11.
7+
8+
Both are constant-factor regressions on the per-op (auto-commit) editing path
9+
introduced by the lazy-snapshot work in #985, measured against the 1.11.1
10+
release.
11+
12+
1. Every `MapHandler`/`ListHandler`/`MovableListHandler` insert validated its
13+
value with `ensure_no_regular_container_value`, which heap-allocated a `Vec`
14+
on each call even for scalar values (the common case). A scalar fast-path now
15+
skips the allocation and traversal entirely. `map create 10^4 key`:
16+
~19.4ms -> ~10.7ms.
17+
18+
2. The per-op text bounds check (`TextHandler::len`/`len_unicode`/`len_utf16`)
19+
took two `DocState` locks — one to check whether the container state was
20+
decoded, then another to query the length. These are now consolidated into a
21+
single `DocState::get_text_len` that takes one lock and one container-store
22+
lookup. The lazy-snapshot memory behavior is preserved: a still-lazy
23+
container reads its cached length metadata without materializing the full
24+
richtext state. `bench_text B4 apply` (per-op text editing): ~389ms -> ~352ms.

crates/loro-internal/src/handler.rs

Lines changed: 18 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,18 @@ const REGULAR_CONTAINER_VALUE_ARG_ERROR: &str =
3939
mod text_update;
4040

4141
fn ensure_no_regular_container_value(value: &LoroValue) -> LoroResult<()> {
42+
// Fast path: scalar values can never transitively hold a container, so we
43+
// skip the heap allocation + traversal below. This is the common case on
44+
// the per-op insert hot path (inserting numbers/strings/bools), where the
45+
// previous unconditional `vec![value]` allocation showed up as a measurable
46+
// regression.
47+
if !matches!(
48+
value,
49+
LoroValue::Container(_) | LoroValue::List(_) | LoroValue::Map(_)
50+
) {
51+
return Ok(());
52+
}
53+
4254
let mut stack = vec![value];
4355
while let Some(value) = stack.pop() {
4456
match value {
@@ -1492,10 +1504,9 @@ impl TextHandler {
14921504
let t = t.lock();
14931505
t.value.len_utf8()
14941506
}
1495-
MaybeDetached::Attached(a) if a.has_decoded_state() => {
1496-
a.with_state(|state| state.as_richtext_state_mut().unwrap().len_utf8())
1507+
MaybeDetached::Attached(a) => {
1508+
a.with_doc_state(|state| state.get_text_len(a.container_idx, PosType::Bytes))
14971509
}
1498-
MaybeDetached::Attached(a) => a.get_value().as_string().unwrap().len(),
14991510
}
15001511
}
15011512

@@ -1506,7 +1517,7 @@ impl TextHandler {
15061517
t.value.len_utf16()
15071518
}
15081519
MaybeDetached::Attached(a) => {
1509-
a.with_doc_state(|state| state.get_text_utf16_len(a.container_idx))
1520+
a.with_doc_state(|state| state.get_text_len(a.container_idx, PosType::Utf16))
15101521
}
15111522
}
15121523
}
@@ -1518,7 +1529,7 @@ impl TextHandler {
15181529
t.value.len_unicode()
15191530
}
15201531
MaybeDetached::Attached(a) => {
1521-
a.with_doc_state(|state| state.get_text_unicode_len(a.container_idx))
1532+
a.with_doc_state(|state| state.get_text_len(a.container_idx, PosType::Unicode))
15221533
}
15231534
}
15241535
}
@@ -1536,25 +1547,9 @@ impl TextHandler {
15361547
fn len(&self, pos_type: PosType) -> usize {
15371548
match &self.inner {
15381549
MaybeDetached::Detached(t) => t.lock().value.len(pos_type),
1539-
MaybeDetached::Attached(a) if a.has_decoded_state() || pos_type == PosType::Entity => {
1540-
a.with_state(|state| state.as_richtext_state_mut().unwrap().len(pos_type))
1550+
MaybeDetached::Attached(a) => {
1551+
a.with_doc_state(|state| state.get_text_len(a.container_idx, pos_type))
15411552
}
1542-
MaybeDetached::Attached(a) => match pos_type {
1543-
PosType::Bytes => a.get_value().as_string().unwrap().len(),
1544-
PosType::Unicode => {
1545-
a.with_doc_state(|state| state.get_text_unicode_len(a.container_idx))
1546-
}
1547-
PosType::Utf16 => {
1548-
a.with_doc_state(|state| state.get_text_utf16_len(a.container_idx))
1549-
}
1550-
PosType::Event if cfg!(feature = "wasm") => {
1551-
a.with_doc_state(|state| state.get_text_utf16_len(a.container_idx))
1552-
}
1553-
PosType::Event => {
1554-
a.with_doc_state(|state| state.get_text_unicode_len(a.container_idx))
1555-
}
1556-
PosType::Entity => unreachable!("entity length is handled by the state path"),
1557-
},
15581553
}
15591554
}
15601555

crates/loro-internal/src/state.rs

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ use tracing::{info_span, instrument, warn};
1717
use crate::{
1818
configure::{Configure, DefaultRandom, SecureRandomGenerator},
1919
container::{idx::ContainerIdx, richtext::config::StyleConfigMap},
20-
cursor::Cursor,
20+
cursor::{Cursor, PosType},
2121
delta::TreeExternalDiff,
2222
diff_calc::{DiffCalculator, DiffMode},
2323
event::{Diff, EventTriggerKind, Index, InternalContainerDiff, InternalDiff},
@@ -976,10 +976,37 @@ impl DocState {
976976
self.store.text_utf16_len(container_idx).unwrap_or(0)
977977
}
978978

979+
pub(crate) fn get_text_utf8_len(&mut self, container_idx: ContainerIdx) -> usize {
980+
self.store.text_utf8_len(container_idx).unwrap_or(0)
981+
}
982+
979983
pub(crate) fn has_decoded_container_state(&mut self, container_idx: ContainerIdx) -> bool {
980984
self.store.has_decoded_state(container_idx)
981985
}
982986

987+
/// Length of a text container in the given `pos_type`, taking a single
988+
/// `DocState` lock and a single container-store lookup.
989+
///
990+
/// The per-`pos_type` store helpers already branch on decoded-vs-lazy
991+
/// internally, and their lazy branch reads the cheap length metadata
992+
/// without materializing the full richtext state — preserving the
993+
/// lazy-snapshot memory behavior. Callers previously took two separate
994+
/// locks (one to check decoded-ness, one to query), which showed up as a
995+
/// per-op regression on the text editing hot path. Only `Entity` length has
996+
/// no store helper and falls back to the state path.
997+
pub(crate) fn get_text_len(&mut self, container_idx: ContainerIdx, pos_type: PosType) -> usize {
998+
match pos_type {
999+
PosType::Unicode => self.get_text_unicode_len(container_idx),
1000+
PosType::Utf16 => self.get_text_utf16_len(container_idx),
1001+
PosType::Event if cfg!(feature = "wasm") => self.get_text_utf16_len(container_idx),
1002+
PosType::Event => self.get_text_unicode_len(container_idx),
1003+
PosType::Bytes => self.get_text_utf8_len(container_idx),
1004+
PosType::Entity => self.with_state_mut(container_idx, |state| {
1005+
state.as_richtext_state_mut().unwrap().len(PosType::Entity)
1006+
}),
1007+
}
1008+
}
1009+
9831010
/// Set the state of the container with the given container idx.
9841011
/// This is only used for decode.
9851012
///

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,11 @@ impl ContainerStore {
161161
.with_container_for_read(idx, |c| c.text_utf16_len(idx, ctx!(self)))?
162162
}
163163

164+
pub fn text_utf8_len(&mut self, idx: ContainerIdx) -> Option<usize> {
165+
self.store
166+
.with_container_for_read(idx, |c| c.text_utf8_len(idx, ctx!(self)))?
167+
}
168+
164169
pub fn has_decoded_state(&mut self, idx: ContainerIdx) -> bool {
165170
self.store.has_decoded_state(idx)
166171
}

crates/loro-internal/src/state/container_store/container_wrapper.rs

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,15 @@ impl LazyDecodedValue {
103103
_ => None,
104104
}
105105
}
106+
107+
fn utf8_len(&self) -> Option<usize> {
108+
match self {
109+
// The decoded text value is the string itself, so its byte length
110+
// is a cheap `str::len` — no need to cache a separate field.
111+
Self::Text { value, .. } => value.as_string().map(|s| s.len()),
112+
_ => None,
113+
}
114+
}
106115
}
107116

108117
impl ContainerWrapper {
@@ -337,6 +346,25 @@ impl ContainerWrapper {
337346
}
338347
}
339348

349+
pub fn text_utf8_len(
350+
&mut self,
351+
idx: ContainerIdx,
352+
ctx: ContainerCreationContext,
353+
) -> Option<usize> {
354+
match &mut self.data {
355+
ContainerData::State(state) => Some(state.as_richtext_state_mut().unwrap().len_utf8()),
356+
ContainerData::Lazy(_) => {
357+
self.decode_value(idx, ctx).unwrap();
358+
match &mut self.data {
359+
ContainerData::State(state) => {
360+
Some(state.as_richtext_state_mut().unwrap().len_utf8())
361+
}
362+
ContainerData::Lazy(lazy) => lazy.value.as_ref()?.utf8_len(),
363+
}
364+
}
365+
}
366+
}
367+
340368
pub fn list_get(
341369
&mut self,
342370
idx: ContainerIdx,

0 commit comments

Comments
 (0)