Skip to content

Commit

Permalink
Add into_inner method so we don't have to clone (#548)
Browse files Browse the repository at this point in the history
  • Loading branch information
richardpringle authored Feb 21, 2024
1 parent 56e2d2d commit b7f000f
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 20 deletions.
7 changes: 1 addition & 6 deletions firewood/src/merkle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1519,7 +1519,7 @@ impl<S: ShaleStore<Node> + Send + Sync, T> Merkle<S, T> {
) -> Result<NodeObjRef<'a>, MerkleError> {
if let Err(ObjWriteSizeError) = write_result {
let old_node_address = node.as_ptr();
node = self.put_node(node.clone())?;
node = self.put_node(node.into_inner())?;
deleted.push(old_node_address);

set_parent(node.as_ptr(), parents);
Expand Down Expand Up @@ -2323,7 +2323,6 @@ mod tests {
}

#[test]
#[ignore]
fn single_key_proof_with_one_node() {
let mut merkle = create_test_merkle();
let root = merkle.init_root().unwrap();
Expand Down Expand Up @@ -2362,7 +2361,6 @@ mod tests {
}

#[test]
#[ignore]
fn update_leaf_with_larger_path() -> Result<(), MerkleError> {
let path = vec![0x00];
let data = vec![0x00];
Expand All @@ -2382,7 +2380,6 @@ mod tests {
}

#[test]
#[ignore]
fn update_leaf_with_larger_data() -> Result<(), MerkleError> {
let path = vec![0x00];
let data = vec![0x00];
Expand All @@ -2402,7 +2399,6 @@ mod tests {
}

#[test]
#[ignore]
fn update_branch_with_larger_path() -> Result<(), MerkleError> {
let path = vec![0x00];
let data = vec![0x00];
Expand All @@ -2424,7 +2420,6 @@ mod tests {
}

#[test]
#[ignore]
fn update_branch_with_larger_data() -> Result<(), MerkleError> {
let path = vec![0x00];
let data = vec![0x00];
Expand Down
6 changes: 3 additions & 3 deletions firewood/src/shale/compact.rs
Original file line number Diff line number Diff line change
Expand Up @@ -603,7 +603,7 @@ impl<T: Storable + Debug + 'static + PartialEq, M: CachedStore + Send + Sync> Sh

let cache = &self.obj_cache;

let mut obj_ref = ObjRef::new(Some(obj), cache);
let mut obj_ref = ObjRef::new(obj, cache);

// should this use a `?` instead of `unwrap`?
#[allow(clippy::unwrap_used)]
Expand All @@ -628,7 +628,7 @@ impl<T: Storable + Debug + 'static + PartialEq, M: CachedStore + Send + Sync> Sh
let cache = &self.obj_cache;

if let Some(obj) = obj {
return Ok(ObjRef::new(Some(obj), cache));
return Ok(ObjRef::new(obj, cache));
}

#[allow(clippy::unwrap_used)]
Expand All @@ -645,7 +645,7 @@ impl<T: Storable + Debug + 'static + PartialEq, M: CachedStore + Send + Sync> Sh
let obj = self.obj_cache.put(inner.get_data_ref(ptr, payload_size)?);
let cache = &self.obj_cache;

Ok(ObjRef::new(Some(obj), cache))
Ok(ObjRef::new(obj, cache))
}

#[allow(clippy::unwrap_used)]
Expand Down
49 changes: 38 additions & 11 deletions firewood/src/shale/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ use std::sync::{Arc, RwLock, RwLockWriteGuard};

use thiserror::Error;

use crate::merkle::{LeafNode, Node, PartialPath};

pub mod cached;
pub mod compact;
pub mod disk_address;
Expand Down Expand Up @@ -155,6 +157,17 @@ impl<T: Storable> Obj<T> {
}
}

impl Obj<Node> {
pub fn into_inner(mut self) -> Node {
let empty_node = LeafNode {
path: PartialPath(Vec::new()),
data: Vec::new().into(),
};

std::mem::replace(&mut self.value.decoded, Node::from_leaf(empty_node))
}
}

impl<T: Storable> Drop for Obj<T> {
fn drop(&mut self) {
self.flush_dirty()
Expand All @@ -171,12 +184,15 @@ impl<T: Storable> Deref for Obj<T> {
/// User handle that offers read & write access to the stored [ShaleStore] item.
#[derive(Debug)]
pub struct ObjRef<'a, T: Storable> {
/// WARNING:
/// [Self::inner] should only set to [None] when consuming [Self] or inside [Drop::drop].
inner: Option<Obj<T>>,
cache: &'a ObjCache<T>,
}

impl<'a, T: Storable + Debug> ObjRef<'a, T> {
const fn new(inner: Option<Obj<T>>, cache: &'a ObjCache<T>) -> Self {
const fn new(inner: Obj<T>, cache: &'a ObjCache<T>) -> Self {
let inner = Some(inner);
Self { inner, cache }
}

Expand All @@ -196,6 +212,17 @@ impl<'a, T: Storable + Debug> ObjRef<'a, T> {
}
}

impl<'a> ObjRef<'a, Node> {
/// # Panics:
/// if inner is not set
pub fn into_inner(mut self) -> Node {
self.inner
.take()
.expect("inner should already be set")
.into_inner()
}
}

impl<'a, T: Storable + Debug> Deref for ObjRef<'a, T> {
type Target = Obj<T>;
fn deref(&self) -> &Obj<T> {
Expand All @@ -207,16 +234,16 @@ impl<'a, T: Storable + Debug> Deref for ObjRef<'a, T> {

impl<'a, T: Storable> Drop for ObjRef<'a, T> {
fn drop(&mut self) {
#[allow(clippy::unwrap_used)]
let mut inner = self.inner.take().unwrap();
let ptr = inner.as_ptr();
let mut cache = self.cache.lock();
match cache.pinned.remove(&ptr) {
Some(true) => {
inner.dirty = None;
}
_ => {
cache.cached.put(ptr, inner);
if let Some(mut inner) = self.inner.take() {
let ptr = inner.as_ptr();
let mut cache = self.cache.lock();
match cache.pinned.remove(&ptr) {
Some(true) => {
inner.dirty = None;
}
_ => {
cache.cached.put(ptr, inner);
}
}
}
}
Expand Down

0 comments on commit b7f000f

Please sign in to comment.