From 743240a95e290e4a1c424264bf1c3f7005840088 Mon Sep 17 00:00:00 2001 From: Zixuan Chen Date: Sun, 27 Sep 2026 23:26:41 +0800 Subject: [PATCH 1/5] fix: keep root-time map values when checking out a shallow doc Checkout skipped map keys (and movable-list values) whose value was equal at both versions even when a different op won, leaving the later op's lamport/peer in the state. Shallow and state-only exports build their root state through that checkout; on import the stale entry collided with the retained later op in the map checkout index and was dropped, so checkout to the root lost the key. Emit the winner change and apply it as a silent metadata update. See "Winner metadata, not just values" in crates/loro-internal/docs/diff_calc.md. Co-Authored-By: Claude Opus 5.5 (1M context) --- .changeset/shallow-checkout-equal-value.md | 5 + context/internal-encoding.md | 6 +- crates/loro-internal/docs/diff_calc.md | 20 ++ crates/loro-internal/src/diff_calc.rs | 22 +- crates/loro-internal/src/state/map_state.rs | 28 +- .../src/state/movable_list_state.rs | 12 +- .../tests/shallow_checkout_equal_value.rs | 309 ++++++++++++++++++ 7 files changed, 390 insertions(+), 12 deletions(-) create mode 100644 .changeset/shallow-checkout-equal-value.md create mode 100644 crates/loro/tests/shallow_checkout_equal_value.rs diff --git a/.changeset/shallow-checkout-equal-value.md b/.changeset/shallow-checkout-equal-value.md new file mode 100644 index 000000000..c28b408a5 --- /dev/null +++ b/.changeset/shallow-checkout-equal-value.md @@ -0,0 +1,5 @@ +--- +"loro-crdt": patch +--- + +Fix a map value disappearing after importing a shallow snapshot and checking out its root. It happened when a key was rewritten after the root with the value it had at the root (for example by `revertTo`) and the op that wrote the root value was trimmed. Checking out a version also now updates the last editor of map entries and movable-list items whose value is equal at both versions, so `getLastEditor` is correct after a checkout, and shallow and state-only snapshots record the right writer in their root state. Snapshots exported by older versions can still carry the wrong writer in their root state; export them again from a full-history document to fix them. diff --git a/context/internal-encoding.md b/context/internal-encoding.md index fa5b62a1a..3a619030c 100644 --- a/context/internal-encoding.md +++ b/context/internal-encoding.md @@ -252,7 +252,11 @@ after the root, never concurrent with it (loro-dev/loro#1095). The meet of the requested heads is not enough: a branch merged later that forked below the requested version is concurrent with it. `StateOnly` retains history only up to its target, so it uses the target on both sides. -`crates/loro/tests/shallow_root_critical.rs` checks the property op by op. The +`crates/loro/tests/shallow_root_critical.rs` checks the property op by op. +Because the root state is materialized by a checkout, it is only as correct as +the checkout's winner metadata (lamport/peer), not just its values: see +"Winner metadata, not just values" in +[crates/loro-internal/docs/diff_calc.md](../crates/loro-internal/docs/diff_calc.md). The root is then moved past a rich-text StyleStart when necessary and clamped to an existing shallow root. The root state carries `fr`; a later state overlay does not. Import loads the root first and then either overlays the later state diff --git a/crates/loro-internal/docs/diff_calc.md b/crates/loro-internal/docs/diff_calc.md index 81599f1be..78a202281 100644 --- a/crates/loro-internal/docs/diff_calc.md +++ b/crates/loro-internal/docs/diff_calc.md @@ -78,6 +78,26 @@ Map diffs in `Checkout`/`Import` mode only look up the keys written inside the replayed span, and skip replayed ops that both versions already contain, so their cost follows the update instead of the map size. +## Winner metadata, not just values + +Map and MovableList states store the winning op's lamport/peer (map entry, +movable-list `value_id`) next to the value. A checkout must move that metadata +even when both versions hold an equal value written by different ops (a value +rewritten after the target, e.g. by `revert_to`). `MapDiffCalculator` therefore +emits a key whenever the winning op differs, and `MapState` / +`MovableListState` apply an equal-value entry as a silent metadata update with +no event, so events still only report value changes. + +Skipping those entries left the later op's metadata in the checked-out state. +Shallow and state-only exports build their root state through such a checkout, +so the root state carried a lamport/peer that is not in the root version. On +import, `ensure_shallow_map_seeded` inserted that entry into the map checkout +index, where it compared equal to the retained later op (the index is keyed by +container, key, lamport and peer) and was dropped, so `checkout(root)` lost the +key whenever its root-time writer was trimmed. `get_last_editor` after a +checkout was wrong for the same reason. Regression tests: +`crates/loro/tests/shallow_checkout_equal_value.rs`. + ## Diff modes - `Checkout` is the general and slowest mode. It can move in either direction diff --git a/crates/loro-internal/src/diff_calc.rs b/crates/loro-internal/src/diff_calc.rs index 28a620563..e5acb8fdc 100644 --- a/crates/loro-internal/src/diff_calc.rs +++ b/crates/loro-internal/src/diff_calc.rs @@ -687,29 +687,39 @@ impl DiffCalculatorTrait for MapDiffCalculator { oplog, ); + // A key whose winning op differs must be emitted even when both + // ops carry the same value: the state keeps the winner's + // lamport/peer, and a shallow root state encoded from it seeds + // the history cache. Stale metadata there collides with a + // retained op and loses the root entry. `MapState` applies such + // metadata-only entries without reporting a value change. for (k, peek_from) in from_map.iter() { let peek_to = to_map.remove(k); match peek_to { - None => changed.push((k.clone(), None)), + None => changed.push((k.clone(), None, true)), Some(b) => { if peek_from.value != b.value { - changed.push((k.clone(), Some(b))) + changed.push((k.clone(), Some(b), true)) + } else if peek_from.lamport != b.lamport || peek_from.peer != b.peer { + changed.push((k.clone(), Some(b), false)) } } } } for (k, peek_to) in to_map.into_iter() { - changed.push((k, Some(peek_to))); + changed.push((k, Some(peek_to), true)); } let mut updated = FxHashMap::with_capacity_and_hasher(changed.len(), Default::default()); - for (key, value) in changed { + for (key, value, value_changed) in changed { let value = value.map(|v| { let value = v.value.clone(); - if let Some(LoroValue::Container(c)) = &value { - on_new_container(c); + if value_changed { + if let Some(LoroValue::Container(c)) = &value { + on_new_container(c); + } } MapValue { diff --git a/crates/loro-internal/src/state/map_state.rs b/crates/loro-internal/src/state/map_state.rs index d82492a3a..a615bb997 100644 --- a/crates/loro-internal/src/state/map_state.rs +++ b/crates/loro-internal/src/state/map_state.rs @@ -260,6 +260,18 @@ impl ContainerState for MapState { continue; }; + if mode == DiffMode::Checkout + && self + .map + .get(&key) + .is_some_and(|old| old.value == value.value) + { + // Only the winning op changed (see `MapDiffCalculator`): keep the + // lamport/peer accurate without reporting a value change. + self.insert(key, value); + continue; + } + let mut changed = false; if force { self.insert(key.clone(), value.clone()); @@ -394,13 +406,21 @@ impl MapState { pub fn insert(&mut self, key: InternalString, value: MapValue) -> Option { let value_yes = value.value.is_some(); - if let Some(LoroValue::Container(id)) = &value.value { - self.child_containers.insert(id.clone(), key.clone()); - } + let new_child = match &value.value { + Some(LoroValue::Container(id)) => { + self.child_containers.insert(id.clone(), key.clone()); + Some(id.clone()) + } + _ => None, + }; let result = self.map.insert(key.clone(), value); if let Some(Some(LoroValue::Container(c))) = result.as_ref().map(|x| &x.value) { - self.child_containers.remove(c); + // Re-inserting the same child (e.g. a metadata-only checkout update) + // must keep its parent edge. + if new_child.as_ref() != Some(c) { + self.child_containers.remove(c); + } } match (&result, value_yes) { diff --git a/crates/loro-internal/src/state/movable_list_state.rs b/crates/loro-internal/src/state/movable_list_state.rs index ea8f89ba1..5b2b75650 100644 --- a/crates/loro-internal/src/state/movable_list_state.rs +++ b/crates/loro-internal/src/state/movable_list_state.rs @@ -1134,7 +1134,17 @@ impl ContainerState for MovableListState { Some(elem) => { // Update value if needed if let Some(value_id) = value_id { - if elem.value != value && (!need_compare || elem.value_id < value_id) { + if elem.value == value + && elem.value_id != value_id + && (!need_compare || elem.value_id < value_id) + { + // Only the winning set op changed (e.g. a checkout + // past a later set of the same value): keep the + // value id accurate without an event, as for maps. + self.inner.update_value(elem_id, value.clone(), value_id); + } else if elem.value != value + && (!need_compare || elem.value_id < value_id) + { maybe_moved.remove(&elem_id); self.inner.update_value(elem_id, value.clone(), value_id); let index = self.get_index_of_elem(elem_id); diff --git a/crates/loro/tests/shallow_checkout_equal_value.rs b/crates/loro/tests/shallow_checkout_equal_value.rs new file mode 100644 index 000000000..18628e864 --- /dev/null +++ b/crates/loro/tests/shallow_checkout_equal_value.rs @@ -0,0 +1,309 @@ +//! A value written again after the shallow root, equal to its value at the +//! root, must not make the root value disappear from a shallow replica. +//! +//! Checkout used to skip map keys whose value is the same at both versions even +//! when a different op wins, so the state kept the later op's lamport/peer. The +//! shallow root state is built by such a checkout, and on import its entry then +//! collided with the retained later op in the map history cache and was dropped: +//! `checkout(root)` lost the key. The root commit holds more than one op so the +//! root-time writer of the key is trimmed from the shallow history. +//! +//! Every test compares a shallow replica against a full-history replica over the +//! whole retained range, with 0 and 300 padding ops (the latter is above the +//! 256-op threshold that ships a latest-state overlay). + +use loro::{ExportMode, Frontiers, LoroDoc, LoroMap, LoroValue, ToJson, TreeParentId, UndoManager}; + +const PADS: [usize; 2] = [0, 300]; + +struct Fixture { + doc: LoroDoc, + root: Frontiers, + versions: Vec, +} + +fn commit(doc: &LoroDoc, versions: &mut Vec) { + doc.commit(); + versions.push(doc.oplog_frontiers()); +} + +/// Every container kind gets a value at the root, a different value after it, +/// and then its root-time value again. +fn fixture(pad: usize, revert: bool) -> Fixture { + let doc = LoroDoc::new(); + doc.set_peer_id(1).unwrap(); + let map = doc.get_map("map"); + map.insert("x", "root").unwrap(); + map.insert("untouched", "root").unwrap(); + let child = map.insert_container("child", LoroMap::new()).unwrap(); + child.insert("x", 1).unwrap(); + let sub = map.insert_container("sub", LoroMap::new()).unwrap(); + sub.insert("k", "v").unwrap(); + let mergeable = map.ensure_mergeable_map("merge").unwrap(); + mergeable.insert("x", "root").unwrap(); + let mlist = doc.get_movable_list("mlist"); + mlist.push("a").unwrap(); + mlist.push("b").unwrap(); + let list = doc.get_list("list"); + list.push("a").unwrap(); + let text = doc.get_text("text"); + text.insert(0, "ab").unwrap(); + let tree = doc.get_tree("tree"); + let n1 = tree.create(TreeParentId::Root).unwrap(); + let n2 = tree.create(TreeParentId::Root).unwrap(); + tree.get_meta(n1).unwrap().insert("x", "root").unwrap(); + let counter = doc.get_counter("counter"); + counter.increment(1.0).unwrap(); + doc.commit(); + let root = doc.oplog_frontiers(); + let mut versions = vec![root.clone()]; + + map.insert("x", "later").unwrap(); + map.insert("sub", "not a map").unwrap(); + child.insert("x", 2).unwrap(); + mergeable.insert("x", "later").unwrap(); + mlist.set(0, "later").unwrap(); + mlist.mov(0, 1).unwrap(); + list.delete(0, 1).unwrap(); + list.push("a").unwrap(); + text.delete(0, 1).unwrap(); + text.insert(0, "a").unwrap(); + tree.mov(n1, n2).unwrap(); + tree.get_meta(n1).unwrap().insert("x", "later").unwrap(); + counter.increment(2.0).unwrap(); + commit(&doc, &mut versions); + + let pad_map = doc.get_map("pad"); + for i in 0..pad { + pad_map.insert("k", i as i64).unwrap(); + } + commit(&doc, &mut versions); + + if revert { + doc.revert_to(&root).unwrap(); + } else { + map.insert("x", "root").unwrap(); + child.insert("x", 1).unwrap(); + mergeable.insert("x", "root").unwrap(); + mlist.mov(1, 0).unwrap(); + mlist.set(0, "a").unwrap(); + tree.mov(n1, TreeParentId::Root).unwrap(); + tree.get_meta(n1).unwrap().insert("x", "root").unwrap(); + counter.decrement(2.0).unwrap(); + } + commit(&doc, &mut versions); + map.insert("tail", 1).unwrap(); + commit(&doc, &mut versions); + + Fixture { + doc, + root, + versions, + } +} + +fn import(bytes: &[u8]) -> LoroDoc { + let doc = LoroDoc::new(); + doc.import(bytes).unwrap(); + doc +} + +fn value_at(doc: &LoroDoc, version: &Frontiers) -> LoroValue { + doc.checkout(version).unwrap(); + let value = doc.get_deep_value(); + doc.checkout_to_latest(); + value +} + +fn assert_same_history(full: &LoroDoc, shallow: &LoroDoc, versions: &[Frontiers], ctx: &str) { + assert_eq!( + shallow.get_deep_value(), + full.get_deep_value(), + "{ctx}: latest" + ); + for version in versions { + assert_eq!( + value_at(shallow, version), + value_at(full, version), + "{ctx}: checkout {version:?}" + ); + } + for from in versions { + for to in versions { + let applied = |doc: &LoroDoc| { + let diff = doc.diff(from, to).unwrap(); + let fork = full.fork_at(from).unwrap(); + fork.set_detached_editing(true); + fork.apply_diff(diff).unwrap(); + fork.get_deep_value() + }; + assert_eq!( + applied(shallow), + applied(full), + "{ctx}: diff {from:?} -> {to:?}" + ); + } + } +} + +#[test] +fn shallow_checkout_to_root_keeps_values_rewritten_after_root() { + for pad in PADS { + for revert in [false, true] { + let f = fixture(pad, revert); + let shallow = import(&f.doc.export(ExportMode::shallow_snapshot(&f.root)).unwrap()); + let ctx = format!("pad={pad} revert={revert}"); + assert_eq!( + value_at(&shallow, &f.root).to_json_value()["map"]["x"], + "root", + "{ctx}" + ); + assert_same_history(&f.doc, &shallow, &f.versions, &ctx); + } + } +} + +#[test] +fn shallow_reexport_keeps_values_rewritten_after_root() { + for pad in PADS { + let f = fixture(pad, true); + let mut shallow = import(&f.doc.export(ExportMode::shallow_snapshot(&f.root)).unwrap()); + for hop in 0..2 { + // Checking out first leaves the root-time state in the store, as + // an export that goes through a checkout would. + shallow.checkout(&f.root).unwrap(); + shallow.checkout_to_latest(); + shallow = import( + &shallow + .export(ExportMode::shallow_snapshot(&f.root)) + .unwrap(), + ); + assert_same_history( + &f.doc, + &shallow, + &f.versions, + &format!("pad={pad} hop={hop}"), + ); + } + // Re-export at a later root from a replica that is already shallow. + let later = &f.versions[1]; + let reexported = import(&shallow.export(ExportMode::shallow_snapshot(later)).unwrap()); + assert_same_history( + &f.doc, + &reexported, + &f.versions[1..], + &format!("pad={pad} later"), + ); + } +} + +#[test] +fn state_only_at_root_keeps_values_rewritten_after_root() { + for pad in PADS { + let f = fixture(pad, true); + let state_only = import(&f.doc.export(ExportMode::state_only(Some(&f.root))).unwrap()); + assert_eq!( + state_only.get_deep_value(), + value_at(&f.doc, &f.root), + "pad={pad}" + ); + // The state-only replica must carry the root-time winners: with the + // later writer's lamport, LWW would reject the next retained update. + let next = f.doc.fork_at(&f.versions[1]).unwrap(); + let root_vv = f.doc.frontiers_to_vv(&f.root).unwrap(); + state_only + .import(&next.export(ExportMode::updates(&root_vv)).unwrap()) + .unwrap(); + assert_eq!( + state_only.get_deep_value(), + next.get_deep_value(), + "pad={pad}" + ); + } +} + +#[test] +fn shallow_undo_matches_full_history() { + for pad in PADS { + let f = fixture(pad, true); + let full = import(&f.doc.export(ExportMode::Snapshot).unwrap()); + let shallow = import(&f.doc.export(ExportMode::shallow_snapshot(&f.root)).unwrap()); + let mut values = Vec::new(); + for doc in [&full, &shallow] { + doc.set_peer_id(2).unwrap(); + let mut undo = UndoManager::new(doc); + let map = doc.get_map("map"); + map.insert("x", "local").unwrap(); + doc.get_movable_list("mlist").set(0, "local").unwrap(); + doc.commit(); + undo.record_new_checkpoint().unwrap(); + undo.undo().unwrap(); + let after_undo = doc.get_deep_value(); + undo.redo().unwrap(); + values.push((after_undo, doc.get_deep_value())); + } + assert_eq!(values[1], values[0], "pad={pad}"); + assert_eq!(values[0].0.to_json_value()["map"]["x"], "root", "pad={pad}"); + } +} + +/// Peer 2 writes back the value peer 1 had at the root. Every replica that +/// materializes the root must report peer 1's op as the winner, not only its +/// value. +#[test] +fn checkout_reports_root_time_winner_of_equal_value() { + for pad in PADS { + let doc = LoroDoc::new(); + doc.set_peer_id(1).unwrap(); + let map = doc.get_map("map"); + let list = doc.get_movable_list("mlist"); + map.insert("x", "root").unwrap(); + map.insert("y", "root").unwrap(); + list.push("a").unwrap(); + list.push("b").unwrap(); + doc.commit(); + let root = doc.oplog_frontiers(); + map.insert("x", "later").unwrap(); + list.set(0, "later").unwrap(); + for i in 0..pad { + doc.get_map("pad").insert("k", i as i64).unwrap(); + } + doc.commit(); + doc.set_peer_id(2).unwrap(); + map.insert("x", "root").unwrap(); + list.set(0, "a").unwrap(); + doc.commit(); + + let full = import(&doc.export(ExportMode::Snapshot).unwrap()); + let shallow = import(&doc.export(ExportMode::shallow_snapshot(&root)).unwrap()); + let state_only = import(&doc.export(ExportMode::state_only(Some(&root))).unwrap()); + let expected = value_at(&full, &root); + for (name, replica) in [ + ("full", &full), + ("shallow", &shallow), + ("state-only", &state_only), + ] { + replica.checkout(&root).unwrap(); + let ctx = format!("pad={pad} {name}"); + assert_eq!( + replica.get_map("map").get_last_editor("x"), + Some(1), + "{ctx}" + ); + assert_eq!( + replica.get_movable_list("mlist").get_last_editor_at(0), + Some(1), + "{ctx}" + ); + assert_eq!(replica.get_deep_value(), expected, "{ctx}"); + replica.checkout_to_latest(); + // A state-only replica's latest version is the root itself. + let latest_writer = if name == "state-only" { 1 } else { 2 }; + assert_eq!( + replica.get_map("map").get_last_editor("x"), + Some(latest_writer), + "{ctx} latest" + ); + } + } +} From 1e465926169f50ab42ede962b3ea3bd6f341a72f Mon Sep 17 00:00:00 2001 From: Zixuan Chen Date: Mon, 28 Sep 2026 01:02:56 +0800 Subject: [PATCH 2/5] perf: compare map winners by op id and let the state detect equal values The previous commit fetched both the `from` and `to` values for every candidate key and then sent equal-value winner changes through a second state lookup, so equal-value checkout/diff/import regressed ~20%. Compare winners by (lamport, peer) and fetch only the `to` value; the state, which is at `from`, detects equal values in the same `insert`. See "Winner metadata, not just values" in docs/diff_calc.md; benchmark in crates/loro/examples/map_equal_value_bench.rs. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/loro-internal/docs/diff_calc.md | 18 +- crates/loro-internal/src/diff_calc.rs | 53 ++-- crates/loro-internal/src/history_cache.rs | 122 +++++---- crates/loro-internal/src/state/map_state.rs | 20 +- .../src/state/movable_list_state.rs | 24 +- crates/loro/examples/map_equal_value_bench.rs | 250 ++++++++++++++++++ 6 files changed, 361 insertions(+), 126 deletions(-) create mode 100644 crates/loro/examples/map_equal_value_bench.rs diff --git a/crates/loro-internal/docs/diff_calc.md b/crates/loro-internal/docs/diff_calc.md index 78a202281..9a1a9494b 100644 --- a/crates/loro-internal/docs/diff_calc.md +++ b/crates/loro-internal/docs/diff_calc.md @@ -83,10 +83,20 @@ their cost follows the update instead of the map size. Map and MovableList states store the winning op's lamport/peer (map entry, movable-list `value_id`) next to the value. A checkout must move that metadata even when both versions hold an equal value written by different ops (a value -rewritten after the target, e.g. by `revert_to`). `MapDiffCalculator` therefore -emits a key whenever the winning op differs, and `MapState` / -`MovableListState` apply an equal-value entry as a silent metadata update with -no event, so events still only report value changes. +rewritten after the target, e.g. by `revert_to`). + +`MapDiffCalculator` (Checkout/Import) compares the winners at `from` and `to` +by op id only (`MapHistoryCache::changed_winners_for_keys`): the same winner is +skipped without reading any value, and a different winner is emitted with the +`to` value. The `from` value is never fetched, because the state applying the +diff is already at `from` (the isolated-import fast path diffs from the empty +version, where every winner is new). `MapState` writes each entry with one +`insert` and reports a change only when the previous value differs, so an +equal-value entry is a silent metadata update and events still only report +value changes. `MovableListState` does the same for element values. This keeps +equal-value checkouts at the cost of `main` before the fix: the added state +write per key is paid for by the dropped `from` value lookup (loro-dev/loro#1124; +benchmark: `crates/loro/examples/map_equal_value_bench.rs`). Skipping those entries left the later op's metadata in the checked-out state. Shallow and state-only exports build their root state through such a checkout, diff --git a/crates/loro-internal/src/diff_calc.rs b/crates/loro-internal/src/diff_calc.rs index e5acb8fdc..8a6c4d58a 100644 --- a/crates/loro-internal/src/diff_calc.rs +++ b/crates/loro-internal/src/diff_calc.rs @@ -672,58 +672,35 @@ impl DiffCalculatorTrait for MapDiffCalculator { DiffMode::Checkout | DiffMode::Import => oplog.with_history_cache(|h| { h.ensure_shallow_map_seeded(self.container_idx); let checkout_index = &h.get_checkout_index().map; - let mut changed = Vec::new(); let keys = std::mem::take(&mut self.changed); - let from_map = checkout_index.get_container_latest_op_at_vv_for_keys( + // Every key whose winning op differs is emitted, even when both + // winners carry the same value: the state keeps the winner's + // lamport/peer, and a shallow root state encoded from stale + // metadata collides with a retained op in this index and loses + // the root entry. The state is at `from` and already holds the + // `from` value, so `MapState` tells value changes from + // metadata-only updates in the same write, and the `from` value + // is never fetched here. + let changed = checkout_index.changed_winners_for_keys( self.container_idx, from_vv, - keys.keys().cloned(), - oplog, - ); - let mut to_map = checkout_index.get_container_latest_op_at_vv_for_keys( - self.container_idx, to_vv, keys.into_keys(), oplog, ); - // A key whose winning op differs must be emitted even when both - // ops carry the same value: the state keeps the winner's - // lamport/peer, and a shallow root state encoded from it seeds - // the history cache. Stale metadata there collides with a - // retained op and loses the root entry. `MapState` applies such - // metadata-only entries without reporting a value change. - for (k, peek_from) in from_map.iter() { - let peek_to = to_map.remove(k); - match peek_to { - None => changed.push((k.clone(), None, true)), - Some(b) => { - if peek_from.value != b.value { - changed.push((k.clone(), Some(b), true)) - } else if peek_from.lamport != b.lamport || peek_from.peer != b.peer { - changed.push((k.clone(), Some(b), false)) - } - } - } - } - - for (k, peek_to) in to_map.into_iter() { - changed.push((k, Some(peek_to), true)); - } - let mut updated = FxHashMap::with_capacity_and_hasher(changed.len(), Default::default()); - for (key, value, value_changed) in changed { + for (key, value) in changed { let value = value.map(|v| { - let value = v.value.clone(); - if value_changed { - if let Some(LoroValue::Container(c)) = &value { - on_new_container(c); - } + // A normal container id is minted by one op, so a new + // winner holding a container always changes the value. + if let Some(LoroValue::Container(c)) = &v.value { + on_new_container(c); } MapValue { - value, + value: v.value, lamp: v.lamport, peer: v.peer, } diff --git a/crates/loro-internal/src/history_cache.rs b/crates/loro-internal/src/history_cache.rs index 35475b3ad..6769e7d5f 100644 --- a/crates/loro-internal/src/history_cache.rs +++ b/crates/loro-internal/src/history_cache.rs @@ -679,78 +679,84 @@ impl MapHistoryCache { }); } - /// Resolve the winning op at `vv` for a restricted set of keys. + /// The entry of the op that wins `key` at `vv`, if any. + fn latest_entry_at_vv( + &self, + container: ContainerIdx, + key: u32, + vv: &VersionVector, + ) -> Option<&MapHistoryCacheEntry> { + let bound = |lamport, peer| MapHistoryCacheEntry { + container, + key, + lamport, + peer, + counter_or_value: Either::Left(0), + }; + self.map + .range(bound(0, 0)..=bound(Lamport::MAX, PeerID::MAX)) + .rev() + .find(|entry| match &entry.counter_or_value { + Either::Left(cnt) => vv.get(&entry.peer).copied().unwrap_or(0) > *cnt, + // Shallow-root entries are part of every version the doc can reach. + Either::Right(_) => true, + }) + } + + fn entry_info(entry: &MapHistoryCacheEntry, oplog: &OpLog) -> GroupedMapOpInfo { + let value = match &entry.counter_or_value { + Either::Left(cnt) => { + let op = oplog + .get_op_that_includes(ID::new(entry.peer, *cnt)) + .unwrap(); + debug_assert_eq!(op.atom_len(), 1); + match &op.content { + InnerContent::Map(map) => map.value.clone(), + _ => unreachable!(), + } + } + Either::Right(v) => (**v).clone(), + }; + GroupedMapOpInfo { + value, + lamport: entry.lamport, + peer: entry.peer, + } + } + + /// For each key, the op that wins it at `to_vv` when that differs from the + /// op that wins it at `from_vv`; `None` when no op sets the key at `to_vv`. /// /// Every key whose value can differ between two versions must have been /// written by an op inside the replayed span, so a diff only ever needs the /// candidate keys instead of the whole container. This keeps map diffing /// proportional to the update instead of to the size of the map. - pub fn get_container_latest_op_at_vv_for_keys( + /// + /// Winners are compared by op id only, so only the `to` value is ever + /// fetched. A different winner can still carry an equal value; the caller's + /// state has the `from` value and tells the two cases apart (see + /// `MapDiffCalculator::calculate_diff`). + pub fn changed_winners_for_keys( &self, container: ContainerIdx, - vv: &VersionVector, + from_vv: &VersionVector, + to_vv: &VersionVector, keys: impl Iterator, oplog: &OpLog, - ) -> FxHashMap { - let mut ans = FxHashMap::default(); + ) -> Vec<(InternalString, Option)> { + let mut ans = Vec::new(); for key in keys { let Some(key_idx) = self.keys.get(&key) else { continue; }; - let key_idx = key_idx as u32; - let range = ( - Bound::Included(MapHistoryCacheEntry { - container, - key: key_idx, - lamport: 0, - peer: 0, - counter_or_value: Either::Left(0), - }), - Bound::Included(MapHistoryCacheEntry { - container, - key: key_idx, - lamport: Lamport::MAX, - peer: PeerID::MAX, - counter_or_value: Either::Left(0), - }), - ); - - for entry in self.map.range(range).rev() { - match &entry.counter_or_value { - Either::Left(cnt) => { - if vv.get(&entry.peer).copied().unwrap_or(0) > *cnt { - let id = ID::new(entry.peer, *cnt); - let op = oplog.get_op_that_includes(id).unwrap(); - debug_assert_eq!(op.atom_len(), 1); - match &op.content { - InnerContent::Map(map) => { - ans.insert( - key, - GroupedMapOpInfo { - value: map.value.clone(), - lamport: entry.lamport, - peer: entry.peer, - }, - ); - } - _ => unreachable!(), - } - break; - } - } - Either::Right(v) => { - ans.insert( - key, - GroupedMapOpInfo { - value: (**v).clone(), - lamport: entry.lamport, - peer: entry.peer, - }, - ); - break; - } - } + let from = self.latest_entry_at_vv(container, key_idx, from_vv); + let to = self.latest_entry_at_vv(container, key_idx, to_vv); + match (from, to) { + (None, None) => {} + (Some(_), None) => ans.push((key, None)), + (Some(a), Some(b)) if a.lamport == b.lamport && a.peer == b.peer => {} + (_, Some(b)) => ans.push((key, Some(Self::entry_info(b, oplog)))), } } diff --git a/crates/loro-internal/src/state/map_state.rs b/crates/loro-internal/src/state/map_state.rs index a615bb997..79005a95f 100644 --- a/crates/loro-internal/src/state/map_state.rs +++ b/crates/loro-internal/src/state/map_state.rs @@ -260,22 +260,14 @@ impl ContainerState for MapState { continue; }; - if mode == DiffMode::Checkout - && self - .map - .get(&key) - .is_some_and(|old| old.value == value.value) - { - // Only the winning op changed (see `MapDiffCalculator`): keep the - // lamport/peer accurate without reporting a value change. - self.insert(key, value); - continue; - } - let mut changed = false; if force { - self.insert(key.clone(), value.clone()); - changed = true; + let prev = self.insert(key.clone(), value.clone()); + // A checkout diff carries every key whose winning op changed, + // including ones whose value did not (see `MapDiffCalculator`): + // those only refresh the lamport/peer and report no change. + changed = mode != DiffMode::Checkout + || !matches!(prev, Some(prev) if prev.value == value.value); } else { match self.map.get(&key) { Some(old_value) if old_value > &value => {} diff --git a/crates/loro-internal/src/state/movable_list_state.rs b/crates/loro-internal/src/state/movable_list_state.rs index 5b2b75650..119b89a56 100644 --- a/crates/loro-internal/src/state/movable_list_state.rs +++ b/crates/loro-internal/src/state/movable_list_state.rs @@ -1133,18 +1133,18 @@ impl ContainerState for MovableListState { match self.inner.elements().get(&elem_id).cloned() { Some(elem) => { // Update value if needed - if let Some(value_id) = value_id { - if elem.value == value - && elem.value_id != value_id - && (!need_compare || elem.value_id < value_id) - { - // Only the winning set op changed (e.g. a checkout - // past a later set of the same value): keep the - // value id accurate without an event, as for maps. - self.inner.update_value(elem_id, value.clone(), value_id); - } else if elem.value != value - && (!need_compare || elem.value_id < value_id) - { + if let Some(value_id) = + value_id.filter(|value_id| !need_compare || elem.value_id < *value_id) + { + if elem.value == value { + // Only the winning set op may have changed (e.g. a + // checkout past a later set of the same value): + // keep the value id accurate without an event, as + // for maps. + if elem.value_id != value_id { + self.inner.update_value(elem_id, value, value_id); + } + } else { maybe_moved.remove(&elem_id); self.inner.update_value(elem_id, value.clone(), value_id); let index = self.get_index_of_elem(elem_id); diff --git a/crates/loro/examples/map_equal_value_bench.rs b/crates/loro/examples/map_equal_value_bench.rs new file mode 100644 index 000000000..4618f8852 --- /dev/null +++ b/crates/loro/examples/map_equal_value_bench.rs @@ -0,0 +1,250 @@ +//! Checkout/diff/import/undo cost when map keys and movable-list items move +//! between versions whose values are equal but whose winning ops differ +//! (loro-dev/loro#1124). Every map scenario uses 100k keys written 0 -> 1 -> 0. +//! +//! ```sh +//! cargo run -p loro --release --example map_equal_value_bench +//! ``` +//! +//! Prints `scenario,median_ms` per line (medians of the timed iterations after +//! warm-up; setup, source export and fresh-doc imports are outside timers). To +//! compare two revisions, build this example on each and run the binaries +//! alternately several times on the same machine, then take the median per +//! scenario. `BENCH_FILTER=` runs only matching scenarios and +//! `BENCH_ITERS=` overrides the number of timed iterations (e.g. to profile). + +use loro::{ExportMode, Frontiers, LoroDoc, UndoManager, VersionVector}; +use std::{hint::black_box, time::Instant}; + +const KEYS: usize = 100_000; +const LIST: usize = 20_000; +const WARMUP: usize = 2; +const DEFAULT_ITERS: usize = 9; + +fn load(bytes: &[u8]) -> LoroDoc { + let doc = LoroDoc::new(); + doc.import(bytes).unwrap(); + doc +} + +fn median(mut v: Vec) -> f64 { + v.sort_by(|a, b| a.partial_cmp(b).unwrap()); + v[v.len() / 2] +} + +/// Runs `setup` then times `run` on its output, `WARMUP + ITERS` times. +fn bench(name: &str, mut setup: impl FnMut() -> S, mut run: impl FnMut(S)) { + if std::env::var("BENCH_FILTER").is_ok_and(|f| !name.contains(&f)) { + return; + } + let iters = std::env::var("BENCH_ITERS") + .ok() + .and_then(|n| n.parse().ok()) + .unwrap_or(DEFAULT_ITERS); + let mut times = Vec::with_capacity(iters); + for i in 0..WARMUP + iters { + let input = setup(); + let start = Instant::now(); + run(input); + let elapsed = start.elapsed().as_secs_f64() * 1000.0; + if i >= WARMUP { + times.push(elapsed); + } + } + println!("{name},{:.3}", median(times)); +} + +struct MapFixture { + doc: LoroDoc, + /// Every key is 0. + a: Frontiers, + /// Every key is 1. + b: Frontiers, + /// Every key is 0 again, written by newer ops. + c: Frontiers, + base: Vec, + /// Updates from `a` to `b`, and from `a` to `c`. + to_b: Vec, + to_c: Vec, +} + +fn map_fixture() -> MapFixture { + let doc = LoroDoc::new(); + doc.set_peer_id(1).unwrap(); + let map = doc.get_map("m"); + let keys: Vec<_> = (0..KEYS).map(|i| format!("key{i:06}")).collect(); + for k in &keys { + map.insert(k, 0).unwrap(); + } + doc.commit(); + let a = doc.oplog_frontiers(); + let a_vv: VersionVector = doc.oplog_vv(); + let base = doc.export(ExportMode::Snapshot).unwrap(); + for k in &keys { + map.insert(k, 1).unwrap(); + } + doc.commit(); + let b = doc.oplog_frontiers(); + let to_b = doc + .fork_at(&b) + .unwrap() + .export(ExportMode::updates(&a_vv)) + .unwrap(); + for k in &keys { + map.insert(k, 0).unwrap(); + } + doc.commit(); + let c = doc.oplog_frontiers(); + let to_c = doc.export(ExportMode::updates(&a_vv)).unwrap(); + MapFixture { + doc, + a, + b, + c, + base, + to_b, + to_c, + } +} + +fn concurrent_receiver(base: &[u8]) -> LoroDoc { + let doc = load(base); + doc.set_peer_id(2).unwrap(); + doc.get_map("m").insert("unrelated", 0).unwrap(); + doc.commit(); + black_box(doc.get_map("m").get_value()); + doc +} + +fn map_benches() { + let f = map_fixture(); + let d = &f.doc; + for (label, target) in [("equal", &f.c), ("changed", &f.b)] { + bench( + &format!("map_checkout_roundtrip_{label}"), + || (), + |_| { + d.checkout(&f.a).unwrap(); + d.checkout(target).unwrap(); + }, + ); + bench( + &format!("map_diff_{label}"), + || (), + |_| { + black_box(d.diff(&f.a, target).unwrap()); + }, + ); + } + d.checkout_to_latest(); + for (label, updates) in [("equal", &f.to_c), ("changed", &f.to_b)] { + bench( + &format!("map_import_linear_{label}"), + || { + let doc = load(&f.base); + black_box(doc.get_map("m").get_value()); + doc + }, + |doc| doc.import(updates).map(|_| ()).unwrap(), + ); + bench( + &format!("map_import_concurrent_{label}"), + || concurrent_receiver(&f.base), + |doc| doc.import(updates).map(|_| ()).unwrap(), + ); + } + + let keys: Vec<_> = (0..KEYS).map(|i| format!("key{i:06}")).collect(); + let undo_setup = || { + let doc = load(&f.base); + doc.set_peer_id(3).unwrap(); + let mut undo = UndoManager::new(&doc); + undo.set_merge_interval(0); + let map = doc.get_map("m"); + for k in &keys { + map.insert(k, 1).unwrap(); + } + doc.commit(); + undo.record_new_checkpoint().unwrap(); + (doc, undo) + }; + bench("map_undo", undo_setup, |(_doc, mut undo)| { + black_box(undo.undo().unwrap()); + }); + bench( + "map_redo", + || { + let (doc, mut undo) = undo_setup(); + undo.undo().unwrap(); + (doc, undo) + }, + |(_doc, mut undo)| { + black_box(undo.redo().unwrap()); + }, + ); +} + +fn movable_list_benches() { + let doc = LoroDoc::new(); + doc.set_peer_id(1).unwrap(); + let list = doc.get_movable_list("l"); + for _ in 0..LIST { + list.push(0).unwrap(); + } + doc.commit(); + let a = doc.oplog_frontiers(); + let base = doc.export(ExportMode::Snapshot).unwrap(); + let a_vv = doc.oplog_vv(); + for i in 0..LIST { + list.set(i, 1).unwrap(); + } + doc.commit(); + let b = doc.oplog_frontiers(); + let to_b = doc.export(ExportMode::updates(&a_vv)).unwrap(); + for i in 0..LIST { + list.set(i, 0).unwrap(); + } + doc.commit(); + let c = doc.oplog_frontiers(); + + bench( + "movable_set_local", + || { + let doc = load(&base); + let list = doc.get_movable_list("l"); + black_box(list.get_value()); + doc + }, + |doc| { + let list = doc.get_movable_list("l"); + for i in 0..LIST { + list.set(i, 1).unwrap(); + } + doc.commit(); + }, + ); + bench( + "movable_import_set", + || { + let doc = load(&base); + black_box(doc.get_movable_list("l").get_value()); + doc + }, + |doc| doc.import(&to_b).map(|_| ()).unwrap(), + ); + for (label, target) in [("equal", &c), ("changed", &b)] { + bench( + &format!("movable_checkout_roundtrip_{label}"), + || (), + |_| { + doc.checkout(&a).unwrap(); + doc.checkout(target).unwrap(); + }, + ); + } +} + +fn main() { + map_benches(); + movable_list_benches(); +} From e6a0ef8b1df784300171afef761ac2d364d137c6 Mon Sep 17 00:00:00 2001 From: Zixuan Chen Date: Mon, 28 Sep 2026 01:03:41 +0800 Subject: [PATCH 3/5] test: equal-value winner changes emit no events Co-Authored-By: Claude Opus 5.5 (1M context) --- .../tests/shallow_checkout_equal_value.rs | 63 +++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/crates/loro/tests/shallow_checkout_equal_value.rs b/crates/loro/tests/shallow_checkout_equal_value.rs index 18628e864..c1109a243 100644 --- a/crates/loro/tests/shallow_checkout_equal_value.rs +++ b/crates/loro/tests/shallow_checkout_equal_value.rs @@ -307,3 +307,66 @@ fn checkout_reports_root_time_winner_of_equal_value() { } } } + +/// Refreshing the winner of an equal value is a state-only update: checkout, +/// `diff` and import must not report it as a change. +#[test] +fn equal_value_winner_change_emits_no_events() { + let doc = LoroDoc::new(); + doc.set_peer_id(1).unwrap(); + let map = doc.get_map("map"); + let list = doc.get_movable_list("mlist"); + map.insert("x", "same").unwrap(); + map.insert("y", "old").unwrap(); + list.push("same").unwrap(); + doc.commit(); + let a = doc.oplog_frontiers(); + let a_vv = doc.oplog_vv(); + let base = doc.export(ExportMode::Snapshot).unwrap(); + map.insert("x", "later").unwrap(); + list.set(0, "later").unwrap(); + doc.commit(); + map.insert("x", "same").unwrap(); + map.insert("y", "new").unwrap(); + list.set(0, "same").unwrap(); + doc.commit(); + let c = doc.oplog_frontiers(); + + let events = std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); + let sink = events.clone(); + let _sub = doc.subscribe_root(std::sync::Arc::new(move |e| { + for ev in e.events { + sink.lock().unwrap().push(format!("{:?}", ev.diff)); + } + })); + let changed_only = |events: &[String]| { + assert!(events.iter().all(|e| !e.contains("same")), "{events:?}"); + assert!(events.iter().any(|e| e.contains("new") || e.contains("old"))); + }; + doc.checkout(&a).unwrap(); + doc.checkout(&c).unwrap(); + changed_only(&events.lock().unwrap()); + let diff = format!("{:?}", doc.diff(&a, &c).unwrap()); + assert!(!diff.contains("same") && diff.contains("new"), "{diff}"); + + let receiver = import(&base); + receiver.set_peer_id(2).unwrap(); + receiver.get_map("map").insert("unrelated", 1).unwrap(); + receiver.commit(); + let received = std::sync::Arc::new(std::sync::Mutex::new(Vec::new())); + let sink = received.clone(); + let _sub = receiver.subscribe_root(std::sync::Arc::new(move |e| { + for ev in e.events { + sink.lock().unwrap().push(format!("{:?}", ev.diff)); + } + })); + receiver + .import(&doc.export(ExportMode::updates(&a_vv)).unwrap()) + .unwrap(); + changed_only(&received.lock().unwrap()); + assert_eq!(receiver.get_map("map").get_last_editor("x"), Some(1)); + assert_eq!( + receiver.get_map("map").get("x").unwrap().into_value().unwrap(), + LoroValue::from("same") + ); +} From 722777ded7ff75aaa89adfd310a3280788a106da Mon Sep 17 00:00:00 2001 From: Zixuan Chen Date: Mon, 28 Sep 2026 03:05:56 +0800 Subject: [PATCH 4/5] bench: isolated local MovableList set benchmark Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/loro/examples/movable_set_bench.rs | 82 +++++++++++++++++++++++ 1 file changed, 82 insertions(+) create mode 100644 crates/loro/examples/movable_set_bench.rs diff --git a/crates/loro/examples/movable_set_bench.rs b/crates/loro/examples/movable_set_bench.rs new file mode 100644 index 000000000..869f4e70e --- /dev/null +++ b/crates/loro/examples/movable_set_bench.rs @@ -0,0 +1,82 @@ +//! Local `MovableList::set` + commit cost, one scenario per process +//! (loro-dev/loro#1124 review). +//! +//! ```sh +//! cargo run -p loro --release --example movable_set_bench -- cold # fresh 20k list, first set pass +//! cargo run -p loro --release --example movable_set_bench -- seq # 100k list, repeated set passes +//! ``` +//! +//! `cold` builds a fresh document per sample (push 20k items + commit) and +//! times only the first pass of 20k `set` calls plus the commit; the document +//! is dropped after the timer stops. `seq` times 20 consecutive passes of 100k +//! `set` calls on one document and skips the first 2. Prints +//! `scenario,median_ms,samples...`. + +use loro::LoroDoc; +use std::time::Instant; + +fn median(v: &[f64]) -> f64 { + let mut v = v.to_vec(); + v.sort_by(|a, b| a.partial_cmp(b).unwrap()); + v[v.len() / 2] +} + +fn report(name: &str, samples: &[f64]) { + let all: Vec<_> = samples.iter().map(|s| format!("{s:.3}")).collect(); + println!("{name},{:.3},{}", median(samples), all.join(",")); +} + +fn cold() { + const N: usize = 20_000; + const SAMPLES: usize = 15; + let mut samples = Vec::with_capacity(SAMPLES); + for _ in 0..SAMPLES { + let doc = LoroDoc::new(); + doc.set_peer_id(1).unwrap(); + let list = doc.get_movable_list("l"); + for _ in 0..N { + list.push(0).unwrap(); + } + doc.commit(); + let start = Instant::now(); + for i in 0..N { + list.set(i, 1).unwrap(); + } + doc.commit(); + samples.push(start.elapsed().as_secs_f64() * 1000.0); + drop(list); + drop(doc); + } + report("movable_set_local_cold_20k", &samples); +} + +fn seq() { + const N: usize = 100_000; + const ROUNDS: usize = 20; + const SKIP: usize = 2; + let doc = LoroDoc::new(); + doc.set_peer_id(1).unwrap(); + let list = doc.get_movable_list("l"); + for _ in 0..N { + list.push(0).unwrap(); + } + doc.commit(); + let mut samples = Vec::with_capacity(ROUNDS); + for round in 0..ROUNDS { + let start = Instant::now(); + for i in 0..N { + list.set(i, round as i64 + 1).unwrap(); + } + doc.commit(); + samples.push(start.elapsed().as_secs_f64() * 1000.0); + } + report("movable_set_local_seq_100k", &samples[SKIP..]); +} + +fn main() { + match std::env::args().nth(1).as_deref() { + Some("cold") => cold(), + Some("seq") => seq(), + _ => panic!("usage: movable_set_bench cold|seq"), + } +} From ae18063c1c9150cad2dee2c4a5b02fee6e04809c Mon Sep 17 00:00:00 2001 From: Zixuan Chen Date: Mon, 28 Sep 2026 03:17:40 +0800 Subject: [PATCH 5/5] bench: allow overriding movable set bench rounds Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/loro/examples/movable_set_bench.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/crates/loro/examples/movable_set_bench.rs b/crates/loro/examples/movable_set_bench.rs index 869f4e70e..1a7500062 100644 --- a/crates/loro/examples/movable_set_bench.rs +++ b/crates/loro/examples/movable_set_bench.rs @@ -9,7 +9,8 @@ //! `cold` builds a fresh document per sample (push 20k items + commit) and //! times only the first pass of 20k `set` calls plus the commit; the document //! is dropped after the timer stops. `seq` times 20 consecutive passes of 100k -//! `set` calls on one document and skips the first 2. Prints +//! `set` calls on one document and skips the first 2 (`BENCH_ROUNDS=` +//! overrides the pass count, e.g. to profile). Prints //! `scenario,median_ms,samples...`. use loro::LoroDoc; @@ -52,8 +53,11 @@ fn cold() { fn seq() { const N: usize = 100_000; - const ROUNDS: usize = 20; const SKIP: usize = 2; + let rounds: usize = std::env::var("BENCH_ROUNDS") + .ok() + .and_then(|n| n.parse().ok()) + .unwrap_or(20); let doc = LoroDoc::new(); doc.set_peer_id(1).unwrap(); let list = doc.get_movable_list("l"); @@ -61,8 +65,8 @@ fn seq() { list.push(0).unwrap(); } doc.commit(); - let mut samples = Vec::with_capacity(ROUNDS); - for round in 0..ROUNDS { + let mut samples = Vec::with_capacity(rounds); + for round in 0..rounds { let start = Instant::now(); for i in 0..N { list.set(i, round as i64 + 1).unwrap();