From 9f4cc26c02be486487f161bb29ae9f43aaade49f Mon Sep 17 00:00:00 2001 From: Sergey Pepyakin Date: Wed, 3 Jul 2019 14:00:02 +0200 Subject: [PATCH] Results and polishing. --- src/memory/mmap_bytebuf.rs | 66 +++++++++++++++++++++++++++-------- src/memory/mod.rs | 70 +++++++++++++++++--------------------- src/memory/vec_bytebuf.rs | 16 ++++++--- 3 files changed, 95 insertions(+), 57 deletions(-) diff --git a/src/memory/mmap_bytebuf.rs b/src/memory/mmap_bytebuf.rs index 7dd4822..39c11ec 100644 --- a/src/memory/mmap_bytebuf.rs +++ b/src/memory/mmap_bytebuf.rs @@ -3,7 +3,7 @@ //! This implementation uses `mmap` on POSIX systems (and should use `VirtualAlloc` on windows). //! There are possibilities to improve the performance for the reallocating case by reserving //! memory up to maximum. This might be a problem for systems that don't have a lot of virtual -//! memory. +//! memory (i.e. 32-bit platforms). use std::ptr::{self, NonNull}; use std::slice; @@ -20,9 +20,20 @@ struct Mmap { } impl Mmap { - fn new(len: usize) -> Self { - assert!(len < isize::max_value() as usize); - assert!(len > 0); + /// Create a new mmap mapping + /// + /// Returns `Err` if: + /// - `len` should not exceed `isize::max_value()` + /// - `len` should be greater than 0. + /// - `mmap` returns an error (almost certainly means out of memory). + fn new(len: usize) -> Result { + if len >= isize::max_value() as usize { + return Err("`len` should not exceed `isize::max_value()`"); + } + if len == 0 { + return Err("`len` should be greater than 0"); + } + let ptr_or_err = unsafe { // Safety Proof: // There are not specific safety proofs are required for this call, since the call @@ -48,15 +59,19 @@ impl Mmap { }; match ptr_or_err as usize { - // `mmap` shouldn't return 0 since it has a special meaning. - x if x == 0 || x as isize == -1 => panic!(), + // `mmap` returns -1 in case of an error. + // `mmap` shouldn't return 0 since it has a special meaning for compilers. + // + // With the current parameters, the error can only be returned in case of insufficient + // memory. + x if x == 0 || x as isize == -1 => Err("mmap returned error"), _ => { let ptr = unsafe { // Safety Proof: // the ptr cannot be null as checked within the enclosing match. NonNull::new_unchecked(ptr_or_err as *mut u8) }; - Self { ptr, len } + Ok(Self { ptr, len }) } } } @@ -108,19 +123,23 @@ pub struct ByteBuf { } impl ByteBuf { - pub fn new(len: usize) -> Self { - let mmap = if len == 0 { None } else { Some(Mmap::new(len)) }; - Self { mmap } + pub fn new(len: usize) -> Result { + let mmap = if len == 0 { + None + } else { + Some(Mmap::new(len)?) + }; + Ok(Self { mmap }) } - pub fn realloc(&mut self, new_len: usize) { + pub fn realloc(&mut self, new_len: usize) -> Result<(), &'static str> { let new_mmap = if new_len == 0 { None } else { if self.len() == 0 { - Some(Mmap::new(new_len)) + Some(Mmap::new(new_len)?) } else { - let mut new_mmap = Mmap::new(new_len); + let mut new_mmap = Mmap::new(new_len)?; { let src = self.mmap.as_ref().unwrap().as_slice(); @@ -131,8 +150,8 @@ impl ByteBuf { Some(new_mmap) } }; - self.mmap = new_mmap; + Ok(()) } pub fn len(&self) -> usize { @@ -149,4 +168,23 @@ impl ByteBuf { .map(|m| m.as_slice_mut()) .unwrap_or(&mut []) } + + pub fn erase(&mut self) -> Result<(), &'static str> { + let cur_len = match self.mmap { + // Nothing to do here... + None => return Ok(()), + Some(Mmap { len: cur_len, .. }) => cur_len, + }; + + // The order is important. + // + // 1. First we clear, and thus drop, the current mmap if any. + // 2. And then we create a new one. + // + // Otherwise we double the peak memory consumption. + self.mmap = None; + self.mmap = Some(Mmap::new(cur_len)?); + + Ok(()) + } } diff --git a/src/memory/mod.rs b/src/memory/mod.rs index 21ffd8e..506d5e4 100644 --- a/src/memory/mod.rs +++ b/src/memory/mod.rs @@ -69,22 +69,16 @@ impl fmt::Debug for MemoryInstance { } } -#[cfg(all(unix, not(feature="vec_memory")))] -#[path="mmap_bytebuf.rs"] +#[cfg(all(unix, not(feature = "vec_memory")))] +#[path = "mmap_bytebuf.rs"] mod bytebuf; -#[cfg(any(not(unix), feature="vec_memory"))] -#[path="vec_bytebuf.rs"] +#[cfg(any(not(unix), feature = "vec_memory"))] +#[path = "vec_bytebuf.rs"] mod bytebuf; use self::bytebuf::ByteBuf; -// mod rust_alloc as byte_buf; -// use self::rust_alloc::ByteBuf; - -// mod vec_backed; -// use self::vec_backed::ByteBuf; - struct CheckedRegion { offset: usize, size: usize, @@ -141,22 +135,24 @@ impl MemoryInstance { validation::validate_memory(initial_u32, maximum_u32).map_err(Error::Memory)?; } - let memory = MemoryInstance::new(initial, maximum); + let memory = MemoryInstance::new(initial, maximum)?; Ok(MemoryRef(Rc::new(memory))) } /// Create new linear memory instance. - fn new(initial: Pages, maximum: Option) -> Self { + fn new(initial: Pages, maximum: Option) -> Result { let limits = ResizableLimits::new(initial.0 as u32, maximum.map(|p| p.0 as u32)); let initial_size: Bytes = initial.into(); - MemoryInstance { + Ok(MemoryInstance { limits: limits, - buffer: RefCell::new(ByteBuf::new(initial_size.0)), + buffer: RefCell::new( + ByteBuf::new(initial_size.0).map_err(|err| Error::Memory(err.to_string()))?, + ), initial: initial, current_size: Cell::new(initial_size.0), maximum: maximum, - } + }) } /// Return linear memory limits. @@ -290,8 +286,12 @@ impl MemoryInstance { } let new_buffer_length: Bytes = new_size.into(); + self.buffer + .borrow_mut() + .realloc(new_buffer_length.0) + .map_err(|err| Error::Memory(err.to_string()))?; + self.current_size.set(new_buffer_length.0); - self.buffer.borrow_mut().realloc(new_buffer_length.0); Ok(size_before_grow) } @@ -499,12 +499,14 @@ impl MemoryInstance { self.clear(offset, 0, len) } - /// Set every byte in the entire linear memory to 0. + /// Set every byte in the entire linear memory to 0, preserving its size. /// /// Might be useful for some optimization shenanigans. - pub fn erase(&self) { - let cur_size = self.buffer.borrow().len(); - *self.buffer.borrow_mut() = ByteBuf::new(cur_size); + pub fn erase(&self) -> Result<(), Error> { + self.buffer + .borrow_mut() + .erase() + .map_err(|err| Error::Memory(err.to_string())) } } @@ -518,30 +520,22 @@ mod tests { #[test] fn alloc() { - #[cfg(target_pointer_width = "64")] - let fixtures = &[ + let mut fixtures = vec![ (0, None, true), (0, Some(0), true), (1, None, true), (1, Some(1), true), (0, Some(1), true), (1, Some(0), false), - (0, Some(65536), true), - // TODO: Only use it for rust-alloc/mmap - // (65536, Some(65536), true), - // (65536, Some(0), false), - // (65536, None, true), ]; - #[cfg(target_pointer_width = "32")] - let fixtures = &[ - (0, None, true), - (0, Some(0), true), - (1, None, true), - (1, Some(1), true), - (0, Some(1), true), - (1, Some(0), false), - ]; + #[cfg(target_pointer_width = "64")] + fixtures.extend(&[ + + (65536, Some(65536), true), + (65536, Some(0), false), + (65536, None, true), + ]); for (index, &(initial, maybe_max, expected_ok)) in fixtures.iter().enumerate() { let initial: Pages = Pages(initial); @@ -563,7 +557,7 @@ mod tests { } fn create_memory(initial_content: &[u8]) -> MemoryInstance { - let mem = MemoryInstance::new(Pages(1), Some(Pages(1))); + let mem = MemoryInstance::new(Pages(1), Some(Pages(1))).unwrap(); mem.set(0, initial_content) .expect("Successful initialize the memory"); mem @@ -676,7 +670,7 @@ mod tests { #[test] fn get_into() { - let mem = MemoryInstance::new(Pages(1), None); + let mem = MemoryInstance::new(Pages(1), None).unwrap(); mem.set(6, &[13, 17, 129]) .expect("memory set should not fail"); diff --git a/src/memory/vec_bytebuf.rs b/src/memory/vec_bytebuf.rs index d210e0c..bc5595d 100644 --- a/src/memory/vec_bytebuf.rs +++ b/src/memory/vec_bytebuf.rs @@ -7,16 +7,15 @@ pub struct ByteBuf { } impl ByteBuf { - pub fn new(len: usize) -> Self { + pub fn new(len: usize) -> Result { let mut buf = Vec::new(); buf.resize(len, 0u8); - Self { - buf, - } + Ok(Self { buf }) } - pub fn realloc(&mut self, new_len: usize) { + pub fn realloc(&mut self, new_len: usize) -> Result<(), &'static str> { self.buf.resize(new_len, 0u8); + Ok(()) } pub fn len(&self) -> usize { @@ -30,4 +29,11 @@ impl ByteBuf { pub fn as_slice_mut(&mut self) -> &mut [u8] { self.buf.as_mut() } + + pub fn erase(&mut self) -> Result<(), &'static str> { + for v in &mut self.buf { + *v = 0; + } + Ok(()) + } }