Skip to content

Fix review follow-ups for Rust APIs and benches #935

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
zxch3n merged 2 commits into from
Mar 23, 2026
Merged

Fix review follow-ups for Rust APIs and benches #935

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c3fb676
Fix review follow-ups for Rust APIs and benches
zxch3n Mar 23, 2026
ab82b99
Restore public Block::decode API
zxch3n Mar 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions crates/kv-store/src/block.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,15 +195,15 @@ impl Block {
key: Bytes,
compression_type: CompressionType,
) -> Self {
// we have checked the checksum, so the block should be valid when decompressing
// The caller is responsible for validating SSTable integrity before lazy block reads.
if is_large {
return LargeValueBlock::decode(raw_block_and_check, key, compression_type)
.map(Block::Large)
.unwrap();
.expect("validated SSTable block should decode");
}
NormalBlock::decode(raw_block_and_check, key, compression_type)
.map(Block::Normal)
.unwrap()
.expect("validated SSTable block should decode")
}

pub fn len(&self) -> usize {
Expand Down
2 changes: 2 additions & 0 deletions crates/kv-store/src/mem_store.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,8 @@ impl MemKvStore {
}

/// We can import several times, the latter will override the former.
///
/// The caller is expected to validate blob integrity before passing bytes here.
pub fn import_all(&mut self, bytes: Bytes) -> Result<(), String> {
if bytes.is_empty() {
return Ok(());
Expand Down
2 changes: 1 addition & 1 deletion crates/kv-store/src/sstable.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -449,7 +449,7 @@ impl SsTable {
.get_or_insert_with(&block_idx, || {
Ok::<_, LoroError>(self.read_block(block_idx))
})
.unwrap()
.expect("validated SSTable block cache insert should succeed")
}

pub fn contains_key(&self, key: &[u8]) -> bool {
Expand Down
10 changes: 6 additions & 4 deletions crates/loro-internal/benches/encode.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ mod sync {
let c1 = LoroDoc::new();
c1.set_peer_id(1).unwrap();
let c2 = LoroDoc::new();
c1.set_peer_id(2).unwrap();
c2.set_peer_id(2).unwrap();
let t1 = c1.get_text("text");
let t2 = c2.get_text("text");
for (i, action) in actions.iter().enumerate() {
Expand Down Expand Up @@ -52,7 +52,7 @@ mod sync {
mod run {
use super::*;
use bench_utils::TextAction;
use loro_internal::{encoding::ExportMode, LoroDoc};
use loro_internal::{cursor::PosType, encoding::ExportMode, LoroDoc};

pub fn b4(c: &mut Criterion) {
let loro = LoroDoc::default();
Expand All @@ -63,8 +63,10 @@ mod run {
let text = loro.get_text("text");
for TextAction { pos, ins, del } in actions.iter() {
let mut txn = loro.txn().unwrap();
text.delete_with_txn(&mut txn, *pos, *del).unwrap();
text.insert_with_txn(&mut txn, *pos, ins).unwrap();
text.delete_with_txn(&mut txn, *pos, *del, PosType::Unicode)
.unwrap();
text.insert_with_txn(&mut txn, *pos, ins, PosType::Unicode)
.unwrap();
}
ran = true;
}
Expand Down
2 changes: 2 additions & 0 deletions crates/loro-internal/benches/list.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ mod run {
use super::*;
use arbitrary::Arbitrary;
use arbitrary::Unstructured;
use loro_internal::cursor::PosType;
use loro_internal::loro::ExportMode;
use loro_internal::LoroDoc;
use loro_internal::LoroValue;
Expand Down Expand Up @@ -44,6 +45,7 @@ mod run {
&mut txn,
(action.pos as usize) % text.len_unicode().max(1),
action.value.to_string().as_str(),
PosType::Unicode,
)
.unwrap();
} else {
Expand Down
10 changes: 5 additions & 5 deletions crates/loro-internal/benches/pending.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use criterion::{criterion_group, criterion_main, Criterion};
mod pending {
use super::*;
use bench_utils::TextAction;
use loro_internal::{LoroDoc, VersionVector};
use loro_internal::{cursor::PosType, loro::ExportMode, LoroDoc, VersionVector};

pub fn b4(c: &mut Criterion) {
let mut b = c.benchmark_group("B4 pending decode");
Expand All @@ -18,11 +18,11 @@ mod pending {
for chunks in actions.chunks(action_length / 5) {
for TextAction { pos, ins, del } in chunks {
let mut txn = loro.txn().unwrap();
text.delete_with_txn(&mut txn, *pos, *del).unwrap();
text.insert_with_txn(&mut txn, *pos, ins).unwrap();
updates
.push(loro.export(ExportMode::updates(&latest_vv)))
text.delete_with_txn(&mut txn, *pos, *del, PosType::Unicode)
.unwrap();
text.insert_with_txn(&mut txn, *pos, ins, PosType::Unicode)
.unwrap();
updates.push(loro.export(ExportMode::updates(&latest_vv)).unwrap());
latest_vv = loro.oplog_vv();
}
}
Expand Down
112 changes: 76 additions & 36 deletions crates/loro-internal/benches/text_r.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@ use criterion::{criterion_group, criterion_main, Criterion};

#[cfg(feature = "test_utils")]
mod run {
use std::hint::black_box;
use std::sync::Arc;

use super::*;
use bench_utils::TextAction;
use criterion::black_box;
use loro_common::LoroValue;
use loro_internal::{loro::ExportMode, LoroDoc};
use loro_internal::{cursor::PosType, loro::ExportMode, LoroDoc};

pub fn b4(c: &mut Criterion) {
let actions = bench_utils::get_automerge_actions();
Expand All @@ -21,8 +21,10 @@ mod run {
let mut txn = loro.txn().unwrap();

for TextAction { pos, ins, del } in actions.iter() {
text.delete_with_txn(&mut txn, *pos, *del).unwrap();
text.insert_with_txn(&mut txn, *pos, ins).unwrap();
text.delete_with_txn(&mut txn, *pos, *del, PosType::Unicode)
.unwrap();
text.insert_with_txn(&mut txn, *pos, ins, PosType::Unicode)
.unwrap();
}
})
});
Expand Down Expand Up @@ -51,8 +53,10 @@ mod run {
let mut txn = loro.txn().unwrap();

for TextAction { pos, ins, del } in actions.iter() {
text.delete_with_txn(&mut txn, *pos, *del).unwrap();
text.insert_with_txn(&mut txn, *pos, ins).unwrap();
text.delete_with_txn(&mut txn, *pos, *del, PosType::Unicode)
.unwrap();
text.insert_with_txn(&mut txn, *pos, ins, PosType::Unicode)
.unwrap();
}
},
criterion::BatchSize::SmallInput,
Expand All @@ -68,8 +72,10 @@ mod run {
}));
let mut txn = loro.txn().unwrap();
for TextAction { pos, ins, del } in actions.iter() {
text.delete_with_txn(&mut txn, *pos, *del).unwrap();
text.insert_with_txn(&mut txn, *pos, ins).unwrap();
text.delete_with_txn(&mut txn, *pos, *del, PosType::Unicode)
.unwrap();
text.insert_with_txn(&mut txn, *pos, ins, PosType::Unicode)
.unwrap();
}
})
});
Expand All @@ -87,8 +93,10 @@ mod run {
txn = loro.txn().unwrap();
}
n += 1;
text.delete_with_txn(&mut txn, *pos, *del).unwrap();
text.insert_with_txn(&mut txn, *pos, ins).unwrap();
text.delete_with_txn(&mut txn, *pos, *del, PosType::Unicode)
.unwrap();
text.insert_with_txn(&mut txn, *pos, ins, PosType::Unicode)
.unwrap();
}
txn.commit().unwrap();

Expand All @@ -110,8 +118,10 @@ mod run {
txn = loro.txn().unwrap();
}
n += 1;
text.delete_with_txn(&mut txn, *pos, *del).unwrap();
text.insert_with_txn(&mut txn, *pos, ins).unwrap();
text.delete_with_txn(&mut txn, *pos, *del, PosType::Unicode)
.unwrap();
text.insert_with_txn(&mut txn, *pos, ins, PosType::Unicode)
.unwrap();
}
txn.commit().unwrap();

Expand All @@ -132,8 +142,10 @@ mod run {
txn = loro.txn().unwrap();
}
n += 1;
text.delete_with_txn(&mut txn, *pos, *del).unwrap();
text.insert_with_txn(&mut txn, *pos, ins).unwrap();
text.delete_with_txn(&mut txn, *pos, *del, PosType::Unicode)
.unwrap();
text.insert_with_txn(&mut txn, *pos, ins, PosType::Unicode)
.unwrap();
}
txn.commit().unwrap();

Expand All @@ -157,8 +169,10 @@ mod run {
txn = loro.txn().unwrap();
}
n += 1;
text.delete_with_txn(&mut txn, *pos, *del).unwrap();
text.insert_with_txn(&mut txn, *pos, ins).unwrap();
text.delete_with_txn(&mut txn, *pos, *del, PosType::Unicode)
.unwrap();
text.insert_with_txn(&mut txn, *pos, ins, PosType::Unicode)
.unwrap();
}
txn.commit().unwrap();

Expand Down Expand Up @@ -195,8 +209,10 @@ mod run {
txn = loro.txn().unwrap();
}
n += 1;
text.delete_with_txn(&mut txn, *pos, *del).unwrap();
text.insert_with_txn(&mut txn, *pos, ins).unwrap();
text.delete_with_txn(&mut txn, *pos, *del, PosType::Unicode)
.unwrap();
text.insert_with_txn(&mut txn, *pos, ins, PosType::Unicode)
.unwrap();
}
})
});
Expand All @@ -208,8 +224,10 @@ mod run {
{
for TextAction { pos, ins, del } in actions.iter() {
let mut txn = loro.txn().unwrap();
text.delete_with_txn(&mut txn, *pos, *del).unwrap();
text.insert_with_txn(&mut txn, *pos, ins).unwrap();
text.delete_with_txn(&mut txn, *pos, *del, PosType::Unicode)
.unwrap();
text.insert_with_txn(&mut txn, *pos, ins, PosType::Unicode)
.unwrap();
txn.commit().unwrap();
}
}
Expand All @@ -226,8 +244,10 @@ mod run {
{
for TextAction { pos, ins, del } in actions.iter() {
let mut txn = loro.txn().unwrap();
text.delete_with_txn(&mut txn, *pos, *del).unwrap();
text.insert_with_txn(&mut txn, *pos, ins).unwrap();
text.delete_with_txn(&mut txn, *pos, *del, PosType::Unicode)
.unwrap();
text.insert_with_txn(&mut txn, *pos, ins, PosType::Unicode)
.unwrap();
txn.commit().unwrap();
}
}
Expand All @@ -242,8 +262,10 @@ mod run {
for TextAction { pos, ins, del } in actions.iter() {
{
let mut txn = loro.txn().unwrap();
text.delete_with_txn(&mut txn, *pos, *del).unwrap();
text.insert_with_txn(&mut txn, *pos, ins).unwrap();
text.delete_with_txn(&mut txn, *pos, *del, PosType::Unicode)
.unwrap();
text.insert_with_txn(&mut txn, *pos, ins, PosType::Unicode)
.unwrap();
}

loro_b
Expand Down Expand Up @@ -276,14 +298,20 @@ mod run {

{
let mut txn = loro.txn().unwrap();
text.delete_with_txn(&mut txn, pos, del).unwrap();
text.insert_with_txn(&mut txn, pos, ins).unwrap();
text.delete_with_txn(&mut txn, pos, del, PosType::Unicode)
.unwrap();
text.insert_with_txn(&mut txn, pos, ins, PosType::Unicode)
.unwrap();
}

{
let mut txn = loro_b.txn().unwrap();
text2.delete_with_txn(&mut txn, pos, del).unwrap();
text2.insert_with_txn(&mut txn, pos, ins).unwrap();
text2
.delete_with_txn(&mut txn, pos, del, PosType::Unicode)
.unwrap();
text2
.insert_with_txn(&mut txn, pos, ins, PosType::Unicode)
.unwrap();
}
loro_b
.import(
Expand Down Expand Up @@ -313,14 +341,20 @@ mod run {
let del = *del;
{
let mut txn = loro.txn().unwrap();
text.delete_with_txn(&mut txn, pos, del).unwrap();
text.insert_with_txn(&mut txn, pos, ins).unwrap();
text.delete_with_txn(&mut txn, pos, del, PosType::Unicode)
.unwrap();
text.insert_with_txn(&mut txn, pos, ins, PosType::Unicode)
.unwrap();
}

{
let mut txn = loro_b.txn().unwrap();
text2.delete_with_txn(&mut txn, pos, del).unwrap();
text2.insert_with_txn(&mut txn, pos, ins).unwrap();
text2
.delete_with_txn(&mut txn, pos, del, PosType::Unicode)
.unwrap();
text2
.insert_with_txn(&mut txn, pos, ins, PosType::Unicode)
.unwrap();
}
loro_b
.import(
Expand Down Expand Up @@ -352,14 +386,20 @@ mod run {
let del = *del;
{
let mut txn = loro.txn().unwrap();
text.delete_with_txn(&mut txn, pos, del).unwrap();
text.insert_with_txn(&mut txn, pos, ins).unwrap();
text.delete_with_txn(&mut txn, pos, del, PosType::Unicode)
.unwrap();
text.insert_with_txn(&mut txn, pos, ins, PosType::Unicode)
.unwrap();
}

{
let mut txn = loro_b.txn().unwrap();
text2.delete_with_txn(&mut txn, pos, del).unwrap();
text2.insert_with_txn(&mut txn, pos, ins).unwrap();
text2
.delete_with_txn(&mut txn, pos, del, PosType::Unicode)
.unwrap();
text2
.insert_with_txn(&mut txn, pos, ins, PosType::Unicode)
.unwrap();
}
loro_b
.import(
Expand Down
10 changes: 5 additions & 5 deletions crates/loro-internal/src/diff_calc.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -583,7 +583,7 @@ impl std::fmt::Debug for ListDiffCalculator {
impl DiffCalculatorTrait for ListDiffCalculator {
fn start_tracking(&mut self, _oplog: &OpLog, vv: &crate::VersionVector, _mode: DiffMode) {
if !vv.includes_vv(&self.start_vv) || !self.tracker.all_vv().includes_vv(vv) {
self.tracker = Box::new(RichtextTracker::new_with_unknown());
*self.tracker = RichtextTracker::new_with_unknown();
self.start_vv = vv.clone();
}

Expand Down Expand Up @@ -804,10 +804,10 @@ impl DiffCalculatorTrait for RichtextDiffCalculator {
) {
match mode {
DiffMode::Linear => {
self.mode = Box::new(RichtextCalcMode::Linear {
*self.mode = RichtextCalcMode::Linear {
diff: DeltaRope::new(),
last_style_start: None,
});
};
}
_ => {
if !matches!(&*self.mode, RichtextCalcMode::Crdt { .. }) {
Expand All @@ -823,7 +823,7 @@ impl DiffCalculatorTrait for RichtextDiffCalculator {
start_vv,
} => {
if !vv.includes_vv(start_vv) || !tracker.all_vv().includes_vv(vv) {
*tracker = Box::new(RichtextTracker::new_with_unknown());
**tracker = RichtextTracker::new_with_unknown();
styles.clear();
*start_vv = vv.clone();
}
Expand Down Expand Up @@ -1203,7 +1203,7 @@ struct MovableListInner {
impl DiffCalculatorTrait for MovableListDiffCalculator {
fn start_tracking(&mut self, _oplog: &OpLog, vv: &crate::VersionVector, mode: DiffMode) {
if !vv.includes_vv(&self.list.start_vv) || !self.list.tracker.all_vv().includes_vv(vv) {
self.list.tracker = Box::new(RichtextTracker::new_with_unknown());
*self.list.tracker = RichtextTracker::new_with_unknown();
self.list.start_vv = vv.clone();
}

Expand Down
2 changes: 1 addition & 1 deletion crates/loro/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2644,7 +2644,7 @@ impl LoroText {
pub fn get_editor_at_unicode_pos(&self, pos: usize) -> Option<PeerID> {
self.handler
.get_cursor(pos, Side::Middle)
.map(|x| x.id.unwrap().peer)
.and_then(|x| x.id.map(|id| id.peer))
}
}

Expand Down
Loading
Loading