From 7f2cf0b809f1fc5741990e2bfff94dc3ec41a04f Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Mon, 6 Jun 2022 14:01:44 +0200 Subject: Safe choice: return Vec and not some fancy zero-copy type --- src/block/manager.rs | 12 ++---- src/db/lib.rs | 101 ++++------------------------------------------- src/db/lmdb_adapter.rs | 70 ++++++-------------------------- src/db/sled_adapter.rs | 39 +++++------------- src/db/sqlite_adapter.rs | 12 +++--- src/garage/repair.rs | 8 ++-- src/table/data.rs | 4 +- src/table/merkle.rs | 6 +-- src/table/sync.rs | 2 +- 9 files changed, 49 insertions(+), 205 deletions(-) (limited to 'src') diff --git a/src/block/manager.rs b/src/block/manager.rs index decf33cc..abc5063d 100644 --- a/src/block/manager.rs +++ b/src/block/manager.rs @@ -555,11 +555,7 @@ impl BlockManager { // - Ok(false) -> no block was processed, but we are ready for the next iteration // - Err(_) -> a Sled error occurred when reading/writing from resync_queue/resync_errors async fn resync_iter(&self, must_exit: &mut watch::Receiver) -> Result { - let next = self - .resync_queue - .first()? - .map(|(k, v)| (k.into_vec(), v.into_vec())); - if let Some((time_bytes, hash_bytes)) = next { + if let Some((time_bytes, hash_bytes)) = self.resync_queue.first()? { let time_msec = u64::from_be_bytes(time_bytes[0..8].try_into().unwrap()); let now = now_msec(); @@ -567,7 +563,7 @@ impl BlockManager { let hash = Hash::try_from(&hash_bytes[..]).unwrap(); if let Some(ec) = self.resync_errors.get(hash.as_slice())? { - let ec = ErrorCounter::decode(ec); + let ec = ErrorCounter::decode(&ec); if now < ec.next_try() { // if next retry after an error is not yet, // don't do resync and return early, but still @@ -608,7 +604,7 @@ impl BlockManager { warn!("Error when resyncing {:?}: {}", hash, e); let err_counter = match self.resync_errors.get(hash.as_slice())? { - Some(ec) => ErrorCounter::decode(ec).add1(now + 1), + Some(ec) => ErrorCounter::decode(&ec).add1(now + 1), None => ErrorCounter::new(now + 1), }; @@ -972,7 +968,7 @@ impl ErrorCounter { } } - fn decode(data: db::Value<'_>) -> Self { + fn decode(data: &db::Value) -> Self { Self { errors: u64::from_be_bytes(data[0..8].try_into().unwrap()), last_try: u64::from_be_bytes(data[8..16].try_into().unwrap()), diff --git a/src/db/lib.rs b/src/db/lib.rs index 2e847f7a..b9c27d9a 100644 --- a/src/db/lib.rs +++ b/src/db/lib.rs @@ -21,93 +21,8 @@ pub struct Transaction<'a>(pub(crate) &'a mut dyn ITx); #[derive(Clone)] pub struct Tree(pub(crate) Arc, pub(crate) usize); -pub type ValueIter<'a> = Box, Value<'a>)>> + 'a>; - -// ---- - -pub struct Value<'a>(pub(crate) Box + 'a>); - -pub trait IValue<'a>: AsRef<[u8]> + core::borrow::Borrow<[u8]> { - fn take_maybe(&mut self) -> Vec; -} - -impl<'a> Value<'a> { - #[inline] - pub fn into_vec(mut self) -> Vec { - self.0.take_maybe() - } -} - -impl<'a> AsRef<[u8]> for Value<'a> { - #[inline] - fn as_ref(&self) -> &[u8] { - self.0.as_ref().as_ref() - } -} - -impl<'a> std::borrow::Borrow<[u8]> for Value<'a> { - #[inline] - fn borrow(&self) -> &[u8] { - self.0.as_ref().borrow() - } -} - -impl<'a> core::ops::Deref for Value<'a> { - type Target = [u8]; - #[inline] - fn deref(&self) -> &[u8] { - self.0.as_ref().as_ref() - } -} - -impl<'a, T> PartialEq for Value<'a> -where - T: AsRef<[u8]>, -{ - fn eq(&self, other: &T) -> bool { - self.as_ref() == other.as_ref() - } -} - -impl<'a> std::fmt::Debug for Value<'a> { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - for line in hexdump::hexdump_iter(self.as_ref()) { - f.write_str(&line)?; - f.write_str("\n")?; - } - Ok(()) - } -} - -impl<'a> IValue<'a> for Vec { - fn take_maybe(&mut self) -> Vec { - std::mem::take(self) - } -} - -impl<'a> From> for Vec { - fn from(v: Value<'a>) -> Vec { - v.into_vec() - } -} - -impl<'a> From> for Value<'a> { - fn from(v: Vec) -> Value<'a> { - Value(Box::new(v)) - } -} - -impl<'a> From<&'a [u8]> for Value<'a> { - fn from(v: &'a [u8]) -> Value<'a> { - Value(Box::new(v)) - } -} - -impl<'a> IValue<'a> for &'a [u8] { - fn take_maybe(&mut self) -> Vec { - self.to_vec() - } -} +pub type Value = Vec; +pub type ValueIter<'a> = Box> + 'a>; // ---- @@ -228,17 +143,17 @@ impl Tree { Db(self.0.clone()) } - pub fn get>(&self, key: T) -> Result>> { + pub fn get>(&self, key: T) -> Result> { self.0.get(self.1, key.as_ref()) } pub fn len(&self) -> Result { self.0.len(self.1) } - pub fn first(&self) -> Result, Value<'_>)>> { + pub fn first(&self) -> Result> { self.iter()?.next().transpose() } - pub fn get_gt>(&self, from: T) -> Result, Value<'_>)>> { + pub fn get_gt>(&self, from: T) -> Result> { self.range((Bound::Excluded(from), Bound::Unbounded))? .next() .transpose() @@ -280,7 +195,7 @@ impl Tree { #[allow(clippy::len_without_is_empty)] impl<'a> Transaction<'a> { - pub fn get>(&self, tree: &Tree, key: T) -> Result>> { + pub fn get>(&self, tree: &Tree, key: T) -> Result> { self.0.get(tree.1, key.as_ref()) } pub fn len(&self, tree: &Tree) -> Result { @@ -342,7 +257,7 @@ pub(crate) trait IDb: Send + Sync { fn open_tree(&self, name: &str) -> Result; fn list_trees(&self) -> Result>; - fn get(&self, tree: usize, key: &[u8]) -> Result>>; + fn get(&self, tree: usize, key: &[u8]) -> Result>; fn len(&self, tree: usize) -> Result; fn insert(&self, tree: usize, key: &[u8], value: &[u8]) -> Result<()>; @@ -368,7 +283,7 @@ pub(crate) trait IDb: Send + Sync { } pub(crate) trait ITx { - fn get(&self, tree: usize, key: &[u8]) -> Result>>; + fn get(&self, tree: usize, key: &[u8]) -> Result>; fn len(&self, tree: usize) -> Result; fn insert(&mut self, tree: usize, key: &[u8], value: &[u8]) -> Result<()>; diff --git a/src/db/lmdb_adapter.rs b/src/db/lmdb_adapter.rs index aa365733..3d0edb38 100644 --- a/src/db/lmdb_adapter.rs +++ b/src/db/lmdb_adapter.rs @@ -11,7 +11,7 @@ use heed::types::ByteSlice; use heed::{BytesDecode, Env, RoTxn, RwTxn, UntypedDatabase as Database}; use crate::{ - Db, Error, IDb, ITx, ITxFn, IValue, Result, TxError, TxFnResult, TxResult, Value, ValueIter, + Db, Error, IDb, ITx, ITxFn, Result, TxError, TxFnResult, TxResult, Value, ValueIter, }; pub use heed; @@ -101,28 +101,15 @@ impl IDb for LmdbDb { // ---- - fn get(&self, tree: usize, key: &[u8]) -> Result>> { + fn get(&self, tree: usize, key: &[u8]) -> Result> { let tree = self.get_tree(tree)?; - let res = TxAndValue { - tx: self.db.read_txn()?, - value: NonNull::dangling(), - _pin: PhantomPinned, - }; - let mut boxed = Box::pin(res); - - unsafe { - let tx = NonNull::from(&boxed.tx); - let val = match tree.get(tx.as_ref(), &key)? { - None => return Ok(None), - Some(v) => v, - }; - - let mut_ref: Pin<&mut TxAndValue<'_>> = Pin::as_mut(&mut boxed); - Pin::get_unchecked_mut(mut_ref).value = NonNull::from(&val); + let tx = self.db.read_txn()?; + let val = tree.get(&tx, &key)?; + match val { + None => Ok(None), + Some(v) => Ok(Some(v.to_vec())), } - - Ok(Some(Value(Box::new(TxAndValuePin(boxed))))) } fn remove(&self, tree: usize, key: &[u8]) -> Result { @@ -227,13 +214,10 @@ impl<'a, 'db> LmdbTx<'a, 'db> { } impl<'a, 'db> ITx for LmdbTx<'a, 'db> { - fn get(&self, tree: usize, key: &[u8]) -> Result>> { + fn get(&self, tree: usize, key: &[u8]) -> Result> { let tree = self.get_tree(tree)?; match tree.get(&self.tx, &key)? { - Some(v) => { - let v: &'_ [u8] = v; - Ok(Some(v.into())) - } + Some(v) => Ok(Some(v.to_vec())), None => Ok(None), } } @@ -279,34 +263,6 @@ impl<'a, 'db> ITx for LmdbTx<'a, 'db> { // ---- -struct TxAndValue<'a> { - tx: RoTxn<'a>, - value: NonNull<&'a [u8]>, - _pin: PhantomPinned, -} - -struct TxAndValuePin<'a>(Pin>>); - -impl<'a> IValue<'a> for TxAndValuePin<'a> { - fn take_maybe(&mut self) -> Vec { - self.as_ref().to_vec() - } -} - -impl<'a> AsRef<[u8]> for TxAndValuePin<'a> { - fn as_ref(&self) -> &[u8] { - unsafe { self.0.value.as_ref() } - } -} - -impl<'a> std::borrow::Borrow<[u8]> for TxAndValuePin<'a> { - fn borrow(&self) -> &[u8] { - self.as_ref() - } -} - -// ---- - type IteratorItem<'a> = heed::Result<( >::DItem, >::DItem, @@ -365,7 +321,7 @@ impl<'a, I> Iterator for TxAndIteratorPin<'a, I> where I: Iterator> + 'a, { - type Item = Result<(Value<'a>, Value<'a>)>; + type Item = Result<(Value, Value)>; fn next(&mut self) -> Option { let iter_ref = unsafe { @@ -375,11 +331,7 @@ where match iter_ref.unwrap().next() { None => None, Some(Err(e)) => Some(Err(e.into())), - Some(Ok((k, v))) => { - let k: &'a [u8] = k; - let v: &'a [u8] = v; - Some(Ok((k.into(), v.into()))) - } + Some(Ok((k, v))) => Some(Ok((k.to_vec(), v.to_vec()))), } } } diff --git a/src/db/sled_adapter.rs b/src/db/sled_adapter.rs index 2953785e..489fae61 100644 --- a/src/db/sled_adapter.rs +++ b/src/db/sled_adapter.rs @@ -10,7 +10,7 @@ use sled::transaction::{ }; use crate::{ - Db, Error, IDb, ITx, ITxFn, IValue, Result, TxError, TxFnResult, TxResult, Value, ValueIter, + Db, Error, IDb, ITx, ITxFn, Result, TxError, TxFnResult, TxResult, Value, ValueIter, }; pub use sled; @@ -23,26 +23,6 @@ impl From for Error { } } -// -- val - -impl<'a> IValue<'a> for sled::IVec { - fn take_maybe(&mut self) -> Vec { - self.to_vec() - } -} - -impl<'a> From> for sled::IVec { - fn from(v: Value<'a>) -> sled::IVec { - sled::IVec::from(v.into_vec()) - } -} - -impl<'a> From for Value<'a> { - fn from(v: sled::IVec) -> Value<'a> { - Value(Box::new(v)) - } -} - // -- db pub struct SledDb { @@ -99,9 +79,10 @@ impl IDb for SledDb { // ---- - fn get(&self, tree: usize, key: &[u8]) -> Result>> { + fn get(&self, tree: usize, key: &[u8]) -> Result> { let tree = self.get_tree(tree)?; - Ok(tree.get(key)?.map(From::from)) + let val = tree.get(key)?; + Ok(val.map(|x| x.to_vec())) } fn remove(&self, tree: usize, key: &[u8]) -> Result { @@ -123,14 +104,14 @@ impl IDb for SledDb { fn iter(&self, tree: usize) -> Result> { let tree = self.get_tree(tree)?; Ok(Box::new(tree.iter().map(|v| { - v.map(|(x, y)| (x.into(), y.into())).map_err(Into::into) + v.map(|(x, y)| (x.to_vec(), y.to_vec())).map_err(Into::into) }))) } fn iter_rev(&self, tree: usize) -> Result> { let tree = self.get_tree(tree)?; Ok(Box::new(tree.iter().rev().map(|v| { - v.map(|(x, y)| (x.into(), y.into())).map_err(Into::into) + v.map(|(x, y)| (x.to_vec(), y.to_vec())).map_err(Into::into) }))) } @@ -142,7 +123,7 @@ impl IDb for SledDb { ) -> Result> { let tree = self.get_tree(tree)?; Ok(Box::new(tree.range::<&'r [u8], _>((low, high)).map(|v| { - v.map(|(x, y)| (x.into(), y.into())).map_err(Into::into) + v.map(|(x, y)| (x.to_vec(), y.to_vec())).map_err(Into::into) }))) } fn range_rev<'r>( @@ -153,7 +134,7 @@ impl IDb for SledDb { ) -> Result> { let tree = self.get_tree(tree)?; Ok(Box::new(tree.range::<&'r [u8], _>((low, high)).rev().map( - |v| v.map(|(x, y)| (x.into(), y.into())).map_err(Into::into), + |v| v.map(|(x, y)| (x.to_vec(), y.to_vec())).map_err(Into::into), ))) } @@ -218,10 +199,10 @@ impl<'a> SledTx<'a> { } impl<'a> ITx for SledTx<'a> { - fn get(&self, tree: usize, key: &[u8]) -> Result>> { + fn get(&self, tree: usize, key: &[u8]) -> Result> { let tree = self.get_tree(tree)?; let tmp = self.save_error(tree.get(key))?; - Ok(tmp.map(From::from)) + Ok(tmp.map(|x| x.to_vec())) } fn len(&self, _tree: usize) -> Result { unimplemented!(".len() in transaction not supported with Sled backend") diff --git a/src/db/sqlite_adapter.rs b/src/db/sqlite_adapter.rs index e945c31c..f0bca257 100644 --- a/src/db/sqlite_adapter.rs +++ b/src/db/sqlite_adapter.rs @@ -105,7 +105,7 @@ impl IDb for SqliteDb { // ---- - fn get(&self, tree: usize, key: &[u8]) -> Result>> { + fn get(&self, tree: usize, key: &[u8]) -> Result> { trace!("get {}: lock db", tree); let this = self.0.lock().unwrap(); trace!("get {}: lock acquired", tree); @@ -118,7 +118,7 @@ impl IDb for SqliteDb { let mut res_iter = stmt.query([key])?; match res_iter.next()? { None => Ok(None), - Some(v) => Ok(Some(v.get::<_, Vec>(0)?.into())), + Some(v) => Ok(Some(v.get::<_, Vec>(0)?)), } } @@ -279,7 +279,7 @@ impl<'a> SqliteTx<'a> { } impl<'a> ITx for SqliteTx<'a> { - fn get(&self, tree: usize, key: &[u8]) -> Result>> { + fn get(&self, tree: usize, key: &[u8]) -> Result> { let tree = self.get_tree(tree)?; let mut stmt = self .tx @@ -287,7 +287,7 @@ impl<'a> ITx for SqliteTx<'a> { let mut res_iter = stmt.query([key])?; match res_iter.next()? { None => Ok(None), - Some(v) => Ok(Some(v.get::<_, Vec>(0)?.into())), + Some(v) => Ok(Some(v.get::<_, Vec>(0)?)), } } fn len(&self, tree: usize) -> Result { @@ -394,7 +394,7 @@ impl<'a> Drop for DbValueIterator<'a> { struct DbValueIteratorPin<'a>(Pin>>); impl<'a> Iterator for DbValueIteratorPin<'a> { - type Item = Result<(Value<'a>, Value<'a>)>; + type Item = Result<(Value, Value)>; fn next(&mut self) -> Option { let next = unsafe { @@ -414,7 +414,7 @@ impl<'a> Iterator for DbValueIteratorPin<'a> { Err(e) => return Some(Err(e.into())), Ok(y) => y, }; - Some(Ok((k.into(), v.into()))) + Some(Ok((k, v))) } } diff --git a/src/garage/repair.rs b/src/garage/repair.rs index 1ae26181..ae28f351 100644 --- a/src/garage/repair.rs +++ b/src/garage/repair.rs @@ -72,8 +72,8 @@ impl Repair { Some(pair) => pair, None => break, }; - pos = k.into_vec(); - v.into_vec() + pos = k; + v }; i += 1; @@ -124,8 +124,8 @@ impl Repair { Some(pair) => pair, None => break, }; - pos = k.into_vec(); - v.into_vec() + pos = k; + v }; i += 1; diff --git a/src/table/data.rs b/src/table/data.rs index cca96f68..fe8c5dbc 100644 --- a/src/table/data.rs +++ b/src/table/data.rs @@ -138,7 +138,7 @@ where } }; if keep { - ret.push(Arc::new(ByteBuf::from(value.as_ref()))); + ret.push(Arc::new(ByteBuf::from(value))); } if ret.len() >= limit { break; @@ -271,7 +271,7 @@ where ) -> Result { let removed = self.store.db().transaction(|mut tx| { let remove_v = match tx.get(&self.store, k)? { - Some(cur_v) if blake2sum(&cur_v[..]) == vhash => Some(cur_v.into_vec()), + Some(cur_v) if blake2sum(&cur_v[..]) == vhash => Some(cur_v), _ => None, }; if remove_v.is_some() { diff --git a/src/table/merkle.rs b/src/table/merkle.rs index 92e1445b..dc67e8b7 100644 --- a/src/table/merkle.rs +++ b/src/table/merkle.rs @@ -292,7 +292,7 @@ where k: &MerkleNodeKey, ) -> db::TxResult { let ent = tx.get(&self.data.merkle_tree, k.encode())?; - MerkleNode::decode_opt(ent).map_err(db::TxError::Abort) + MerkleNode::decode_opt(&ent).map_err(db::TxError::Abort) } fn put_node_txn( @@ -316,7 +316,7 @@ where // Access a node in the Merkle tree, used by the sync protocol pub(crate) fn read_node(&self, k: &MerkleNodeKey) -> Result { let ent = self.data.merkle_tree.get(k.encode())?; - MerkleNode::decode_opt(ent) + MerkleNode::decode_opt(&ent) } pub fn merkle_tree_len(&self) -> usize { @@ -351,7 +351,7 @@ impl MerkleNodeKey { } impl MerkleNode { - fn decode_opt(ent: Option>) -> Result { + fn decode_opt(ent: &Option) -> Result { match ent { None => Ok(MerkleNode::Empty), Some(v) => Ok(rmp_serde::decode::from_slice::(&v[..])?), diff --git a/src/table/sync.rs b/src/table/sync.rs index 87dfd1d8..20066d73 100644 --- a/src/table/sync.rs +++ b/src/table/sync.rs @@ -260,7 +260,7 @@ where for item in self.data.store.range(begin.to_vec()..end.to_vec())? { let (key, value) = item?; - items.push((key.to_vec(), Arc::new(ByteBuf::from(value.as_ref())))); + items.push((key.to_vec(), Arc::new(ByteBuf::from(value)))); if items.len() >= 1024 { break; -- cgit v1.2.3