diff --git a/.changeset/perf-per-op-editing-regression.md b/.changeset/perf-per-op-editing-regression.md new file mode 100644 index 000000000..1364bec5a --- /dev/null +++ b/.changeset/perf-per-op-editing-regression.md @@ -0,0 +1,24 @@ +--- +"loro-crdt": patch +"loro-crdt-map": patch +--- + +Recover two per-operation 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, measured against the 1.11.1 +release. + +1. Every `MapHandler`/`ListHandler`/`MovableListHandler` insert validated its + value with `ensure_no_regular_container_value`, which heap-allocated a `Vec` + on each call even for scalar values (the common case). A scalar fast-path now + skips the allocation and traversal entirely. `map create 10^4 key`: + ~19.4ms -> ~10.7ms. + +2. The per-op text bounds check (`TextHandler::len`/`len_unicode`/`len_utf16`) + took two `DocState` locks — one to check whether the container state was + decoded, then another to query the length. These are now consolidated into a + single `DocState::get_text_len` that takes one lock and one container-store + lookup. The lazy-snapshot memory behavior is preserved: a still-lazy + container reads its cached length metadata without materializing the full + richtext state. `bench_text B4 apply` (per-op text editing): ~389ms -> ~352ms. diff --git a/Cargo.lock b/Cargo.lock index 52b43d90d..07bc3b280 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1873,7 +1873,7 @@ checksum = "3f3d053a135388e6b1df14e8af1212af5064746e9b87a06a345a7a779ee9695a" [[package]] name = "loro-wasm" -version = "1.13.3" +version = "1.13.4" dependencies = [ "console_error_panic_hook", "js-sys", diff --git a/crates/loro-internal/src/handler.rs b/crates/loro-internal/src/handler.rs index 56d4a408c..9d779925a 100644 --- a/crates/loro-internal/src/handler.rs +++ b/crates/loro-internal/src/handler.rs @@ -39,6 +39,18 @@ const REGULAR_CONTAINER_VALUE_ARG_ERROR: &str = mod text_update; fn ensure_no_regular_container_value(value: &LoroValue) -> LoroResult<()> { + // Fast path: scalar values can never transitively hold a container, so we + // skip the heap allocation + traversal below. This is the common case on + // the per-op insert hot path (inserting numbers/strings/bools), where the + // previous unconditional `vec![value]` allocation showed up as a measurable + // regression. + if !matches!( + value, + LoroValue::Container(_) | LoroValue::List(_) | LoroValue::Map(_) + ) { + return Ok(()); + } + let mut stack = vec![value]; while let Some(value) = stack.pop() { match value { @@ -1492,10 +1504,9 @@ impl TextHandler { let t = t.lock(); t.value.len_utf8() } - MaybeDetached::Attached(a) if a.has_decoded_state() => { - a.with_state(|state| state.as_richtext_state_mut().unwrap().len_utf8()) + MaybeDetached::Attached(a) => { + a.with_doc_state(|state| state.get_text_len(a.container_idx, PosType::Bytes)) } - MaybeDetached::Attached(a) => a.get_value().as_string().unwrap().len(), } } @@ -1506,7 +1517,7 @@ impl TextHandler { t.value.len_utf16() } MaybeDetached::Attached(a) => { - a.with_doc_state(|state| state.get_text_utf16_len(a.container_idx)) + a.with_doc_state(|state| state.get_text_len(a.container_idx, PosType::Utf16)) } } } @@ -1518,7 +1529,7 @@ impl TextHandler { t.value.len_unicode() } MaybeDetached::Attached(a) => { - a.with_doc_state(|state| state.get_text_unicode_len(a.container_idx)) + a.with_doc_state(|state| state.get_text_len(a.container_idx, PosType::Unicode)) } } } @@ -1536,25 +1547,9 @@ impl TextHandler { fn len(&self, pos_type: PosType) -> usize { match &self.inner { MaybeDetached::Detached(t) => t.lock().value.len(pos_type), - MaybeDetached::Attached(a) if a.has_decoded_state() || pos_type == PosType::Entity => { - a.with_state(|state| state.as_richtext_state_mut().unwrap().len(pos_type)) + MaybeDetached::Attached(a) => { + a.with_doc_state(|state| state.get_text_len(a.container_idx, pos_type)) } - MaybeDetached::Attached(a) => match pos_type { - PosType::Bytes => a.get_value().as_string().unwrap().len(), - PosType::Unicode => { - a.with_doc_state(|state| state.get_text_unicode_len(a.container_idx)) - } - PosType::Utf16 => { - a.with_doc_state(|state| state.get_text_utf16_len(a.container_idx)) - } - PosType::Event if cfg!(feature = "wasm") => { - a.with_doc_state(|state| state.get_text_utf16_len(a.container_idx)) - } - PosType::Event => { - a.with_doc_state(|state| state.get_text_unicode_len(a.container_idx)) - } - PosType::Entity => unreachable!("entity length is handled by the state path"), - }, } } diff --git a/crates/loro-internal/src/state.rs b/crates/loro-internal/src/state.rs index d0a768038..f78e3782a 100644 --- a/crates/loro-internal/src/state.rs +++ b/crates/loro-internal/src/state.rs @@ -17,7 +17,7 @@ use tracing::{info_span, instrument, warn}; use crate::{ configure::{Configure, DefaultRandom, SecureRandomGenerator}, container::{idx::ContainerIdx, richtext::config::StyleConfigMap}, - cursor::Cursor, + cursor::{Cursor, PosType}, delta::TreeExternalDiff, diff_calc::{DiffCalculator, DiffMode}, event::{Diff, EventTriggerKind, Index, InternalContainerDiff, InternalDiff}, @@ -976,10 +976,37 @@ impl DocState { self.store.text_utf16_len(container_idx).unwrap_or(0) } + pub(crate) fn get_text_utf8_len(&mut self, container_idx: ContainerIdx) -> usize { + self.store.text_utf8_len(container_idx).unwrap_or(0) + } + pub(crate) fn has_decoded_container_state(&mut self, container_idx: ContainerIdx) -> bool { self.store.has_decoded_state(container_idx) } + /// Length of a text container in the given `pos_type`, taking a single + /// `DocState` lock and a single container-store lookup. + /// + /// The per-`pos_type` store helpers already branch on decoded-vs-lazy + /// internally, and their lazy branch reads the cheap length metadata + /// without materializing the full richtext state — preserving the + /// lazy-snapshot memory behavior. Callers previously took two separate + /// locks (one to check decoded-ness, one to query), which showed up as a + /// per-op regression on the text editing hot path. Only `Entity` length has + /// no store helper and falls back to the state path. + pub(crate) fn get_text_len(&mut self, container_idx: ContainerIdx, pos_type: PosType) -> usize { + match pos_type { + PosType::Unicode => self.get_text_unicode_len(container_idx), + PosType::Utf16 => self.get_text_utf16_len(container_idx), + PosType::Event if cfg!(feature = "wasm") => self.get_text_utf16_len(container_idx), + PosType::Event => self.get_text_unicode_len(container_idx), + PosType::Bytes => self.get_text_utf8_len(container_idx), + PosType::Entity => self.with_state_mut(container_idx, |state| { + state.as_richtext_state_mut().unwrap().len(PosType::Entity) + }), + } + } + /// Set the state of the container with the given container idx. /// This is only used for decode. /// diff --git a/crates/loro-internal/src/state/container_store.rs b/crates/loro-internal/src/state/container_store.rs index 445bc1582..73d4d8c81 100644 --- a/crates/loro-internal/src/state/container_store.rs +++ b/crates/loro-internal/src/state/container_store.rs @@ -161,6 +161,11 @@ impl ContainerStore { .with_container_for_read(idx, |c| c.text_utf16_len(idx, ctx!(self)))? } + pub fn text_utf8_len(&mut self, idx: ContainerIdx) -> Option { + self.store + .with_container_for_read(idx, |c| c.text_utf8_len(idx, ctx!(self)))? + } + pub fn has_decoded_state(&mut self, idx: ContainerIdx) -> bool { self.store.has_decoded_state(idx) } diff --git a/crates/loro-internal/src/state/container_store/container_wrapper.rs b/crates/loro-internal/src/state/container_store/container_wrapper.rs index 6c2255fb9..0dc46c55b 100644 --- a/crates/loro-internal/src/state/container_store/container_wrapper.rs +++ b/crates/loro-internal/src/state/container_store/container_wrapper.rs @@ -103,6 +103,15 @@ impl LazyDecodedValue { _ => None, } } + + fn utf8_len(&self) -> Option { + match self { + // The decoded text value is the string itself, so its byte length + // is a cheap `str::len` — no need to cache a separate field. + Self::Text { value, .. } => value.as_string().map(|s| s.len()), + _ => None, + } + } } impl ContainerWrapper { @@ -337,6 +346,25 @@ impl ContainerWrapper { } } + pub fn text_utf8_len( + &mut self, + idx: ContainerIdx, + ctx: ContainerCreationContext, + ) -> Option { + match &mut self.data { + ContainerData::State(state) => Some(state.as_richtext_state_mut().unwrap().len_utf8()), + ContainerData::Lazy(_) => { + self.decode_value(idx, ctx).unwrap(); + match &mut self.data { + ContainerData::State(state) => { + Some(state.as_richtext_state_mut().unwrap().len_utf8()) + } + ContainerData::Lazy(lazy) => lazy.value.as_ref()?.utf8_len(), + } + } + } + } + pub fn list_get( &mut self, idx: ContainerIdx,