Skip to content

Commit

Permalink
shale: improve error handling (#70)
Browse files Browse the repository at this point in the history 
Signed-off-by: Sam Batschelet <[email protected]>
  • Loading branch information
@hexfusion
hexfusion authored Apr 28, 2023
1 parent f8b6b1c commit 503f4e5
Show file tree
Hide file tree
Showing 10 changed files with 283 additions and 178 deletions.
1 change: 1 addition & 0 deletions firewood-shale/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ license = "MIT"
[dependencies]
hex = "0.4.3"
lru = "0.10.0"
thiserror = "1.0.38"

[dev-dependencies]
criterion = { version = "0.4.0", features = ["html_reports"] }
Expand Down
6 changes: 4 additions & 2 deletions firewood-shale/benches/shale-bench.rs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
extern crate firewood_shale as shale;

use criterion::{criterion_group, criterion_main, profiler::Profiler, Bencher, Criterion};
use criterion::{
black_box, criterion_group, criterion_main, profiler::Profiler, Bencher, Criterion,
};
use pprof::ProfilerGuard;
use rand::Rng;
use shale::{
Expand Down Expand Up @@ -55,7 +57,7 @@ fn get_view<C: CachedStore>(b: &mut Bencher, mut cached: C) {

b.iter(|| {
let len = rng.gen_range(0..26);
let rdata = &"abcdefghijklmnopqrstuvwxyz".as_bytes()[..len];
let rdata = black_box(&"abcdefghijklmnopqrstuvwxyz".as_bytes()[..len]);

let offset = rng.gen_range(0..BENCH_MEM_SIZE - len as u64);

Expand Down
14 changes: 7 additions & 7 deletions firewood-shale/src/cached.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,11 @@ impl CachedStore for PlainMem {
}
}

fn get_shared(&self) -> Option<Box<dyn DerefMut<Target = dyn CachedStore>>> {
Some(Box::new(PlainMemShared(Self {
fn get_shared(&self) -> Box<dyn DerefMut<Target = dyn CachedStore>> {
Box::new(PlainMemShared(Self {
space: self.space.clone(),
id: self.id,
})))
}))
}

fn write(&mut self, offset: u64, change: &[u8]) {
Expand Down Expand Up @@ -140,11 +140,11 @@ impl CachedStore for DynamicMem {
}))
}

fn get_shared(&self) -> Option<Box<dyn DerefMut<Target = dyn CachedStore>>> {
Some(Box::new(DynamicMemShared(Self {
fn get_shared(&self) -> Box<dyn DerefMut<Target = dyn CachedStore>> {
Box::new(DynamicMemShared(Self {
space: self.space.clone(),
id: self.id,
})))
}))
}

fn write(&mut self, offset: u64, change: &[u8]) {
Expand Down Expand Up @@ -240,7 +240,7 @@ mod tests {
mem.write(0, &[1, 2]);
mem.write(0, &[3, 4]);
assert_eq!(mem.get_view(0, 2).unwrap().as_deref(), [3, 4]);
mem.get_shared().unwrap().write(0, &[5, 6]);
mem.get_shared().write(0, &[5, 6]);

// capacity is increased
mem.write(5, &[0; 10]);
Expand Down
133 changes: 84 additions & 49 deletions firewood-shale/src/compact.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,15 @@ impl Storable for CompactHeader {
fn hydrate<T: CachedStore>(addr: u64, mem: &T) -> Result<Self, ShaleError> {
let raw = mem
.get_view(addr, Self::MSIZE)
.ok_or(ShaleError::LinearCachedStoreError)?;
let payload_size = u64::from_le_bytes(raw.as_deref()[..8].try_into().unwrap());
.ok_or(ShaleError::InvalidCacheView {
offset: addr,
size: Self::MSIZE,
})?;
let payload_size =
u64::from_le_bytes(raw.as_deref()[..8].try_into().expect("invalid slice"));
let is_freed = raw.as_deref()[8] != 0;
let desc_addr = u64::from_le_bytes(raw.as_deref()[9..17].try_into().unwrap());
let desc_addr =
u64::from_le_bytes(raw.as_deref()[9..17].try_into().expect("invalid slice"));
Ok(Self {
payload_size,
is_freed,
Expand All @@ -39,11 +44,12 @@ impl Storable for CompactHeader {
Self::MSIZE
}

fn dehydrate(&self, to: &mut [u8]) {
fn dehydrate(&self, to: &mut [u8]) -> Result<(), ShaleError> {
let mut cur = Cursor::new(to);
cur.write_all(&self.payload_size.to_le_bytes()).unwrap();
cur.write_all(&[if self.is_freed { 1 } else { 0 }]).unwrap();
cur.write_all(&self.desc_addr.addr().to_le_bytes()).unwrap();
cur.write_all(&self.payload_size.to_le_bytes())?;
cur.write_all(&[if self.is_freed { 1 } else { 0 }])?;
cur.write_all(&self.desc_addr.addr().to_le_bytes())?;
Ok(())
}
}

Expand All @@ -59,7 +65,10 @@ impl Storable for CompactFooter {
fn hydrate<T: CachedStore>(addr: u64, mem: &T) -> Result<Self, ShaleError> {
let raw = mem
.get_view(addr, Self::MSIZE)
.ok_or(ShaleError::LinearCachedStoreError)?;
.ok_or(ShaleError::InvalidCacheView {
offset: addr,
size: Self::MSIZE,
})?;
let payload_size = u64::from_le_bytes(raw.as_deref().try_into().unwrap());
Ok(Self { payload_size })
}
Expand All @@ -68,10 +77,9 @@ impl Storable for CompactFooter {
Self::MSIZE
}

fn dehydrate(&self, to: &mut [u8]) {
Cursor::new(to)
.write_all(&self.payload_size.to_le_bytes())
.unwrap();
fn dehydrate(&self, to: &mut [u8]) -> Result<(), ShaleError> {
Cursor::new(to).write_all(&self.payload_size.to_le_bytes())?;
Ok(())
}
}

Expand All @@ -89,9 +97,13 @@ impl Storable for CompactDescriptor {
fn hydrate<T: CachedStore>(addr: u64, mem: &T) -> Result<Self, ShaleError> {
let raw = mem
.get_view(addr, Self::MSIZE)
.ok_or(ShaleError::LinearCachedStoreError)?;
let payload_size = u64::from_le_bytes(raw.as_deref()[..8].try_into().unwrap());
let haddr = u64::from_le_bytes(raw.as_deref()[8..].try_into().unwrap());
.ok_or(ShaleError::InvalidCacheView {
offset: addr,
size: Self::MSIZE,
})?;
let payload_size =
u64::from_le_bytes(raw.as_deref()[..8].try_into().expect("invalid slice"));
let haddr = u64::from_le_bytes(raw.as_deref()[8..].try_into().expect("invalid slice"));
Ok(Self {
payload_size,
haddr,
Expand All @@ -102,10 +114,11 @@ impl Storable for CompactDescriptor {
Self::MSIZE
}

fn dehydrate(&self, to: &mut [u8]) {
fn dehydrate(&self, to: &mut [u8]) -> Result<(), ShaleError> {
let mut cur = Cursor::new(to);
cur.write_all(&self.payload_size.to_le_bytes()).unwrap();
cur.write_all(&self.haddr.to_le_bytes()).unwrap();
cur.write_all(&self.payload_size.to_le_bytes())?;
cur.write_all(&self.haddr.to_le_bytes())?;
Ok(())
}
}

Expand Down Expand Up @@ -158,11 +171,18 @@ impl Storable for CompactSpaceHeader {
fn hydrate<T: CachedStore + std::fmt::Debug>(addr: u64, mem: &T) -> Result<Self, ShaleError> {
let raw = mem
.get_view(addr, Self::MSIZE)
.ok_or(ShaleError::LinearCachedStoreError)?;
let meta_space_tail = u64::from_le_bytes(raw.as_deref()[..8].try_into().unwrap());
let compact_space_tail = u64::from_le_bytes(raw.as_deref()[8..16].try_into().unwrap());
let base_addr = u64::from_le_bytes(raw.as_deref()[16..24].try_into().unwrap());
let alloc_addr = u64::from_le_bytes(raw.as_deref()[24..].try_into().unwrap());
.ok_or(ShaleError::InvalidCacheView {
offset: addr,
size: Self::MSIZE,
})?;
let meta_space_tail =
u64::from_le_bytes(raw.as_deref()[..8].try_into().expect("invalid slice"));
let compact_space_tail =
u64::from_le_bytes(raw.as_deref()[8..16].try_into().expect("invalid slice"));
let base_addr =
u64::from_le_bytes(raw.as_deref()[16..24].try_into().expect("invalid slice"));
let alloc_addr =
u64::from_le_bytes(raw.as_deref()[24..].try_into().expect("invalid slice"));
Ok(Self {
meta_space_tail,
compact_space_tail,
Expand All @@ -175,14 +195,13 @@ impl Storable for CompactSpaceHeader {
Self::MSIZE
}

fn dehydrate(&self, to: &mut [u8]) {
fn dehydrate(&self, to: &mut [u8]) -> Result<(), ShaleError> {
let mut cur = Cursor::new(to);
cur.write_all(&self.meta_space_tail.to_le_bytes()).unwrap();
cur.write_all(&self.compact_space_tail.to_le_bytes())
.unwrap();
cur.write_all(&self.base_addr.addr().to_le_bytes()).unwrap();
cur.write_all(&self.alloc_addr.addr().to_le_bytes())
.unwrap();
cur.write_all(&self.meta_space_tail.to_le_bytes())?;
cur.write_all(&self.compact_space_tail.to_le_bytes())?;
cur.write_all(&self.base_addr.addr().to_le_bytes())?;
cur.write_all(&self.alloc_addr.addr().to_le_bytes())?;
Ok(())
}
}

Expand All @@ -196,16 +215,19 @@ impl<T> Storable for ObjPtrField<T> {
fn hydrate<U: CachedStore>(addr: u64, mem: &U) -> Result<Self, ShaleError> {
let raw = mem
.get_view(addr, Self::MSIZE)
.ok_or(ShaleError::LinearCachedStoreError)?;
Ok(Self(ObjPtr::new_from_addr(u64::from_le_bytes(
raw.as_deref().try_into().unwrap(),
))))
}

fn dehydrate(&self, to: &mut [u8]) {
Cursor::new(to)
.write_all(&self.0.addr().to_le_bytes())
.unwrap()
.ok_or(ShaleError::InvalidCacheView {
offset: addr,
size: Self::MSIZE,
})?;
let obj_ptr = ObjPtr::new_from_addr(u64::from_le_bytes(
<[u8; 8]>::try_from(&raw.as_deref()[0..8]).expect("invalid slice"),
));
Ok(Self(obj_ptr))
}

fn dehydrate(&self, to: &mut [u8]) -> Result<(), ShaleError> {
Cursor::new(to).write_all(&self.0.addr().to_le_bytes())?;
Ok(())
}

fn dehydrated_len(&self) -> u64 {
Expand Down Expand Up @@ -236,16 +258,20 @@ impl Storable for U64Field {
fn hydrate<U: CachedStore>(addr: u64, mem: &U) -> Result<Self, ShaleError> {
let raw = mem
.get_view(addr, Self::MSIZE)
.ok_or(ShaleError::LinearCachedStoreError)?;
.ok_or(ShaleError::InvalidCacheView {
offset: addr,
size: Self::MSIZE,
})?;
Ok(Self(u64::from_le_bytes(raw.as_deref().try_into().unwrap())))
}

fn dehydrated_len(&self) -> u64 {
Self::MSIZE
}

fn dehydrate(&self, to: &mut [u8]) {
Cursor::new(to).write_all(&self.0.to_le_bytes()).unwrap()
fn dehydrate(&self, to: &mut [u8]) -> Result<(), ShaleError> {
Cursor::new(to).write_all(&self.0.to_le_bytes())?;
Ok(())
}
}

Expand Down Expand Up @@ -570,7 +596,10 @@ impl<T: Storable + 'static, M: CachedStore> ShaleStore<T> for CompactSpace<T, M>
return Ok(r);
}
if ptr.addr() < CompactSpaceHeader::MSIZE {
return Err(ShaleError::ObjPtrInvalid);
return Err(ShaleError::InvalidAddressLength {
expected: CompactSpaceHeader::MSIZE,
found: ptr.addr(),
});
}
let h = inner.get_header(ObjPtr::new(ptr.addr() - CompactHeader::MSIZE))?;
Ok(inner
Expand Down Expand Up @@ -614,19 +643,25 @@ mod tests {
fn hydrate<T: CachedStore>(addr: u64, mem: &T) -> Result<Self, ShaleError> {
let raw = mem
.get_view(addr, Self::MSIZE)
.ok_or(ShaleError::LinearCachedStoreError)?;
.ok_or(ShaleError::InvalidCacheView {
offset: addr,
size: Self::MSIZE,
})?;
Ok(Self(
raw.as_deref()[..Self::MSIZE as usize].try_into().unwrap(),
raw.as_deref()[..Self::MSIZE as usize]
.try_into()
.expect("invalid slice"),
))
}

fn dehydrated_len(&self) -> u64 {
Self::MSIZE
}

fn dehydrate(&self, to: &mut [u8]) {
fn dehydrate(&self, to: &mut [u8]) -> Result<(), ShaleError> {
let mut cur = to;
cur.write_all(&self.0).unwrap()
cur.write_all(&self.0)?;
Ok(())
}
}

Expand All @@ -642,7 +677,7 @@ mod tests {
let compact_header: ObjPtr<CompactSpaceHeader> = ObjPtr::new_from_addr(0x1);
dm.write(
compact_header.addr(),
&crate::to_dehydrated(&CompactSpaceHeader::new(RESERVED, RESERVED)),
&crate::to_dehydrated(&CompactSpaceHeader::new(RESERVED, RESERVED)).unwrap(),
);
let compact_header =
StoredView::ptr_to_obj(&dm, compact_header, CompactHeader::MSIZE).unwrap();
Expand Down
Loading

0 comments on commit 503f4e5

Please sign in to comment.