[zxch3n]

Summary

A shallow replica could lose a map value when checking out its shallow root. This happened when the key was rewritten after the root with the value it had at the root (for example by revert_to) and the op that wrote the root value was trimmed. The trim happens whenever the root commit holds more than one op. Found by Codex while reviewing #1120; it also fails on ad5b2a6d (the base of #1120), so #1120 did not introduce it.

Root cause

MapDiffCalculator::calculate_diff (Checkout/Import) skipped a key when from.value == to.value, even when a different op won. After checkout(root) the state therefore kept the later op's lamport/peer next to the root-time value. MovableListState had the same skip for value_id.

Shallow and state-only exports build their root state through that checkout, so the stale metadata was encoded into the shallow-root state. On import, ensure_shallow_map_seeded inserted the root entry into the map checkout index. That index is a BTreeSet keyed by (container, key, lamport, peer), so the entry compared equal to the retained later op and was dropped. With no candidate at or below the root version, checkout(root) removed the key.

The same stale metadata made get_last_editor / get_last_editor_at wrong after a checkout on a full-history document, and state-only snapshots recorded the wrong writer.

Fix

• MapDiffCalculator also emits a key when the winning op changed but the value is equal.
• MapState and MovableListState apply such an entry as a silent metadata update with no event entry, so events still report only value changes.
• MapState::insert keeps the child-container edge when the same container is re-inserted, which this new path makes reachable.

List, Text and Counter carry no per-value winner metadata. Tree moves always get a new position, so there is no equal-state case; the new tests cover Tree too.

The design note is in crates/loro-internal/docs/diff_calc.md ("Winner metadata, not just values"), with a pointer from context/internal-encoding.md.

Known limitation: blobs exported by older versions already contain the wrong metadata in their root state. They still lose the value on checkout until they are exported again from a full-history document.

Validation

New crates/loro/tests/shallow_checkout_equal_value.rs. Each test runs with 0 and 300 padding ops, which covers both the ≤256 path and the >256 latest-state-overlay path. The tests compare shallow replicas with a full-history replica across Map, nested/mergeable maps, MovableList, List, Text, Tree and Counter:

• checkout at every retained version, and diff for every version pair (applied to a fork);
• shallow re-export (2 hops at the same root, plus one at a later root);
• state-only at root, followed by importing the next retained update;
• undo/redo on a shallow vs a full replica;
• a two-peer case asserting get_last_editor / get_last_editor_at after checkout on full, shallow and state-only replicas.

The checkout and re-export tests fail without the fix. The state-only and undo tests pass on main too and are kept as coverage.

• cargo test -p loro --test shallow_checkout_equal_value --test shallow_root_critical --test shallow_snapshot_concurrency --test shallow_snapshot_deleted_containers --test shallow_lazy_map_checkout_index: passing
• cargo test -p loro -p loro-internal: passing
• cargo fmt --all, pnpm check (clippy -Dwarnings): clean

🤖 Generated with Claude Code

[github-actions]

WASM Size Report

• Original size: 3210.11 KB
• Gzipped size: 1062.59 KB
• Brotli size: 744.13 KB

[zxch3n]

Perf follow-up to the adversarial review (743240a → 1e46592, e6a0ef8)

Problem

743240a made every map key whose winning op changed flow through the generic state apply, even when the value stayed the same. It also still fetched both the from and the to value of every candidate key from the oplog (get_op_that_includes), and MapState then did a get followed by an insert: two B-tree lookups. In main an all-equal map produced an empty diff and skipped the state entirely, so equal-value checkout, diff() and concurrent import regressed by roughly 20–50%.

Fix (1e46592)

• MapHistoryCache::changed_winners_for_keys compares the from and to winners by op id (lamport, peer) only. A key with the same winner is skipped without reading any value. A key with a different winner is emitted with only the to value fetched. The from value is never read from the oplog: the state applying a Checkout/Import diff is already at from (the isolated-import fast path diffs from the empty version, where every winner is new).
• MapState handles each entry with a single insert and uses the returned previous value to decide whether it was a value change (event) or a metadata-only refresh (no event). That is one B-tree lookup per key, the same as a changed key in main.
• MovableListState checks the value once per element; metadata-only updates skip the write when the id already matches.
• Public diff() is left alone deliberately: it works by checking the live state out to a and then b and back, so the metadata writes are part of keeping that state correct. With the fix above its equal-value cost is already at or below main, so a special path would add risk for no gain.

Dropping the from-value fetch also makes changed-value checkouts, diff(), concurrent imports and undo/redo faster than main.

Numbers

Bench: crates/loro/examples/map_equal_value_bench.rs (100k map keys written 0→1→0, 20k movable-list items; BENCH_FILTER / BENCH_ITERS env vars). Each binary prints the median of 9 timed iterations after 2 warm-ups. The runner alternates the binaries every round (reversing the order each round) on one machine and takes the median across rounds. Release build, Apple silicon. The machine is shared with other sessions, so I report the runs taken at the lowest load.

Full suite, 5 alternating rounds, load avg 6.7 → 2.7 (ms):

scenario	main (f91d727)	743240a	this PR (1e46592)	743240a vs main	PR vs main
map checkout round trip, equal	185.21	254.29	187.25	+37.3%	+1.1%
map diff(), equal	183.33	272.96	165.56	+48.9%	−9.7%
map import concurrent, equal	145.15	174.88	148.06	+20.5%	+2.0%
map checkout round trip, changed	233.85	269.08	178.02	+15.1%	−23.9%
map diff(), changed	232.22	285.42	194.21	+22.9%	−16.4%
map import concurrent, changed	141.70	149.05	113.51	+5.2%	−19.9%
map import linear, equal	71.89	78.63	83.80	+9.4%	+16.6%¹
map import linear, changed	61.83	60.20	61.03	−2.6%	−1.3%
undo 100k keys	328.42	365.44	263.61	+11.3%	−19.7%
redo 100k keys	289.94	332.38	230.75	+14.6%	−20.4%
movable set, local	4.29	4.36	4.57	+1.5%	+6.4%¹
movable import set	18.17	19.10	19.08	+5.1%	+5.0%¹
movable checkout round trip, equal	21.40	24.66	21.86	+15.2%	+2.2%
movable checkout round trip, changed	48.96	58.51	57.29	+19.5%	+17.0%¹

¹ These paths don't touch the changed calculator code, and their sign flipped between full-suite runs. Focused re-runs with more samples:

focused re-run (main vs PR)	rounds × iters	main	PR	Δ
map import linear, equal	16 × 20	82.33	81.99	−0.4%
map import linear, changed	16 × 20	68.77	69.94	+1.7%
movable import set	20 × 40	18.24	18.20	−0.2%
movable checkout round trip, changed	12 × 25	58.03	57.87	−0.3%
movable checkout round trip, equal	12 × 25	22.93	23.55	+2.7%
movable set, local	12 × 25	4.50	4.52	+0.3%

A focused run of only the equal scenarios (10 rounds × 15 iters) also has the PR at or below main: checkout −46.9%, diff −18.8%, concurrent import −15.0%, linear import −1.2%. Load climbed to ~24 during that run, so its absolute times are inflated and I don't use them as the headline numbers.

Correctness

• New test equal_value_winner_change_emits_no_events (e6a0ef8): equal-value checkout, diff() and concurrent import report only the real value changes. Mutating MapState to always report a change makes it fail.
• shallow_checkout_equal_value (6 tests), shallow_root_critical, shallow_snapshot_concurrency, shallow_snapshot_deleted_containers, shallow_lazy_map_checkout_index: pass.
• cargo test -p loro -p loro-internal: pass (44 test binaries, 0 failures). pnpm check: clean.
• The design note in crates/loro-internal/docs/diff_calc.md ("Winner metadata, not just values") now describes the op-id comparison and the state-side equality check.

🤖 Generated with Claude Code

[zxch3n]

Round-2 follow-up: movable_set_local_20k (+22.1%) is measurement noise, not a regression

Conclusion: local MovableList set + commit does not run any code this PR changes. An isolated low-load re-measurement shows no difference: cold 20k −1.6%, sequential 100k +0.4%. No code fix was needed.

Why the path is unaffected

• The PR's only MovableList change is one hunk in MovableListState::apply_diff_and_convert (diff application: checkout/import). Local ops go through MovableListState::apply_local_op → InnerState::update_value, which is unchanged. The map/history-cache changes are not on this path either.
• sample of the PR binary running 400 × 100k local sets (line-tables build): the only symbol on the path matching the PR's touched names is InnerState::update_value (unchanged); apply_diff_and_convert, changed_winners_for_keys, MapDiffCalculator, MovableListDiffCalculator, and MapState::insert do not appear. Top frames: MovableListHandler::set, the generic_btree lookup, Transaction::apply_local_op, and the mutex lock/unlock.
• The reviewer's movable_set_local_20k is one ~3ms sample taken in the same process right after the 100k-key map scenarios, so heap/allocator state from earlier scenarios (which differ between the two binaries) carries into it.

Isolated re-measurement

• New crates/loro/examples/movable_set_bench.rs runs one scenario per process:
  • cold: 15 fresh docs per process. Each builds 20k items and commits, then only the first set pass + commit is timed; the doc is dropped after the timer stops.
  • seq: 20 consecutive passes of 100k sets on one doc; the first 2 are dropped.
• Binaries were built from origin/main 106d9e59 and this branch (787963cb, which merges main incl. fix: drop dead containers when re-exporting a shallow doc at its cached root #1123/fix(loro.js): export the latest state in a snapshot of a detached document #1130), with separate CARGO_TARGET_DIRs.
• 12 alternating main/PR pairs (order flipped every pair). Runs were gated on 1-minute loadavg < 4; the loadavg recorded at each run was 3.66–3.95.

scenario (ms, median of 12 pairs)	main	PR	Δ	pairs where PR faster
cold 20k, per-process median	2.676	2.633	−1.6%	8/12
cold 20k, first sample of each process (truly cold)	3.013	2.947	−2.2%	—
sequential 100k set passes	17.811	17.876	+0.4%	5/12

Raw per-pair values:

• cold main 2.69 2.62 3.02 2.56 2.69 2.60 2.67 5.21 2.84 2.55 2.56 2.69
• cold PR 2.62 2.54 2.78 2.63 2.64 2.85 2.60 2.74 2.76 2.59 2.92 2.62
• seq main 17.15 19.01 22.20 18.05 17.78 18.10 15.63 22.37 17.84 15.63 17.43 15.76
• seq PR 18.62 17.06 17.42 18.20 19.75 17.96 17.79 27.41 15.57 15.67 18.64 15.58

Single outliers (5.21 on main, 27.41 on PR) show up on both sides; this is the same kind of jitter that produced the single-sample +22%.

Branch / CI

• Merged origin/main (106d9e59: fix: drop dead containers when re-exporting a shallow doc at its cached root #1123 84885677, fix(loro.js): export the latest state in a snapshot of a detached document #1130) into the branch with no conflicts.
• Locally, on the merged branch: cargo test -p loro -p loro-internal passes (45 test binaries, 0 failures); pnpm check is clean.
• CI build passes on the head commit ae18063c (and on 722777de before it).

🤖 Generated with Claude Code