From 3f962452bb321b6c93fe3f8e85eda295feb8da1e Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 10 Aug 2022 08:00:42 -0700 Subject: [PATCH 1/3] Rename `MmapVec::drain` to `split_off` As suggested on #4609 --- crates/runtime/src/mmap_vec.rs | 13 +++++++------ crates/wasmtime/src/module/serialization.rs | 2 +- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/crates/runtime/src/mmap_vec.rs b/crates/runtime/src/mmap_vec.rs index 2779668b20f0..56fc2e845491 100644 --- a/crates/runtime/src/mmap_vec.rs +++ b/crates/runtime/src/mmap_vec.rs @@ -73,14 +73,15 @@ impl MmapVec { self.mmap.is_readonly() } - /// "Drains" leading bytes up to the end specified in `range` from this + /// "Splits off" leading bytes up to the end specified in `range` from this /// `MmapVec`, returning a separately owned `MmapVec` which retains access /// to the bytes. /// - /// This method is similar to the `Vec` type's `drain` method, except that - /// the return value is not an iterator but rather a new `MmapVec`. The - /// purpose of this method is the ability to split-off new `MmapVec` values - /// which are sub-slices of the original one. + /// This method is similar to the `Vec` type's `split_off`` method, except + /// that the return value for a different portion of the internal vector: + /// the front and not the end. The purpose of this method is the ability to + /// split-off new `MmapVec` values which are sub-slices of the original + /// one. /// /// Once data has been drained from an `MmapVec` it is no longer accessible /// from the original `MmapVec`, it's only accessible from the returned @@ -91,7 +92,7 @@ impl MmapVec { /// to the bytes that come after the drain range. /// /// This is an `O(1)` operation which does not involve copies. - pub fn drain(&mut self, range: RangeTo) -> MmapVec { + pub fn split_off(&mut self, range: RangeTo) -> MmapVec { let amt = range.end; assert!(amt <= (self.range.end - self.range.start)); diff --git a/crates/wasmtime/src/module/serialization.rs b/crates/wasmtime/src/module/serialization.rs index 4dac54b391fc..bb06fcd55e90 100644 --- a/crates/wasmtime/src/module/serialization.rs +++ b/crates/wasmtime/src/module/serialization.rs @@ -335,7 +335,7 @@ impl<'a> SerializedModule<'a> { .section_headers(NE, data) .context("failed to read section headers")?; let range = subslice_range(object::bytes_of_slice(sections), data); - Ok(mmap.drain(..range.end)) + Ok(mmap.split_off(..range.end)) } } From aff90aad5c4b260a89d2c42f381a1d571d0ba6af Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 10 Aug 2022 09:00:14 -0700 Subject: [PATCH 2/3] Fix tests --- crates/runtime/src/mmap_vec.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/runtime/src/mmap_vec.rs b/crates/runtime/src/mmap_vec.rs index 56fc2e845491..de3a54689187 100644 --- a/crates/runtime/src/mmap_vec.rs +++ b/crates/runtime/src/mmap_vec.rs @@ -177,9 +177,9 @@ mod tests { fn drain() { let mut mmap = MmapVec::from_slice(&[1, 2, 3, 4]).unwrap(); assert_eq!(mmap.len(), 4); - assert!(mmap.drain(..0).is_empty()); + assert!(mmap.split_off(..0).is_empty()); assert_eq!(mmap.len(), 4); - let one = mmap.drain(..1); + let one = mmap.split_off(..1); assert_eq!(one.len(), 1); assert_eq!(one[0], 1); assert_eq!(mmap.len(), 3); @@ -187,16 +187,16 @@ mod tests { drop(one); assert_eq!(mmap.len(), 3); - let two = mmap.drain(..2); + let two = mmap.split_off(..2); assert_eq!(two.len(), 2); assert_eq!(two[0], 2); assert_eq!(two[1], 3); assert_eq!(mmap.len(), 1); assert_eq!(mmap[0], 4); drop(two); - assert!(mmap.drain(..0).is_empty()); - assert!(mmap.drain(..1).len() == 1); + assert!(mmap.split_off(..0).is_empty()); + assert!(mmap.split_off(..1).len() == 1); assert!(mmap.is_empty()); - assert!(mmap.drain(..0).is_empty()); + assert!(mmap.split_off(..0).is_empty()); } } From d5358abbc9839dd61cd7a6df263ae1d216346769 Mon Sep 17 00:00:00 2001 From: Jamey Sharp Date: Thu, 11 Aug 2022 12:54:44 -0700 Subject: [PATCH 3/3] Make MmapVec::split_off work like Vec::split_off --- crates/runtime/src/mmap_vec.rs | 75 ++++++++------------- crates/wasmtime/src/module/serialization.rs | 10 +-- 2 files changed, 34 insertions(+), 51 deletions(-) diff --git a/crates/runtime/src/mmap_vec.rs b/crates/runtime/src/mmap_vec.rs index de3a54689187..2eaedc5a3034 100644 --- a/crates/runtime/src/mmap_vec.rs +++ b/crates/runtime/src/mmap_vec.rs @@ -1,7 +1,7 @@ use crate::Mmap; use anyhow::{Context, Result}; use std::fs::File; -use std::ops::{Deref, DerefMut, Range, RangeTo}; +use std::ops::{Deref, DerefMut, Range}; use std::path::Path; use std::sync::Arc; @@ -73,37 +73,25 @@ impl MmapVec { self.mmap.is_readonly() } - /// "Splits off" leading bytes up to the end specified in `range` from this - /// `MmapVec`, returning a separately owned `MmapVec` which retains access - /// to the bytes. + /// Splits the collection into two at the given index. /// - /// This method is similar to the `Vec` type's `split_off`` method, except - /// that the return value for a different portion of the internal vector: - /// the front and not the end. The purpose of this method is the ability to - /// split-off new `MmapVec` values which are sub-slices of the original - /// one. - /// - /// Once data has been drained from an `MmapVec` it is no longer accessible - /// from the original `MmapVec`, it's only accessible from the returned - /// `MmapVec`. In other words ownership of the drain'd bytes is returned - /// through the `MmapVec` return value. - /// - /// This `MmapVec` will shrink by `range.end` bytes, and it will only refer - /// to the bytes that come after the drain range. + /// Returns a separate `MmapVec` which shares the underlying mapping, but + /// only has access to elements in the range `[at, len)`. After the call, + /// the original `MmapVec` will be left with access to the elements in the + /// range `[0, at)`. /// /// This is an `O(1)` operation which does not involve copies. - pub fn split_off(&mut self, range: RangeTo) -> MmapVec { - let amt = range.end; - assert!(amt <= (self.range.end - self.range.start)); + pub fn split_off(&mut self, at: usize) -> MmapVec { + assert!(at <= self.range.len()); // Create a new `MmapVec` which refers to the same underlying mmap, but // has a disjoint range from ours. Our own range is adjusted to be // disjoint just after `ret` is created. let ret = MmapVec { mmap: self.mmap.clone(), - range: self.range.start..self.range.start + amt, + range: at..self.range.end, }; - self.range.start += amt; + self.range.end = self.range.start + at; return ret; } @@ -174,29 +162,24 @@ mod tests { } #[test] - fn drain() { - let mut mmap = MmapVec::from_slice(&[1, 2, 3, 4]).unwrap(); - assert_eq!(mmap.len(), 4); - assert!(mmap.split_off(..0).is_empty()); - assert_eq!(mmap.len(), 4); - let one = mmap.split_off(..1); - assert_eq!(one.len(), 1); - assert_eq!(one[0], 1); - assert_eq!(mmap.len(), 3); - assert_eq!(&mmap[..], &[2, 3, 4]); - drop(one); - assert_eq!(mmap.len(), 3); - - let two = mmap.split_off(..2); - assert_eq!(two.len(), 2); - assert_eq!(two[0], 2); - assert_eq!(two[1], 3); - assert_eq!(mmap.len(), 1); - assert_eq!(mmap[0], 4); - drop(two); - assert!(mmap.split_off(..0).is_empty()); - assert!(mmap.split_off(..1).len() == 1); - assert!(mmap.is_empty()); - assert!(mmap.split_off(..0).is_empty()); + fn split_off() { + let mut vec = Vec::from([1, 2, 3, 4]); + let mut mmap = MmapVec::from_slice(&vec).unwrap(); + assert_eq!(&mmap[..], &vec[..]); + // remove nothing; vec length remains 4 + assert_eq!(&mmap.split_off(4)[..], &vec.split_off(4)[..]); + assert_eq!(&mmap[..], &vec[..]); + // remove 1 element; vec length is now 3 + assert_eq!(&mmap.split_off(3)[..], &vec.split_off(3)[..]); + assert_eq!(&mmap[..], &vec[..]); + // remove 2 elements; vec length is now 1 + assert_eq!(&mmap.split_off(1)[..], &vec.split_off(1)[..]); + assert_eq!(&mmap[..], &vec[..]); + // remove last element; vec length is now 0 + assert_eq!(&mmap.split_off(0)[..], &vec.split_off(0)[..]); + assert_eq!(&mmap[..], &vec[..]); + // nothing left to remove, but that's okay + assert_eq!(&mmap.split_off(0)[..], &vec.split_off(0)[..]); + assert_eq!(&mmap[..], &vec[..]); } } diff --git a/crates/wasmtime/src/module/serialization.rs b/crates/wasmtime/src/module/serialization.rs index bb06fcd55e90..bb0d7fd401dc 100644 --- a/crates/wasmtime/src/module/serialization.rs +++ b/crates/wasmtime/src/module/serialization.rs @@ -268,11 +268,11 @@ impl<'a> SerializedModule<'a> { // First validate that this is at least somewhat an elf file within // `mmap` and additionally skip to the end of the elf file to find our // metadata. - let elf = take_first_elf(&mut mmap)?; + let metadata = data_after_elf(&mut mmap)?; // The metadata has a few guards up front which we process first, and // eventually this bottoms out in a `bincode::deserialize` call. - let metadata = mmap + let metadata = metadata .strip_prefix(HEADER) .ok_or_else(|| anyhow!("bytes are not a compatible serialized wasmtime module"))?; if metadata.is_empty() { @@ -309,13 +309,13 @@ impl<'a> SerializedModule<'a> { .context("deserialize compilation artifacts")?; return Ok(SerializedModule { - artifacts: MyCow::Owned(elf), + artifacts: MyCow::Owned(mmap), metadata, }); /// This function will return the trailing data behind the ELF file /// parsed from `data` which is where we find our metadata section. - fn take_first_elf(mmap: &mut MmapVec) -> Result { + fn data_after_elf(mmap: &mut MmapVec) -> Result { use object::NativeEndian as NE; // There's not actually a great utility for figuring out where // the end of an ELF file is in the `object` crate. In lieu of that @@ -335,7 +335,7 @@ impl<'a> SerializedModule<'a> { .section_headers(NE, data) .context("failed to read section headers")?; let range = subslice_range(object::bytes_of_slice(sections), data); - Ok(mmap.split_off(..range.end)) + Ok(mmap.split_off(range.end)) } }