Skip to content

Commit

Permalink
wip
Browse files Browse the repository at this point in the history
  • Loading branch information
fominok committed Nov 5, 2024
1 parent ac4b627 commit 740d97d
Show file tree
Hide file tree
Showing 3 changed files with 104 additions and 14 deletions.
9 changes: 9 additions & 0 deletions costs/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,15 @@ impl<T, E> CostResult<T, E> {
pub fn cost_as_result(self) -> Result<OperationCost, E> {
self.value.map(|_| self.cost)
}

/// Call the provided function on success without altering result or cost.
pub fn for_ok(self, f: impl FnOnce(&T)) -> CostResult<T, E> {
if let Ok(x) = &self.value {
f(x)
}

self
}
}

impl<T, E> CostResult<Result<T, E>, E> {
Expand Down
37 changes: 24 additions & 13 deletions grovedb/src/merk_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,18 @@
//! after usage automatically.
use std::{
collections::{hash_map::Entry, HashMap, HashSet},
collections::{btree_map::Entry, BTreeMap, HashSet},
mem::{self, MaybeUninit},
ops::Deref,
};

use grovedb_costs::{cost_return_on_error, CostResult, CostsExt};
use grovedb_merk::Merk;
use grovedb_merk::{Merk, MerkOptions};
use grovedb_path::SubtreePath;
use grovedb_storage::{rocksdb_storage::PrefixedRocksDbTransactionContext, StorageBatch};
use grovedb_version::version::GroveVersion;

use crate::{Error, GroveDb, Transaction};
use crate::{Element, Error, GroveDb, Transaction};

type TxMerk<'db> = Merk<PrefixedRocksDbTransactionContext<'db>>;

Expand All @@ -29,7 +29,7 @@ pub(crate) struct MerkCache<'db, 'b, B> {
tx: &'db Transaction<'db>,
batch: &'db StorageBatch,
version: &'db GroveVersion,
inner: HashMap<SubtreePath<'b, B>, TxMerk<'db>>,
merks: BTreeMap<SubtreePath<'b, B>, TxMerk<'db>>,
}

impl<'db, 'b, B: AsRef<[u8]>> MerkCache<'db, 'b, B> {
Expand All @@ -44,7 +44,7 @@ impl<'db, 'b, B: AsRef<[u8]>> MerkCache<'db, 'b, B> {
tx,
batch,
version,
inner: Default::default(),
merks: Default::default(),
}
}

Expand All @@ -57,7 +57,7 @@ impl<'db, 'b, B: AsRef<[u8]>> MerkCache<'db, 'b, B> {
) -> CostResult<&'s mut TxMerk<'db>, Error> {
let mut cost = Default::default();

match self.inner.entry(path) {
match self.merks.entry(path) {
Entry::Occupied(e) => Ok(e.into_mut()).wrap_with_cost(cost),
Entry::Vacant(e) => {
let merk = cost_return_on_error!(
Expand Down Expand Up @@ -100,12 +100,13 @@ impl<'db, 'b, B: AsRef<[u8]>> MerkCache<'db, 'b, B> {
// mutable references. The mandatory keys' uniqueness check above makes
// sure no overlapping memory will be referenced.
let merk_ref = unsafe {
MerkHandle(
(cost_return_on_error!(&mut cost, self.get_merk_mut_internal(path))
MerkHandle {
merk: (cost_return_on_error!(&mut cost, self.get_merk_mut_internal(path))
as *mut TxMerk<'db>)
.as_mut::<'s>()
.expect("not a null pointer"),
)
version: &self.version,

Check warning on line 108 in grovedb/src/merk_cache.rs

View workflow job for this annotation

GitHub Actions / clippy

this expression creates a reference which is immediately dereferenced by the compiler

warning: this expression creates a reference which is immediately dereferenced by the compiler --> grovedb/src/merk_cache.rs:108:30 | 108 | version: &self.version, | ^^^^^^^^^^^^^ help: change this to: `self.version` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrow
}
};
result_uninit[i].write(merk_ref);
}
Expand All @@ -123,21 +124,31 @@ impl<'db, 'b, B: AsRef<[u8]>> MerkCache<'db, 'b, B> {
}

/// Handle to a cached Merk.
pub(crate) struct MerkHandle<'db, 'c>(&'c mut TxMerk<'db>);
pub(crate) struct MerkHandle<'db, 'c> {

Check warning on line 127 in grovedb/src/merk_cache.rs

View workflow job for this annotation

GitHub Actions / clippy

struct `MerkHandle` is never constructed

warning: struct `MerkHandle` is never constructed --> grovedb/src/merk_cache.rs:127:19 | 127 | pub(crate) struct MerkHandle<'db, 'c> { | ^^^^^^^^^^
merk: &'c mut TxMerk<'db>,
version: &'db GroveVersion,
}

/// It is allowed to dereference `MerkHandle` to regular Merks but in a
/// non-mutable way since we want to track what have been done to those Merks.
impl<'db, 'c> Deref for MerkHandle<'db, 'c> {
type Target = TxMerk<'db>;

fn deref(&self) -> &Self::Target {
&self.0
&self.merk

Check warning on line 138 in grovedb/src/merk_cache.rs

View workflow job for this annotation

GitHub Actions / clippy

this expression creates a reference which is immediately dereferenced by the compiler

warning: this expression creates a reference which is immediately dereferenced by the compiler --> grovedb/src/merk_cache.rs:138:9 | 138 | &self.merk | ^^^^^^^^^^ help: change this to: `self.merk` | = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrow
}
}

impl<'db, 'c> MerkHandle<'db, 'c> {
pub(crate) fn insert(&mut self) {
todo!()
pub(crate) fn insert(

Check warning on line 143 in grovedb/src/merk_cache.rs

View workflow job for this annotation

GitHub Actions / clippy

method `insert` is never used

warning: method `insert` is never used --> grovedb/src/merk_cache.rs:143:19 | 142 | impl<'db, 'c> MerkHandle<'db, 'c> { | --------------------------------- method in this implementation 143 | pub(crate) fn insert( | ^^^^^^
&mut self,
key: impl AsRef<[u8]>,
element: Element,
options: Option<MerkOptions>,
) -> CostResult<(), Error> {
element
.insert(self.merk, key, options, self.version)
.for_ok(|_| todo!())
}
}

Expand Down
72 changes: 71 additions & 1 deletion path/src/subtree_path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,10 @@
//! combined with it's various `From` implementations it can cover slices, owned
//! subtree paths and other path references if use as generic [Into].
use std::hash::{Hash, Hasher};
use std::{
cmp,
hash::{Hash, Hasher},
};

use crate::{
subtree_path_builder::{SubtreePathBuilder, SubtreePathRelative},
Expand Down Expand Up @@ -78,6 +81,48 @@ where
}
}

/// First and foremost, the order of subtree paths is dictated by their lengths.
/// Therefore, those subtrees closer to the root will come first. The rest it
/// can guarantee is to be free of false equality; however, seemingly unrelated
/// subtrees can come one after another if they share the same length, which was
/// (not) done for performance reasons.
impl<'bl, 'br, BL, BR> PartialOrd<SubtreePath<'br, BR>> for SubtreePath<'bl, BL>
where
BL: AsRef<[u8]>,
BR: AsRef<[u8]>,
{
fn partial_cmp(&self, other: &SubtreePath<'br, BR>) -> Option<cmp::Ordering> {
let iter_a = self.clone().into_reverse_iter();
let iter_b = other.clone().into_reverse_iter();

Some(
iter_a
.len()
.cmp(&iter_b.len())
.then_with(|| iter_a.cmp(iter_b)),
)
}
}

impl<'bl, 'br, BL, BR> PartialOrd<SubtreePathBuilder<'br, BR>> for SubtreePath<'bl, BL>
where
BL: AsRef<[u8]>,
BR: AsRef<[u8]>,
{
fn partial_cmp(&self, other: &SubtreePathBuilder<'br, BR>) -> Option<cmp::Ordering> {
self.partial_cmp(&SubtreePath::from(other))
}
}

impl<'bl, BL> Ord for SubtreePath<'bl, BL>
where
BL: AsRef<[u8]>,
{
fn cmp(&self, other: &Self) -> cmp::Ordering {
self.partial_cmp(other).expect("order is totally defined")
}
}

impl<'b, B: AsRef<[u8]>> Eq for SubtreePath<'b, B> {}

impl<'b, B> From<SubtreePathInner<'b, B>> for SubtreePath<'b, B> {
Expand Down Expand Up @@ -274,4 +319,29 @@ mod tests {
assert_eq!(as_vec, reference_vec);
assert_eq!(parent.len(), reference_vec.len());
}

#[test]
fn ordering() {
let path_a: SubtreePath<_> = (&[b"one" as &[u8], b"two", b"three"]).into();
let path_b = path_a.derive_owned_with_child(b"four");
let path_c = path_a.derive_owned_with_child(b"notfour");
let (path_d_parent, _) = path_a.derive_parent().unwrap();
let path_d = path_d_parent.derive_owned_with_child(b"three");

// Same lengths for different paths don't make them equal:
assert!(!matches!(
SubtreePath::from(&path_b).cmp(&SubtreePath::from(&path_c)),
cmp::Ordering::Equal
));

// Equal paths made the same way are equal:
assert!(matches!(
path_a.cmp(&SubtreePath::from(&path_d)),
cmp::Ordering::Equal
));

// Longer paths are "greater than":
assert!(path_a < path_b);
assert!(path_a < path_c);
}
}

0 comments on commit 740d97d

Please sign in to comment.