perf: coalesce contiguous leaf updates during tracker checkout - #1082
Open
rexikan wants to merge 1 commit into
Open
perf: coalesce contiguous leaf updates during tracker checkout#1082rexikan wants to merge 1 commit into
rexikan wants to merge 1 commit into
Conversation
A single large text op is stored in IdToCursor as one fragment per MAX_FRAGMENT_LEN (256) ids. A checkout across such an op therefore emits one LeafUpdate per fragment, all targeting the same rope leaf with the same status change. CrdtRope::update then split the leaf at every fragment boundary, applied the identical update to each part, and insert_by_path merged every part straight back into the previous leaf - returning the same leaf index once per fragment. The caller re-mapped each of those to the full op span in IdToCursor, making the checkout O((len/256)^2): a 1.2M-char paste took ~3-12s to checkout across, while the insert itself took ~5ms. Merging adjacent same-effect, contiguous-span updates before building the split points removes both the split/re-merge churn and the quadratic re-mapping: the same checkout now takes ~3ms. Adds an ignored perf regression test in the style of the existing perf_update_insert_by_split_quadratic tests, with a deliberately loose 2s bound that a regression to quadratic always trips.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Checkout across a single large text op is O((len/256)^2). A 1.2M-char paste (one op) takes ~3s to
checkoutaway from and ~12s to checkout back across (native, M-series; the wasm build shows 12-26s forcheckout/diff/forkAtacross the same op), while the insert itself takes ~5ms. This PR makes all of these single-digit milliseconds by coalescing contiguous same-effectLeafUpdates inCrdtRope::update.The user-facing shape of this is any app that shows document history. A note that starts life as a large import - a book, a long paste, a migrated document - has that import as its first op forever. Letting the user click through history versions means checking out or diffing across that op, and today the very first click freezes the UI for 10+ seconds. With this fix, time travel over such documents is instant at every size we tested (linear to 12M chars).
Cause
A single op of length L is stored in
IdToCursoras L/MAX_FRAGMENT_LEN(256) fragments.Tracker::_checkoutacross the op therefore emits oneLeafUpdateper fragment — all targeting the same rope leaf with the same status change.CrdtRope::updatesplits the leaf at every fragment boundary, applies the identical update to each part, andupdate_leaves_with_arg_in_ranges'sinsert_by_pathimmediately merges every part back into the previous leaf (contiguous, identical status ⇒can_merge), returning the same leaf index once per fragment, each reporting the full op id-span.Tracker::update_insert_by_splitthen re-maps the whole op span inIdToCursoronce per fragment: ~22M inner iterations for 1.2M chars, ~98% of the checkout's profile samples.diff(),forkAt(), re-attaching after a backwards checkout, and import-with-concurrency all funnel through the sameTracker::_checkout, so they all hit it, and one fix covers them all.Fix
Coalesce adjacent updates (after the existing stable sort by leaf) when they target the same leaf with the same
set_future/delete_times_diffand have contiguous forward id spans. The per-fragment updates collapse into one; no split/re-merge churn happens at all. Adebug_assertpins the (verified) invariant thatLeafUpdatespans are never reversed, and the merged bound is written via the normalizedctr_end().Measured (release, M-series):
diff(empty, latest)/diff(latest, empty)fork_atmid-op / at emptyAlso adds an
#[ignore]d regression test in the style of the existingperf_*tests (LORO_PERF_CHARS-scalable, generous 2s bound that the pre-fix code trips even at 1/6 the default size).Validation
cargo test -p loro-internal --features test_utils,jsonpath --lib(409 passed), full-p lorosuites,-p fuzz(all recorded cases +random_fuzz).fork_at/snapshot_atrecords — byte-identical results before/after, while the coalescing demonstrably fired millions of times. Same for a giant-op shadow-model fuzzer (600 seeds) runningTracker::check()invariants in debug mode.[a,b)+[b,c)into[a,c)for one leaf (= oneFugueSpan) only removes an interior split point; every position receives the identicalset_futureassignment /delete_times_diffsum exactly once either way, and overlapping spans (intentional double-deletes) are never merged because the guard requires strict contiguity.Two observations from the review that may interest you (not addressed here):
check_id_to_cursor_insertions_correctnesscomputesspan.contains(...)twice and discards both results (missingassert!).Disclosure
This fix was produced with substantial AI assistance (Claude): profiling, root-cause analysis, the patch, and an extensive adversarial review (7 independent agent reviews including differential testing against the parent commit) were AI-driven, directed and reviewed by me.