diff --git a/.changeset/tree-checkout-window.md b/.changeset/tree-checkout-window.md new file mode 100644 index 000000000..bbf0a4b9c --- /dev/null +++ b/.changeset/tree-checkout-window.md @@ -0,0 +1,5 @@ +--- +"loro-crdt": patch +--- + +Fix tree checkouts that depended on the path taken. Checking out from a version to a concurrent one could keep a tree node the target version does not have, or, with a subscriber, panic with `get_index_by_tree_id(..).unwrap()` in `tree_state.rs` when the diff created a node's child without the node. The tree diff now retreats and replays from the smallest lamport that differs between the two versions instead of from a replay base, which could be a non-critical meet that skipped earlier concurrent ops. Tree checkouts also no longer search the DAG for a replay base. diff --git a/AGENTS.md b/AGENTS.md index d1c2488f8..4051322d7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -29,6 +29,8 @@ Loro is a Rust CRDT workspace with JS/WASM packaging and a MoonBit codec. [context/shallow-snapshot-style-redaction.md](context/shallow-snapshot-style-redaction.md). - Rich-text insert positions next to style anchors, the insert cursor cache, and delete `start_id`s: [context/richtext-insert-positions.md](context/richtext-insert-positions.md). +- Tree diff cache transitions and why their lamport window comes from the two + versions, not the replay base: [context/tree-checkout-window.md](context/tree-checkout-window.md). - User-facing Loro usage, sync, editor integration, and performance guidance: [skills/loro/SKILL.md](skills/loro/SKILL.md). - Pure TypeScript runtime indexes, complexity contracts, benchmarks, and remaining gaps: diff --git a/context/tree-checkout-window.md b/context/tree-checkout-window.md new file mode 100644 index 000000000..b117d54f1 --- /dev/null +++ b/context/tree-checkout-window.md @@ -0,0 +1,79 @@ +# Tree Checkout Window + +Verified against code 2026-09-28. + +`TreeDiffCalculator` (`crates/loro-internal/src/diff_calc/tree.rs`) computes +tree diffs for checkouts and for imports in `DiffMode::Import`. It keeps one +`TreeCacheForDiff` per tree in the history cache (`TreeOpGroup` in +`history_cache.rs`): the tree's move ops of `current_vv`, grouped by target. + +## Invariant + +Each cached op carries `effected`: whether the cycle check let it apply +(`TreeCacheForDiff::apply`). That flag depends on every op ordered before it +by (lamport, peer), so the cache must always hold ops applied in that order. A +node's parent is its last effected op (`get_parent_with_id`). + +## Transitions + +`checkout` (moves the cache to `from` without diffs) and `checkout_diff` +(records `TreeDeltaItem`s from `from` to `to`) use the same window: + +1. `min_lamport_of_version_diff(a, b)`: the smallest lamport among ops that are + in exactly one of the two versions. Lamports grow along a peer's ops, so it + reads only the first op of each per-peer span of the version-vector + difference, O(peers). +2. Retreat every cached op at or above it, newest first. `checkout_diff` emits + the inverse of each effected op. +3. Apply every op of the target version at or above it, oldest first. + +Ops below the window are in both versions and stay as they are. Both passes +walk `TreeOpGroup::ops` (ordered by `IdLp`), which also holds the seeded +shallow-root entries, so they never scan nodes that did not change. + +The retreat pass ends at `TreeCacheForDiff::max_lamport`: no cached op has a +greater lamport (`apply` and the shallow-root seeding raise it, +`retreated_from` lowers it to just below the window). The group holds the +whole history, so without that bound a checkout between two old versions +walked every later tree op and grew linearly with the history (about 3.8 ms +per checkout at 300k later moves). The forward pass ends at the target's max +lamport. + +## Why not the replay base + +Until 2026-09-28 the window started at the change-start lamport of +`find_replay_base(from, to)`. In `Checkout` mode that base is the meet even +when the meet is not a critical version (`MeetAsBase::Valid`), and an op +concurrent with the meet can have a lower lamport than the meet. Example +(`crates/loro/tests/tree_checkout_path.rs`): `0@3` creates a node at lamport 1 +and is concurrent with `2@2` (lamport 2); checking out from `[3@2]` to +`[1@1]` used the meet `[2@2]`, the window started at 2, and the creation was +never retreated. In the other direction the forward pass skipped a node's +creation but applied its child's, and `TreeState::apply_diff_and_convert` +panicked on the child's `Create` (with a subscriber; without one the state was +wrong). `docs/critical-version-spec.md` Q7 had argued the window was safe. + +The cache's own `checkout` had the same problem with a window taken from the +target frontiers. + +## Shallow documents + +The cache of a shallow document starts at the shallow root, seeded with the +root's nodes (`record_shallow_root_state`). `TreeCacheForDiff::init_current_vv` +sets `current_vv` to the root version before the first transition, so the +window never reaches trimmed ops. The seeding itself only covers the seeded op +IDs, which is why `current_vv` has to be replaced. + +## Tests + +- `crates/loro/tests/tree_checkout_path.rs`: the two repros. +- `retreat_bound_follows_the_cached_version` (unit test in `tree.rs`) and the + ignored `crates/loro/tests/perf_tree_checkout_old_versions.rs` (run with + `--release -- --ignored`): the retreat bound. +- `crates/fuzz/tests/checkout_path.rs`: random concurrent edits on all + container kinds, every ordered pair of versions (including heads no peer + had), compared with a document that imported only that version's changes, + with and without a subscriber (events are mirrored). Wider sweeps: + `CHECKOUT_PATH_SEEDS=0..300 cargo test --release -p fuzz --test checkout_path`. +- `crates/loro/tests/issue.rs`: `checkout_across_non_critical_meet_stays_canonical` + and `checkout_with_low_lamport_concurrent_branch_stays_canonical`. diff --git a/crates/fuzz/tests/checkout_path.rs b/crates/fuzz/tests/checkout_path.rs new file mode 100644 index 000000000..bdec8de2b --- /dev/null +++ b/crates/fuzz/tests/checkout_path.rs @@ -0,0 +1,242 @@ +//! Checkouts must not depend on the path taken. +//! +//! Several peers make concurrent edits (tree create/move/delete, moves into +//! and out of deleted subtrees, metadata edits, plus map/list/movable +//! list/text/counter edits) and sync at random. Then one document walks +//! through every ordered pair `(a, b)` of the collected versions: +//! `checkout(a)`, then `checkout(b)`. After each checkout its value must equal +//! a fresh document that imported only the changes of that version, and the +//! value rebuilt from its events must equal its state. The versions include +//! heads that no peer ever had, so the transitions cross concurrent branches. +//! +//! The seeds are fixed; `CHECKOUT_PATH_SEEDS=start..end` runs others. The +//! `loro.js` differential fuzz found the tree cases (loro-dev/loro#1141): the +//! other fuzzers only check out versions from one peer's own history, which +//! are ordered by inclusion. See `context/tree-checkout-window.md`. + +use fuzz::actor::{assert_value_eq, Actor}; +use loro::{ + ExportMode, Frontiers, LoroDoc, LoroMap, LoroText, LoroTree, LoroValue, TreeID, TreeParentId, + ID, +}; +use rand::{rngs::StdRng, seq::SliceRandom, Rng, SeedableRng}; + +const PEERS: u64 = 3; +const ACTIONS: usize = 40; +const MAX_VERSIONS: usize = 24; + +fn random_parent(tree: &LoroTree, rng: &mut StdRng) -> TreeParentId { + let nodes = tree.nodes(); + if nodes.is_empty() || rng.gen_bool(0.3) { + TreeParentId::Root + } else { + TreeParentId::Node(*nodes.choose(rng).unwrap()) + } +} + +fn random_node(tree: &LoroTree, rng: &mut StdRng) -> Option { + tree.nodes().choose(rng).copied() +} + +fn random_edit(doc: &LoroDoc, rng: &mut StdRng) { + let tree = doc.get_tree("tree"); + let value = rng.gen_range(0..100); + // Errors are expected: moving a node below itself, editing a deleted + // node's metadata, out-of-range indexes after a sync, and so on. + let _ = match rng.gen_range(0..16) { + 0..=2 => { + let parent = random_parent(&tree, rng); + tree.create(parent).map(|_| ()) + } + 3..=5 => match random_node(&tree, rng) { + // Includes moves into deleted subtrees and moves that revive + // nodes under deleted ancestors. + Some(target) => { + let parent = random_parent(&tree, rng); + tree.mov(target, parent) + } + None => Ok(()), + }, + 6 => match random_node(&tree, rng) { + Some(target) => tree.delete(target), + None => Ok(()), + }, + 7..=8 => match random_node(&tree, rng) { + Some(target) => tree + .get_meta(target) + .and_then(|meta| meta.insert(&format!("k{}", value % 3), value)), + None => Ok(()), + }, + 9 => match random_node(&tree, rng) { + Some(target) => tree.get_meta(target).and_then(|meta| { + meta.insert_container("text", LoroText::new())? + .insert(0, "t") + }), + None => Ok(()), + }, + 10 => { + let map = doc.get_map("map"); + if rng.gen_bool(0.3) { + map.delete(&format!("k{}", value % 3)) + } else if rng.gen_bool(0.3) { + map.insert_container(&format!("k{}", value % 3), LoroMap::new()) + .and_then(|m| m.insert("v", value)) + } else { + map.insert(&format!("k{}", value % 3), value) + } + } + 11 => { + let list = doc.get_list("list"); + if !list.is_empty() && rng.gen_bool(0.4) { + list.delete(rng.gen_range(0..list.len()), 1) + } else { + list.insert(rng.gen_range(0..=list.len()), value) + } + } + 12 => { + let list = doc.get_movable_list("movable"); + let len = list.len(); + match rng.gen_range(0..4) { + 0 if len > 1 => list.mov(rng.gen_range(0..len), rng.gen_range(0..len)), + 1 if len > 0 => list.delete(rng.gen_range(0..len), 1), + 2 if len > 0 => list.set(rng.gen_range(0..len), value), + 3 => list + .insert_container(rng.gen_range(0..=len), LoroMap::new()) + .and_then(|m| m.insert("v", value)), + _ => list.insert(rng.gen_range(0..=len), value), + } + } + 13 => { + let text = doc.get_text("text"); + let len = text.len_unicode(); + if len > 0 && rng.gen_bool(0.3) { + let pos = rng.gen_range(0..len); + text.delete(pos, 1) + } else { + text.insert(rng.gen_range(0..=len), "ab") + } + } + 14 => doc.get_counter("counter").increment(value as f64), + _ => { + // A burst of local edits gives this peer higher lamports than the + // others, so concurrent branches end up with very different + // lamport ranges. + (0..3).try_for_each(|_| { + doc.get_map("burst").insert("k", value)?; + doc.commit(); + Ok(()) + }) + } + }; + doc.commit(); +} + +fn random_version(doc: &LoroDoc, rng: &mut StdRng) -> Frontiers { + let vv = doc.oplog_vv(); + let mut ids = Vec::new(); + for _ in 0..rng.gen_range(1..=2) { + let peers: Vec<_> = vv.iter().filter(|(_, end)| **end > 0).collect(); + let Some((peer, end)) = peers.choose(rng) else { + break; + }; + ids.push(ID::new(**peer, rng.gen_range(0..**end))); + } + let vv = doc.frontiers_to_vv(&Frontiers::from(ids)).unwrap(); + doc.vv_to_frontiers(&vv) +} + +fn value_at(all: &LoroDoc, version: &Frontiers) -> LoroValue { + let vv = all.frontiers_to_vv(version).unwrap(); + let doc = LoroDoc::new(); + doc.set_hide_empty_root_containers(true); + doc.import(&all.export(ExportMode::updates_till(&vv)).unwrap()) + .unwrap(); + doc.get_deep_value() +} + +fn check_checkout(doc: &LoroDoc, version: &Frontiers, expected: &LoroValue, context: &str) { + doc.checkout(version).unwrap(); + assert_value_eq( + expected, + &doc.get_deep_value(), + Some(&mut || format!("{context}, checkout to {version:?}")), + ); +} + +fn run(seed: u64) { + let mut rng = StdRng::seed_from_u64(seed); + let peers: Vec = (1..=PEERS) + .map(|peer| { + let doc = LoroDoc::new(); + doc.set_peer_id(peer).unwrap(); + doc + }) + .collect(); + for _ in 0..ACTIONS { + let i = rng.gen_range(0..peers.len()); + if rng.gen_bool(0.25) { + let j = rng.gen_range(0..peers.len()); + if i != j { + let update = peers[j] + .export(ExportMode::updates(&peers[i].oplog_vv())) + .unwrap(); + peers[i].import(&update).unwrap(); + } + } else { + random_edit(&peers[i], &mut rng); + } + } + + let all = LoroDoc::new(); + for peer in peers.iter() { + all.import(&peer.export(ExportMode::all_updates()).unwrap()) + .unwrap(); + } + let mut versions = vec![Frontiers::default(), all.oplog_frontiers()]; + while versions.len() < MAX_VERSIONS { + let version = random_version(&all, &mut rng); + if !versions.contains(&version) { + versions.push(version); + } + } + let expected: Vec = versions.iter().map(|v| value_at(&all, v)).collect(); + + // `Actor` mirrors the document from its events. + let with_events = Actor::new(100); + let plain = LoroDoc::new(); + let updates = all.export(ExportMode::all_updates()).unwrap(); + for doc in [with_events.loro.as_ref(), &plain] { + doc.set_hide_empty_root_containers(true); + doc.import(&updates).unwrap(); + } + with_events.check_tracker(); + for (a, b) in (0..versions.len()).flat_map(|a| (0..versions.len()).map(move |b| (a, b))) { + for (i, context) in [(a, "from"), (b, "to")] { + let context = format!( + "seed {seed}, {context} of {:?} -> {:?}", + versions[a], versions[b] + ); + check_checkout(&with_events.loro, &versions[i], &expected[i], &context); + with_events.check_tracker(); + check_checkout(&plain, &versions[i], &expected[i], &context); + } + } +} + +#[test] +fn checkout_between_any_two_versions_is_path_independent() { + let seeds: Vec = match std::env::var("CHECKOUT_PATH_SEEDS") { + Ok(range) => { + let (start, end) = range + .split_once("..") + .expect("CHECKOUT_PATH_SEEDS=start..end"); + (start.parse().unwrap()..end.parse().unwrap()).collect() + } + Err(_) => (0..24).collect(), + }; + let failed: Vec = seeds + .into_iter() + .filter(|&seed| std::panic::catch_unwind(|| run(seed)).is_err()) + .collect(); + assert!(failed.is_empty(), "failed seeds: {failed:?}"); +} diff --git a/crates/loro-internal/AGENTS.md b/crates/loro-internal/AGENTS.md index 48a49132a..6498d1324 100644 --- a/crates/loro-internal/AGENTS.md +++ b/crates/loro-internal/AGENTS.md @@ -29,7 +29,9 @@ over graceful degradation. `src/handler/movable_list_apply_delta.rs` applies movable-list diffs for `apply_diff` and undo: [../../context/movable-list-apply-diff.md](../../context/movable-list-apply-diff.md). -- `src/diff_calc/`: diff calculation when moving between versions. +- `src/diff_calc/`: diff calculation when moving between versions. The tree + calculator's cache transitions: + [../../context/tree-checkout-window.md](../../context/tree-checkout-window.md). - `src/container/richtext/`: text state with style anchors. Where local inserts go next to anchors, the insert cursor cache, and delete `start_id`s: [../../context/richtext-insert-positions.md](../../context/richtext-insert-positions.md). @@ -65,16 +67,17 @@ coverage under `crates/fuzz` and ask before running long fuzz targets. - Replay-base selection uses Eg-walker terminology (arXiv:2409.14252 §3.5): a version V is **critical** when every event outside `Events(V)` happened after all of `Events(V)` — no concurrency crosses the cut. Non-`Checkout` - diff modes and the tree calculator's lamport windows are only sound when - the base satisfies this; `dag.rs`/`oplog.rs` enforce it via the + diff modes are only sound when the base satisfies this (the tree + calculator's `Checkout` path takes its window from the two versions + instead); `dag.rs`/`oplog.rs` enforce it via the `ImportGreaterUpdates` entry check and, on conservative retreat, the multi-head fixpoint `OpLog::latest_critical_version_below_meet`, with the `latest_single_head_critical_version` descent as fallback. Do not use "LCA" or "common ancestor" in new code or docs: the meet of two versions is generally NOT a safe replay base, and a caller that needs a critical version must ask for one (loro-dev/loro#1095). Read - `docs/critical-version-spec.md` before touching `find_replay_base`, diff - modes, or `diff_calc/tree.rs` windows. + `docs/critical-version-spec.md` before touching `find_replay_base` or diff + modes. `OpLog::iter_from_replay_base_causally` may bypass the DAG when the concurrency is register-only (`docs/diff_calc.md`, "Register-only concurrency"); keep that check container-granular and history-based. diff --git a/crates/loro-internal/docs/critical-version-spec.md b/crates/loro-internal/docs/critical-version-spec.md index 65d559307..d4014bf95 100644 --- a/crates/loro-internal/docs/critical-version-spec.md +++ b/crates/loro-internal/docs/critical-version-spec.md @@ -459,7 +459,7 @@ ancestry(B) 中的事件并发。Eg-walker 的理论最优是"发生在双方之 text/list 依赖下游守卫兜底——本类不受 2026-09 变更影响:L13 只在 第 2 类(保守回退)触发,本类的 meet 基准永远不会被认证为 critical,守卫(base == from ∧ ¬source_not_in_op_context)仍是 - 唯一保障。Tree 的 Checkout 路径见 Q7(已核查安全)。 + 唯一保障。Tree 的 Checkout 路径不再依赖基准,见 Q7。 因此严格的安全陈述是**双层契约**:「(ans, mode) + 下游守卫」合起来 保证收敛;单看走查,无条件承诺的只有 S1/S2/S3/T。用论文语言可以把 @@ -632,20 +632,22 @@ criss-cross ladder 测试钉"tips 去重后复杂度线性"(性能声明,不 Tree 不成立,故不再把守卫当作 IGU 正确性的依据:入场检查(L12)使 S3/S4 直接成立。守卫仍是 text/list 在 Checkout 模式下的兜底,其公理 化留给 Q7。 -- **Q7**(已核查,2026-08-01:安全)Checkout 模式下 meet 非 critical 时 - Tree 的 checkout_diff 是否一致?机制审计发现 Tree 的 Checkout 路径是 - **相对**计算(retreat/forward 都按 meet 前沿的 change 起点 lamport 开窗, - 窗外 op 静默跳过)——窗口隐含 critical 假设。但可证明修复后恒安全: - 区域内经会合进入公共历史的 op 必 ≥ 某 meet 头(在窗口内);与 meet 头 - 并发的 op 必产生 uncovered 死路 → L11 扫描 → critical 基准 → 窗口从 - 基准起点覆盖全区域。实证:非 critical meet 菱形(tree/movable list/ - text)与低 lamport 并发分支两组 checkout 探针全部 canonical,已钉为 - 回归测试 `checkout_across_non_critical_meet_stays_canonical` 与 - `checkout_with_low_lamport_concurrent_branch_stays_canonical` - (后者显式保护"扫描 ↔ Tree 窗口"的耦合:削弱扫描会静默破坏它)。 - 2026-09 注:oplog 的重放基准可能比 Tree 自算的窗口基准**更晚** - (L13 多头 ≥ L11 单头),重放区域只会更小、仍被窗口覆盖;反向 - (把多头基准喂给 Tree 窗口)需要重做本证明,故未做。 +- **Q7**(2026-08-01 判为安全;2026-09-28 推翻并改掉)Checkout 模式下 + meet 非 critical 时 Tree 的 checkout_diff 是否一致?原机制按 meet 前沿 + 的 change 起点 lamport 开窗,窗外 op 静默跳过。原论证称"与 meet 头并发 + 的 op 必产生 uncovered 死路 → L11 扫描 → critical 基准",但这不成立: + 并发 op 的依赖可以先走进 meet 之下的公共历史而会合,不留死路。反例 + (`crates/loro/tests/tree_checkout_path.rs`):`0@3`(lamport 1,建树 + 节点,依赖 `0@1`)与 meet 头 `2@2`(lamport 2)并发;从 `[3@2]` 切到 + `[1@1]` 时 meet = `[2@2]`、无死路,窗口从 2 起,`0@3` 永不被撤回;反方向 + 则前进时漏掉父节点的创建而应用子节点的创建,带订阅者时在 + `TreeState::apply_diff_and_convert` panic。修法:Tree 不再用重放基准, + 而取 from/to 两个版本对称差中最小的 lamport 作窗口(每个 peer 的差段 + 只看首个 op,O(peers)),撤回窗口内全部缓存 op,再按 (lamport, peer) + 顺序应用 to 的窗口内 op;窗口下的 op 两侧相同。此后 Tree 的 Checkout + 路径与基准选择无关,本问题关闭。详见 + `context/tree-checkout-window.md`;随机回归 + `crates/fuzz/tests/checkout_path.rs` 覆盖全部版本对。 - **Q10**(新,2026-09)L13 的搜索只在 ≤ meet 的范围内进行,长期离线 分叉后的每次导入仍付出 O(分歧量)(分岔点确为最晚 critical version,无状态的基准选择无法更好)。是否值得为这一形状缓存 diff --git a/crates/loro-internal/src/dag.rs b/crates/loro-internal/src/dag.rs index 3dc98f50b..cf4538560 100644 --- a/crates/loro-internal/src/dag.rs +++ b/crates/loro-internal/src/dag.rs @@ -83,8 +83,12 @@ pub(crate) trait DagUtils: Dag { /// that need every other op to be before or after the result, such as the /// shallow snapshot root, must use /// [`Self::latest_single_head_critical_version`] (loro-dev/loro#1095). + /// + /// Only the DAG tests call it: the tree calculator used to, and now takes its + /// window from the two versions (`context/tree-checkout-window.md`). + #[cfg(test)] fn find_replay_base(&self, a_id: &Frontiers, b_id: &Frontiers) -> (Frontiers, DiffMode); - /// Like [`Self::find_replay_base`], but leaves the conservative + /// Like `find_replay_base`, but leaves the conservative /// fallback to the caller: on [`MeetAsBase::NeedsCriticalRetreat`] the /// caller must retreat to a critical version (via /// [`Self::latest_single_head_critical_version`], or a cheaper multi-head @@ -116,6 +120,7 @@ pub(crate) trait DagUtils: Dag { } impl DagUtils for T { + #[cfg(test)] #[inline] fn find_replay_base(&self, a_id: &Frontiers, b_id: &Frontiers) -> (Frontiers, DiffMode) { // TODO: perf: make it also return the spans to reach the replay base diff --git a/crates/loro-internal/src/diff_calc/tree.rs b/crates/loro-internal/src/diff_calc/tree.rs index b89fbf93d..7543e5b4e 100644 --- a/crates/loro-internal/src/diff_calc/tree.rs +++ b/crates/loro-internal/src/diff_calc/tree.rs @@ -1,13 +1,11 @@ -use std::{collections::BTreeSet, sync::Arc}; +use std::{collections::BTreeSet, ops::RangeInclusive, sync::Arc}; use fractional_index::FractionalIndex; -use itertools::Itertools; use loro_common::{ContainerID, IdFull, IdLp, Lamport, PeerID, TreeID, ID}; use rustc_hash::FxHashMap; use crate::{ container::{idx::ContainerIdx, tree::tree_op::TreeOp}, - dag::DagUtils, delta::{TreeDelta, TreeDeltaItem, TreeInternalDiff}, event::InternalDiff, state::TreeParentId, @@ -160,69 +158,46 @@ impl TreeDiffCalculator { self.checkout_diff(info, oplog) } + /// Moves the diff cache to `to` without recording diffs. fn checkout(&mut self, to: &VersionVector, to_frontiers: &Frontiers, oplog: &OpLog) { oplog.with_history_cache(|h| { let mark = h.ensure_importing_caches_exist(); let tree_ops = h.get_tree(&self.container, mark).unwrap(); let mut tree_cache = tree_ops.tree().lock(); + tree_cache.init_current_vv(oplog); let s = format!("checkout current {:?} to {:?}", &tree_cache.current_vv, &to); let s = tracing::span!(tracing::Level::INFO, "checkout", s = s); let _e = s.enter(); if to == &tree_cache.current_vv { return; } - let min_lamport = self.get_min_lamport_by_frontiers(to_frontiers, oplog); - // retreat - let mut retreat_ops = vec![]; - for (_target, ops) in tree_cache.tree.iter() { - for op in ops.iter().rev() { - if op.id.lamport < min_lamport { - break; - } - if !to.includes_id(op.id.id()) { - retreat_ops.push(op.clone()); - } - } - } + let Some(min_lamport) = min_lamport_of_version_diff(&tree_cache.current_vv, to, oplog) + else { + tree_cache.current_vv = to.clone(); + return; + }; - for op in retreat_ops { - tree_cache.retreat_op(&op); + let ops = tree_ops.ops(); + let retreat = tree_cache.retreat_range(min_lamport); + for (idlp, op) in retreat.map(|r| ops.range(r)).into_iter().flatten() { + tree_cache.take(IdFull::new(idlp.peer, op.counter, idlp.lamport), &op.value); } - - // forward and apply - let max_lamport = self.get_max_lamport_by_frontiers(to_frontiers, oplog); - let mut forward_ops = vec![]; - let group = h - .get_importing_cache(&self.container, mark) - .unwrap() - .as_tree() - .unwrap(); - for (idlp, op) in group.ops().range( - IdLp { - lamport: 0, - peer: 0, - }..=IdLp { - lamport: max_lamport, - peer: PeerID::MAX, - }, - ) { - if !tree_cache - .current_vv - .includes_id(ID::new(idlp.peer, op.counter)) - && to.includes_id(ID::new(idlp.peer, op.counter)) - { - forward_ops.push((IdFull::new(idlp.peer, op.counter, idlp.lamport), op)); + tree_cache.retreated_from(min_lamport); + + let max_lamport = self + .get_max_lamport_by_frontiers(to_frontiers, oplog) + .max(min_lamport); + for (idlp, op) in + ops.range(IdLp::new(0, min_lamport)..=IdLp::new(PeerID::MAX, max_lamport)) + { + if to.includes_id(ID::new(idlp.peer, op.counter)) { + tree_cache.apply(MoveLamportAndID { + id: IdFull::new(idlp.peer, op.counter, idlp.lamport), + op: op.value.clone(), + effected: false, + }); } } - - for (id, op) in forward_ops { - let op = MoveLamportAndID { - id, - op: op.value.clone(), - effected: false, - }; - tree_cache.apply(op); - } tree_cache.current_vv = to.clone(); }); } @@ -232,190 +207,128 @@ impl TreeDiffCalculator { let mark = h.ensure_importing_caches_exist(); let tree_ops = h.get_tree(&self.container, mark).unwrap(); let mut tree_cache = tree_ops.tree().lock(); + debug_assert_eq!(&tree_cache.current_vv, info.from_vv); let mut parent_to_children_cache = TreeParentToChildrenCache::init_from_tree_cache(&tree_cache); let s = tracing::span!(tracing::Level::INFO, "checkout_diff"); let _e = s.enter(); - let to_frontiers = info.to_frontiers; - let from_frontiers = info.from_frontiers; - let (replay_base, _mode) = oplog.dag.find_replay_base(from_frontiers, to_frontiers); - let base_vv = oplog.dag.frontiers_to_vv(&replay_base).unwrap(); - let base_frontiers = replay_base; - let to_max_lamport = self.get_max_lamport_by_frontiers(to_frontiers, oplog); - // CORRECTNESS: the retreat/forward passes below only look at ops - // with lamport >= `base_min_lamport` (the minimum change-start - // lamport of the base frontiers). Ops outside the window are - // silently skipped, which is only sound if no op of the diff - // region sits below the window. That holds because of how - // `find_replay_base` picks the base: - // - a region op that reaches the base through causal edges is a - // descendant of some base head, so its lamport is strictly - // greater than that head's change-start lamport, which is >= the - // window minimum; - // - a region op CONCURRENT with a base head forces the walk to - // detect an uncovered branch and retreat the base to a critical - // version (latest_single_head_critical_version), below which no - // concurrency crosses — the window then starts at that base. - // See docs/critical-version-spec.md (Q7) and the regression tests - // `checkout_across_non_critical_meet_stays_canonical` and - // `checkout_with_low_lamport_concurrent_branch_stays_canonical` - // in crates/loro/tests/issue.rs. Weakening either the critical - // version fallback or the ImportGreaterUpdates entry check in - // dag.rs breaks this invariant. - // - // The oplog replay base that feeds the calculators may be *later* - // than this window's base: `OpLog::latest_critical_version_below_meet` - // can find a multi-head critical version above the single-head one - // recomputed here. That only shrinks the replayed region relative - // to the window, which stays sound; feeding this window the - // multi-head base instead would require redoing the Q7 proof. - // (The oplog's register-only - // bypass never reaches this path: it only replays from `from` - // when no tree has ops on both sides of the concurrency, and - // such trees take the ImportGreaterUpdates branch instead.) - let base_min_lamport = self.get_min_lamport_by_frontiers(&base_frontiers, oplog); - - // retreat for diff - let mut diffs = vec![]; + // CORRECTNESS: `effected` (the cycle check) of each cached op + // depends on every op ordered before it, so the cache must always + // hold ops applied in (lamport, peer) order. Ops below + // `min_lamport` are the same in `from` and `to`; everything at or + // above it is retreated from `from` and replayed for `to` in that + // order. The window is taken from the versions themselves, not + // from a replay base: in `Checkout` mode `find_replay_base` may + // return a meet that is not a critical version, and ops concurrent + // with that meet can have lower lamports than it. See + // `context/tree-checkout-window.md`. + let Some(min_lamport) = min_lamport_of_version_diff(info.from_vv, info.to_vv, oplog) + else { + tree_cache.current_vv = info.to_vv.clone(); + return TreeDelta::default(); + }; + // `max` keeps the forward range well-formed when `to` only drops ops. + let to_max_lamport = self + .get_max_lamport_by_frontiers(info.to_frontiers, oplog) + .max(min_lamport); + let ops = tree_ops.ops(); - if !(tree_cache.current_vv == base_vv && &base_vv == info.from_vv) { - let mut retreat_ops = vec![]; - for (_target, ops) in tree_cache.tree.iter() { - for op in ops.iter().rev() { - if op.id.lamport < base_min_lamport { - break; - } - if !base_vv.includes_id(op.id.id()) { - retreat_ops.push(op.clone()); - } - } + let mut diffs = vec![]; + // retreat, newest first + let retreat = tree_cache.retreat_range(min_lamport); + for (idlp, op) in retreat.map(|r| ops.range(r)).into_iter().flatten().rev() { + let Some(op) = + tree_cache.take(IdFull::new(idlp.peer, op.counter, idlp.lamport), &op.value) + else { + continue; + }; + if !op.effected { + continue; } - - for op in retreat_ops.into_iter().sorted().rev() { - tree_cache.retreat_op(&op); - let (old_parent, position, last_effective_move_op_id) = - tree_cache.get_parent_with_id(op.op.target()); - if op.effected { - // we need to know whether old_parent is deleted - let is_parent_deleted = tree_cache.is_parent_deleted(op.op.parent_id()); - let is_old_parent_deleted = tree_cache.is_parent_deleted(old_parent); - if op.op.target().id() == op.id.id() { - assert_eq!( - old_parent, - TreeParentId::Unexist, - "old_parent = {:?} instead", - &old_parent - ); - } - parent_to_children_cache.record_change( - op.op.target(), - op.op.parent_id(), - old_parent, - ); - let this_diff = TreeDeltaItem::new( - op.op.target(), - old_parent, - op.op.parent_id(), - last_effective_move_op_id, - is_old_parent_deleted, - is_parent_deleted, - position, - ); - let is_create = matches!(this_diff.action, TreeInternalDiff::Create { .. }); - diffs.push(this_diff); - if is_create { - let mut s = vec![op.op.target()]; - while let Some(t) = s.pop() { - let children = tree_cache.get_children_with_id( - TreeParentId::Node(t), - &parent_to_children_cache, - ); - children.iter().for_each(|c| { - diffs.push(TreeDeltaItem { - target: c.0, - action: TreeInternalDiff::Create { - parent: TreeParentId::Node(t), - position: c.1.clone().unwrap(), - }, - last_effective_move_op_id: c.2, - }) - }); - s.extend(children.iter().map(|c| c.0)); - } - } - } + let (old_parent, position, last_effective_move_op_id) = + tree_cache.get_parent_with_id(op.op.target()); + // we need to know whether old_parent is deleted + let is_parent_deleted = tree_cache.is_parent_deleted(op.op.parent_id()); + let is_old_parent_deleted = tree_cache.is_parent_deleted(old_parent); + if op.op.target().id() == op.id.id() { + assert_eq!( + old_parent, + TreeParentId::Unexist, + "old_parent = {:?} instead", + &old_parent + ); + } + parent_to_children_cache.record_change( + op.op.target(), + op.op.parent_id(), + old_parent, + ); + let this_diff = TreeDeltaItem::new( + op.op.target(), + old_parent, + op.op.parent_id(), + last_effective_move_op_id, + is_old_parent_deleted, + is_parent_deleted, + position, + ); + let is_create = matches!(this_diff.action, TreeInternalDiff::Create { .. }); + diffs.push(this_diff); + if is_create { + tree_cache.push_children_creation( + op.op.target(), + &parent_to_children_cache, + &mut diffs, + ); } } - tree_cache.current_vv = base_vv; - // forward - let group = h - .get_importing_cache(&self.container, mark) - .unwrap() - .as_tree() - .unwrap(); - for (idlp, op) in group.ops().range( - IdLp { - lamport: base_min_lamport, - peer: 0, - }..=IdLp { - lamport: to_max_lamport, - peer: PeerID::MAX, - }, - ) { + + tree_cache.retreated_from(min_lamport); + + // forward, oldest first + for (idlp, op) in + ops.range(IdLp::new(0, min_lamport)..=IdLp::new(PeerID::MAX, to_max_lamport)) + { let id = ID::new(idlp.peer, op.counter); - if !tree_cache.current_vv.includes_id(id) && info.to_vv.includes_id(id) { - let op = MoveLamportAndID { - id: IdFull { - peer: id.peer, - lamport: idlp.lamport, - counter: id.counter, - }, - op: op.value.clone(), - effected: false, - }; - let (old_parent, _position, _id) = - tree_cache.get_parent_with_id(op.op.target()); - let is_parent_deleted = tree_cache.is_parent_deleted(op.op.parent_id()); - let is_old_parent_deleted = tree_cache.is_parent_deleted(old_parent); - let effected = tree_cache.apply(op.clone()); - if effected { - let this_diff = TreeDeltaItem::new( - op.op.target(), - op.op.parent_id(), - old_parent, - op.id_full(), - is_parent_deleted, - is_old_parent_deleted, - op.op.fractional_index(), - ); - parent_to_children_cache.record_change( + if !info.to_vv.includes_id(id) { + continue; + } + let op = MoveLamportAndID { + id: IdFull { + peer: id.peer, + lamport: idlp.lamport, + counter: id.counter, + }, + op: op.value.clone(), + effected: false, + }; + let (old_parent, _position, _id) = tree_cache.get_parent_with_id(op.op.target()); + let is_parent_deleted = tree_cache.is_parent_deleted(op.op.parent_id()); + let is_old_parent_deleted = tree_cache.is_parent_deleted(old_parent); + let effected = tree_cache.apply(op.clone()); + if effected { + let this_diff = TreeDeltaItem::new( + op.op.target(), + op.op.parent_id(), + old_parent, + op.id_full(), + is_parent_deleted, + is_old_parent_deleted, + op.op.fractional_index(), + ); + parent_to_children_cache.record_change( + op.op.target(), + old_parent, + op.op.parent_id(), + ); + let is_create = matches!(this_diff.action, TreeInternalDiff::Create { .. }); + diffs.push(this_diff); + if is_create { + tree_cache.push_children_creation( op.op.target(), - old_parent, - op.op.parent_id(), + &parent_to_children_cache, + &mut diffs, ); - let is_create = matches!(this_diff.action, TreeInternalDiff::Create { .. }); - diffs.push(this_diff); - if is_create { - // TODO: per - let mut s = vec![op.op.target()]; - while let Some(t) = s.pop() { - let children = tree_cache.get_children_with_id( - TreeParentId::Node(t), - &parent_to_children_cache, - ); - children.iter().for_each(|c| { - diffs.push(TreeDeltaItem { - target: c.0, - action: TreeInternalDiff::Create { - parent: TreeParentId::Node(t), - position: c.1.clone().unwrap(), - }, - last_effective_move_op_id: c.2, - }) - }); - s.extend(children.iter().map(|x| x.0)); - } - } } } } @@ -425,14 +338,6 @@ impl TreeDiffCalculator { }) } - fn get_min_lamport_by_frontiers(&self, frontiers: &Frontiers, oplog: &OpLog) -> Lamport { - frontiers - .iter() - .map(|id| oplog.get_min_lamport_at(id)) - .min() - .unwrap_or(0) - } - fn get_max_lamport_by_frontiers(&self, frontiers: &Frontiers, oplog: &OpLog) -> Lamport { frontiers .iter() @@ -442,6 +347,28 @@ impl TreeDiffCalculator { } } +/// The smallest lamport among the ops that are in exactly one of `a` and `b`, +/// or `None` when both contain the same ops. +/// +/// Every tree op below it is in both versions, so moving the diff cache +/// between them only has to retreat and replay the ops at or above it. +fn min_lamport_of_version_diff( + a: &VersionVector, + b: &VersionVector, + oplog: &OpLog, +) -> Option { + a.sub_iter(b) + .chain(b.sub_iter(a)) + .map(|span| { + let id = ID::new(span.peer, span.counter.start); + oplog + .dag + .get_lamport(&id) + .unwrap_or_else(|| panic!("op {id} of a tree diff version is not in the DAG")) + }) + .min() +} + /// All information of an operation for diff calculating of movable tree. #[derive(Debug, Clone)] pub struct MoveLamportAndID { @@ -485,6 +412,14 @@ impl Ord for MoveLamportAndID { pub(crate) struct TreeCacheForDiff { tree: FxHashMap>, current_vv: VersionVector, + /// `false` until the first transition. In a shallow doc the cache starts + /// at the shallow root (seeded with its state, or empty when the tree had + /// no nodes there), which `current_vv` does not express yet. + current_vv_initialized: bool, + /// No cached op has a greater lamport. Bounds the retreat so it does not + /// walk the ops above the cached version, which can be most of the + /// history when checking out between two old versions. + max_lamport: Lamport, } impl std::fmt::Debug for TreeCacheForDiff { @@ -499,9 +434,64 @@ impl std::fmt::Debug for TreeCacheForDiff { } impl TreeCacheForDiff { - fn retreat_op(&mut self, op: &MoveLamportAndID) { - self.tree.get_mut(&op.op.target()).unwrap().remove(op); - self.current_vv.set_end(op.id.id()); + /// Removes the op from the cache if it is there, returning it with its + /// `effected` flag. + fn take(&mut self, id: IdFull, op: &Arc) -> Option { + self.tree.get_mut(&op.target())?.take(&MoveLamportAndID { + id, + op: op.clone(), + effected: false, + }) + } + + /// The `IdLp` range holding every cached op whose lamport is at least + /// `min_lamport`, or `None` when there is none. + fn retreat_range(&self, min_lamport: Lamport) -> Option> { + (self.max_lamport >= min_lamport) + .then(|| IdLp::new(0, min_lamport)..=IdLp::new(PeerID::MAX, self.max_lamport)) + } + + /// Records that every cached op whose lamport is at least `min_lamport` + /// has been retreated. + fn retreated_from(&mut self, min_lamport: Lamport) { + self.max_lamport = self.max_lamport.min(min_lamport.saturating_sub(1)); + } + + fn init_current_vv(&mut self, oplog: &OpLog) { + if self.current_vv_initialized { + return; + } + + self.current_vv_initialized = true; + if oplog.is_shallow() { + self.current_vv = oplog + .dag + .frontiers_to_vv(oplog.dag.shallow_since_frontiers()) + .expect("the shallow root version must be in the DAG"); + } + } + + /// After `target` is (re)created, pushes a `Create` for each node of its + /// current subtree. + fn push_children_creation( + &self, + target: TreeID, + cache: &TreeParentToChildrenCache, + diffs: &mut Vec, + ) { + let mut s = vec![target]; + while let Some(t) = s.pop() { + let children = self.get_children_with_id(TreeParentId::Node(t), cache); + diffs.extend(children.iter().map(|c| TreeDeltaItem { + target: c.0, + action: TreeInternalDiff::Create { + parent: TreeParentId::Node(t), + position: c.1.clone().unwrap(), + }, + last_effective_move_op_id: c.2, + })); + s.extend(children.iter().map(|c| c.0)); + } } fn is_ancestor_of(&self, maybe_ancestor: &TreeID, node_id: &TreeParentId) -> bool { @@ -532,6 +522,7 @@ impl TreeCacheForDiff { effected = false; } node.effected = effected; + self.max_lamport = self.max_lamport.max(node.id.lamport); self.current_vv.set_last(node.id.id()); self.tree.entry(node.op.target()).or_default().insert(node); effected @@ -544,6 +535,7 @@ impl TreeCacheForDiff { debug_assert!(self.tree.is_empty()); for node in nodes.into_iter() { + self.max_lamport = self.max_lamport.max(node.id.lamport); self.current_vv.extend_to_include_last_id(node.id.id()); self.current_vv .extend_to_include_last_id(node.op.target().id()); @@ -682,3 +674,52 @@ impl TreeParentToChildrenCache { self.cache.entry(new_parent).or_default().insert(target); } } + +#[cfg(test)] +mod test { + use loro_common::{TreeID, ID}; + + use crate::{state::TreeParentId, version::Frontiers, HandlerTrait, LoroDoc}; + + /// Checking out between two old versions must retreat only what the cache + /// holds: `max_lamport` bounds every cached op, and stays at the cached + /// version instead of the end of the history. + #[test] + fn retreat_bound_follows_the_cached_version() { + let doc = LoroDoc::new_auto_commit(); + doc.set_peer_id(1).unwrap(); + let tree = doc.get_tree("tree"); + let nodes: Vec = (0..20) + .map(|_| tree.create(TreeParentId::Root).unwrap()) + .collect(); + doc.commit_then_renew(); + let mut versions = vec![]; + for i in 0..400 { + let _ = tree.mov(nodes[i % 20], TreeParentId::Node(nodes[(i * 7 + 3) % 20])); + doc.commit_then_renew(); + versions.push(doc.oplog_frontiers()); + } + + let (v1, v2) = (&versions[10], &versions[20]); + let lamport_of = |f: &Frontiers| { + let id: ID = f.as_single().unwrap(); + doc.oplog().lock().dag.get_lamport(&id).unwrap() + }; + for v in [v1, v2, v1, v2] { + doc.checkout(v).unwrap(); + let (cached_max, bound) = doc.oplog().lock().with_history_cache(|h| { + let mark = h.ensure_importing_caches_exist(); + let cache = h.get_tree(&tree.idx(), mark).unwrap().tree().lock(); + let cached_max = cache + .tree + .values() + .flat_map(|ops| ops.iter().map(|op| op.id.lamport)) + .max() + .unwrap(); + (cached_max, cache.max_lamport) + }); + assert!(cached_max <= bound); + assert!(bound <= lamport_of(v)); + } + } +} diff --git a/crates/loro-internal/src/history_cache.rs b/crates/loro-internal/src/history_cache.rs index 6769e7d5f..c0cad3ed6 100644 --- a/crates/loro-internal/src/history_cache.rs +++ b/crates/loro-internal/src/history_cache.rs @@ -289,14 +289,6 @@ impl ContainerHistoryCache { } } - pub(crate) fn get_importing_cache( - &self, - container_idx: &ContainerIdx, - _: HasImportingCacheMark, - ) -> Option<&HistoryCacheForImporting> { - self.for_importing.as_ref().unwrap().get(container_idx) - } - pub(crate) fn get_tree( &self, container_idx: &ContainerIdx, diff --git a/crates/loro-internal/src/oplog.rs b/crates/loro-internal/src/oplog.rs index cf3de3afd..68e888c0d 100644 --- a/crates/loro-internal/src/oplog.rs +++ b/crates/loro-internal/src/oplog.rs @@ -538,10 +538,6 @@ impl OpLog { self.dag.cmp_frontiers(a, b) } - pub(crate) fn get_min_lamport_at(&self, id: ID) -> Lamport { - self.get_change_at(id).map(|c| c.lamport).unwrap_or(0) - } - pub(crate) fn get_lamport_at(&self, id: ID) -> Option { self.get_change_at(id) .map(|c| c.lamport + (id.counter - c.id.counter) as Lamport) diff --git a/crates/loro-wasm/tests/tree_checkout_path.test.ts b/crates/loro-wasm/tests/tree_checkout_path.test.ts new file mode 100644 index 000000000..e78c235c6 --- /dev/null +++ b/crates/loro-wasm/tests/tree_checkout_path.test.ts @@ -0,0 +1,140 @@ +import { describe, expect, it } from "vitest"; +import { LoroDoc } from "../bundler/index"; + +// Repros from the loro.js differential fuzz (loro-dev/loro#1141). See +// context/tree-checkout-window.md. + +function docWithPeer(peer: bigint): LoroDoc { + const doc = new LoroDoc(); + doc.setPeerId(peer); + return doc; +} + +function merged(docs: LoroDoc[]): LoroDoc { + const doc = new LoroDoc(); + for (const d of docs) doc.import(d.export({ mode: "update" })); + return doc; +} + +describe("tree checkout path", () => { + it("does not panic on a checkout sequence with a subscriber", () => { + const p1 = docWithPeer(1n); + p1.getList("l").insert(0, "x"); + const a = p1.getTree("tr").createNode(); + const b = p1.getTree("tr").createNode(a.id); + p1.commit(); + + const p3 = docWithPeer(3n); + p3.import( + p1.export({ + mode: "updates-in-range", + spans: [{ id: { peer: "1", counter: 0 }, len: 1 }], + }), + ); + p3.getList("l").insert(0, "y"); + p3.commit(); + + const p2 = docWithPeer(2n); + p2.import(p3.export({ mode: "update" })); + p2.getCounter("c").increment(1); + p2.commit(); + + p3.import(p2.export({ mode: "update" })); + p3.getMap("m").set("k", 1); + p3.commit(); + + p2.import(p1.export({ mode: "update" })); + p2.getTree("tr").getNodeByID(b.id)!.data.set("k", true); + p2.commit(); + + const doc = merged([p1, p2, p3]); + let events = 0; + doc.subscribe(() => { + events += 1; + }); + for (const version of [ + [{ peer: "3" as const, counter: 1 }], + [{ peer: "2" as const, counter: 1 }], + ]) { + doc.checkout(version); + const fresh = merged([p1, p2, p3]); + fresh.checkout(version); + expect(doc.toJSON()).toStrictEqual(fresh.toJSON()); + } + expect(events).toBeGreaterThan(0); + }); + + it("removes a node when checking out a concurrent version without its creation", () => { + const [p1, p2, p3] = [1n, 2n, 3n].map(docWithPeer); + p1.getCounter("c").increment(1); + p1.commit(); // 0@1 + p3.import(p1.export({ mode: "update" })); + p3.getTree("tr").createNode(); + p3.commit(); // 0@3, deps 0@1 + p2.getList("l").insert(0, 1); + p2.getList("l").delete(0, 1); + p2.commit(); // 0@2..1@2 + p2.import(p1.export({ mode: "update" })); + p2.getCounter("c").increment(1); + p2.commit(); // 2@2, deps [0@1, 1@2] + p1.import(p2.export({ mode: "update" })); + p1.getMap("m").set("k", 1); + p1.commit(); // 1@1, deps 2@2 + p2.import(p3.export({ mode: "update" })); + p2.getText("t").insert(0, "x"); + p2.commit(); // 3@2, deps [2@2, 0@3] + + const target = [{ peer: "1" as const, counter: 1 }]; + const direct = merged([p1, p2, p3]); + direct.checkout(target); + expect(direct.getTree("tr").toJSON()).toStrictEqual([]); + + const doc = merged([p1, p2, p3]); + doc.checkout([{ peer: "2", counter: 3 }]); + expect(doc.getTree("tr").toJSON().length).toBe(1); + doc.checkout(target); + expect(doc.getTree("tr").toJSON()).toStrictEqual([]); + expect(doc.toJSON()).toStrictEqual(direct.toJSON()); + }); +}); + +describe("tree checkout between old versions", () => { + it("matches a direct checkout after scrubbing back and forth", () => { + const doc = docWithPeer(1n); + const tree = doc.getTree("tree"); + const nodes = Array.from({ length: 30 }, () => tree.createNode().id); + doc.commit(); + let seed = 7; + const next = (n: number) => { + seed = (seed * 1103515245 + 12345) % 2147483648; + return seed % n; + }; + for (let i = 0; i < 1200; i++) { + try { + tree.move(nodes[next(nodes.length)], nodes[next(nodes.length)]); + } catch { + // moving a node under its own descendant is rejected + } + if (i % 20 === 19) doc.commit(); + } + doc.commit(); + + // Rejected moves record no op, so pick versions from the last counter. + const last = doc.frontiers()[0].counter; + const versions = [0.2, 0.25, 0.7].map((f) => [ + { peer: "1" as const, counter: Math.floor(last * f) }, + ]); + const expected = versions.map((v) => { + const fresh = new LoroDoc(); + fresh.import(doc.export({ mode: "update" })); + fresh.checkout(v); + return fresh.getTree("tree").toJSON(); + }); + for (let round = 0; round < 3; round++) { + for (const [i, v] of versions.entries()) { + doc.checkout(v); + expect(doc.getTree("tree").toJSON()).toStrictEqual(expected[i]); + } + } + }); +}); diff --git a/crates/loro/tests/issue.rs b/crates/loro/tests/issue.rs index 3cfb9a152..4068f1dbb 100644 --- a/crates/loro/tests/issue.rs +++ b/crates/loro/tests/issue.rs @@ -586,8 +586,8 @@ fn tree_concurrent_moves_incremental_state_matches_replay() { /// Checkout between divergent versions whose meet is NOT a critical version /// (diamond: the region op `1@1` is concurrent with the meet head `0@2`). -/// The tree/movable-list/text calculators replay relatively around the base; -/// this pins that such checkouts stay canonical. +/// The movable-list/text calculators replay relatively around the base; this +/// pins that such checkouts stay canonical. #[test] fn checkout_across_non_critical_meet_stays_canonical() { use loro::{Frontiers, TreeID, ID}; @@ -645,11 +645,10 @@ fn checkout_across_non_critical_meet_stays_canonical() { } /// Checkout where the diff region contains a LOW-lamport concurrent branch -/// (below the meet frontier's lamport window). The tree calculator's -/// retreat/forward windows skip ops below `lca_min_lamport`, so this is only -/// safe because the walk retreats the base to a critical version (the -/// latest-singleton-cut sweep) whenever such a branch exists. Pins that -/// interplay: weakening the sweep would silently break this. +/// (below the lamport of the meet frontier). The tree calculator takes its +/// retreat/forward window from the two versions, not from the replay base +/// (`context/tree-checkout-window.md`); this pins that the branch is still +/// forwarded and retreated. #[test] fn checkout_with_low_lamport_concurrent_branch_stays_canonical() { use loro::{Frontiers, TreeID, ID}; diff --git a/crates/loro/tests/perf_tree_checkout_old_versions.rs b/crates/loro/tests/perf_tree_checkout_old_versions.rs new file mode 100644 index 000000000..ae48452c8 --- /dev/null +++ b/crates/loro/tests/perf_tree_checkout_old_versions.rs @@ -0,0 +1,74 @@ +use loro::{Frontiers, LoroDoc, TreeID, TreeParentId, ID}; +use std::time::{Duration, Instant}; + +/// A tree with 2k nodes and `moves` random moves by one peer, committed every +/// 500 moves. +fn tree_history(moves: usize) -> LoroDoc { + let doc = LoroDoc::new(); + doc.set_peer_id(1).unwrap(); + let tree = doc.get_tree("tree"); + let mut nodes: Vec = Vec::new(); + let mut seed = 1u64; + let mut next = move |n: usize| { + seed = seed + .wrapping_mul(6364136223846793005) + .wrapping_add(1442695040888963407); + (seed >> 33) as usize % n + }; + for i in 0..2_000 { + let parent = if i < 20 { + TreeParentId::Root + } else { + TreeParentId::Node(nodes[next(nodes.len())]) + }; + nodes.push(tree.create(parent).unwrap()); + } + doc.commit(); + for i in 0..moves { + let target = nodes[next(nodes.len())]; + let parent = nodes[next(nodes.len())]; + let _ = tree.mov(target, parent); + if i % 500 == 499 { + doc.commit(); + } + } + doc.commit(); + doc +} + +/// Checking out back and forth between two old versions, 40 checkouts. +fn scrub_old_versions(doc: &LoroDoc) -> Duration { + let a = Frontiers::from_id(ID::new(1, 2_500)); + let b = Frontiers::from_id(ID::new(1, 2_510)); + doc.checkout(&a).unwrap(); + doc.checkout(&b).unwrap(); + let start = Instant::now(); + for _ in 0..20 { + doc.checkout(&a).unwrap(); + doc.checkout(&b).unwrap(); + } + start.elapsed() +} + +/// The tree diff retreats only the ops the cached version holds, so checking +/// out between two old versions must not get slower as later history grows +/// (it scanned every later tree op before `TreeCacheForDiff::max_lamport`). +/// See `context/tree-checkout-window.md`. +#[test] +#[ignore] +fn perf_tree_checkout_between_old_versions_does_not_scale_with_later_history() { + // Run with: + // cargo test --release -p loro --test perf_tree_checkout_old_versions -- --ignored --nocapture + let short = tree_history(10_000); + let long = tree_history(200_000); + let (mut t_short, mut t_long) = (Duration::MAX, Duration::MAX); + for _ in 0..3 { + t_short = t_short.min(scrub_old_versions(&short)); + t_long = t_long.min(scrub_old_versions(&long)); + } + println!("10k later moves: {t_short:?}, 200k later moves: {t_long:?}"); + assert!( + t_long < t_short * 4 + Duration::from_millis(5), + "checkout between old versions scales with later history: {t_short:?} -> {t_long:?}" + ); +} diff --git a/crates/loro/tests/tree_checkout_path.rs b/crates/loro/tests/tree_checkout_path.rs new file mode 100644 index 000000000..17edeb0fe --- /dev/null +++ b/crates/loro/tests/tree_checkout_path.rs @@ -0,0 +1,126 @@ +//! Tree checkouts must not depend on the path taken. Minimal repros found by +//! the `loro.js` differential fuzz (loro-dev/loro#1141). See +//! `context/tree-checkout-window.md`. + +use loro::{ExportMode, Frontiers, IdSpan, LoroDoc, TreeParentId, ID}; + +fn doc_with_peer(peer: u64) -> LoroDoc { + let doc = LoroDoc::new(); + doc.set_peer_id(peer).unwrap(); + doc +} + +fn merged(docs: &[&LoroDoc]) -> LoroDoc { + let doc = LoroDoc::new(); + for d in docs { + doc.import(&d.export(ExportMode::all_updates()).unwrap()) + .unwrap(); + } + doc +} + +fn tree_node_ids(doc: &LoroDoc, name: &str) -> Vec { + let tree = doc.get_tree(name); + let mut ids: Vec = tree + .nodes() + .into_iter() + .filter(|id| !tree.is_node_deleted(id).unwrap()) + .map(|id| id.to_string()) + .collect(); + ids.sort(); + ids +} + +/// With a subscriber, `checkout([3@1])` then `checkout([2@1])` panicked in +/// `TreeState::apply_diff_and_convert` (`get_index_by_tree_id(..).unwrap()` on +/// a `Create`). +#[test] +fn checkout_sequence_with_subscriber_does_not_panic() { + let p1 = doc_with_peer(1); + p1.get_list("l").insert(0, "x").unwrap(); + let tree = p1.get_tree("tr"); + let a = tree.create(TreeParentId::Root).unwrap(); + let b = tree.create(a).unwrap(); + p1.commit(); + + let p3 = doc_with_peer(3); + p3.import( + &p1.export(ExportMode::updates_in_range(vec![IdSpan::new(1, 0, 1)])) + .unwrap(), + ) + .unwrap(); + p3.get_list("l").insert(0, "y").unwrap(); + p3.commit(); + + let p2 = doc_with_peer(2); + p2.import(&p3.export(ExportMode::all_updates()).unwrap()) + .unwrap(); + p2.get_counter("c").increment(1.0).unwrap(); + p2.commit(); + + p3.import(&p2.export(ExportMode::all_updates()).unwrap()) + .unwrap(); + p3.get_map("m").insert("k", 1).unwrap(); + p3.commit(); + + p2.import(&p1.export(ExportMode::all_updates()).unwrap()) + .unwrap(); + p2.get_tree("tr") + .get_meta(b) + .unwrap() + .insert("k", true) + .unwrap(); + p2.commit(); + + let doc = merged(&[&p1, &p2, &p3]); + let _sub = doc.subscribe_root(std::sync::Arc::new(|_| {})); + for f in [ID::new(3, 1), ID::new(2, 1)] { + let f = Frontiers::from_id(f); + doc.checkout(&f).unwrap(); + let fresh = merged(&[&p1, &p2, &p3]); + fresh.checkout(&f).unwrap(); + assert_eq!(doc.get_deep_value(), fresh.get_deep_value(), "at {f:?}"); + } +} + +/// Checking out `[3@2]` (which contains the creation of a tree node) and then +/// the concurrent `[1@1]` (which does not) kept the node. +#[test] +fn checkout_to_concurrent_version_without_node_creation_removes_node() { + let p1 = doc_with_peer(1); + let p2 = doc_with_peer(2); + let p3 = doc_with_peer(3); + p1.get_counter("c").increment(1.0).unwrap(); + p1.commit(); // 0@1 + p3.import(&p1.export(ExportMode::all_updates()).unwrap()) + .unwrap(); + p3.get_tree("tr").create(TreeParentId::Root).unwrap(); + p3.commit(); // 0@3, deps 0@1 + p2.get_list("l").insert(0, 1).unwrap(); + p2.get_list("l").delete(0, 1).unwrap(); + p2.commit(); // 0@2..1@2 + p2.import(&p1.export(ExportMode::all_updates()).unwrap()) + .unwrap(); + p2.get_counter("c").increment(1.0).unwrap(); + p2.commit(); // 2@2, deps [0@1, 1@2] + p1.import(&p2.export(ExportMode::all_updates()).unwrap()) + .unwrap(); + p1.get_map("m").insert("k", 1).unwrap(); + p1.commit(); // 1@1, deps 2@2 + p2.import(&p3.export(ExportMode::all_updates()).unwrap()) + .unwrap(); + p2.get_text("t").insert(0, "x").unwrap(); + p2.commit(); // 3@2, deps [2@2, 0@3] + + let target = Frontiers::from_id(ID::new(1, 1)); + let direct = merged(&[&p1, &p2, &p3]); + direct.checkout(&target).unwrap(); + assert!(tree_node_ids(&direct, "tr").is_empty()); + + let doc = merged(&[&p1, &p2, &p3]); + doc.checkout(&Frontiers::from_id(ID::new(2, 3))).unwrap(); + assert_eq!(tree_node_ids(&doc, "tr").len(), 1); + doc.checkout(&target).unwrap(); + assert_eq!(tree_node_ids(&doc, "tr"), tree_node_ids(&direct, "tr")); + assert_eq!(doc.get_deep_value(), direct.get_deep_value()); +}