Switch VecStorage to Vec<MaybeUninit<T>>

Author: willglynn

.travis.yml

@@ -2,7 +2,7 @@ language: rust
 
 rust:
 - nightly
-- 1.34.0
+- 1.36.0
 - stable
 
 cache:
@@ -22,7 +22,7 @@ script:
     cargo build --all-features --verbose;
     cargo test --all-features --verbose --no-run;
     cargo bench --verbose --no-run --all-features;
-  elif [ "$TRAVIS_RUST_VERSION" == "1.34.0" ]; then
+  elif [ "$TRAVIS_RUST_VERSION" == "1.36.0" ]; then
     cargo check --tests --no-default-features;
     cargo check --tests --no-default-features --features "parallel";
     cargo check --tests --no-default-features --features "serde";

README.md

@@ -35,7 +35,7 @@ Unlike most other ECS libraries out there, it provides
       other and you can use barriers to force several stages in system execution
 * high performance for real-world applications
 
-Minimum Rust version: 1.34
+Minimum Rust version: 1.36
 
 ## [Link to the book][book]
 

docs/tutorials/src/05_storages.md

@@ -50,11 +50,11 @@ to the corresponding heading.
 
 Certain storages provide access to component slices:  
 
-|Storage Type            | Safety  | Density | Indices       |
-|:----------------------:|---------|---------|---------------|
-| [`DenseVecStorage`]    | Safe    | Dense   | Arbitrary     |
-| [`VecStorage`]         | Unsafe  | Sparse  | Entity `id()` |
-| [`DefaultVecStorage`]  | Safe    | Sparse  | Entity `id()` |
+|Storage Type            | Slice type          | Density | Indices       |
+|:----------------------:|---------------------|---------|---------------|
+| [`DenseVecStorage`]    | `&[T]`              | Dense   | Arbitrary     |
+| [`VecStorage`]         | `&[MaybeUninit<T>]` | Sparse  | Entity `id()` |
+| [`DefaultVecStorage`]  | `&[T]`              | Sparse  | Entity `id()` |
 
 This is intended as an advanced technique. Component slices provide
 maximally efficient reads and writes, but they are incompatible with
@@ -73,10 +73,10 @@ one which provides a mapping from the entity id to the index for the data vec
 (it's a redirection table). This is useful when your component is bigger
 than a `usize` because it consumes less RAM.
 
-`DenseVecStorage` provides `as_slice()` and `as_mut_slice()` accessors to
-directly access component data. The indices in this slice do not correspond
-to entity IDs, nor do they correspond to indices in any other storage, nor
-do they correspond to indices in this storage at a different point in time.
+`DefaultVecStorage<T>` provides `as_slice()` and `as_mut_slice()` accessors
+which return `&[T]`. The indices in this slice do not correspond to entity
+IDs, nor do they correspond to indices in any other storage, nor do they
+correspond to indices in this storage at a different point in time.
 
 ## `HashMapStorage`
 
@@ -103,11 +103,10 @@ Therefore it would be a waste of memory to use this storage for
 rare components, but it's best suited for commonly used components
 (like transform values).
 
-`VecStorage` provides unsafe `as_slice()` and `as_mut_slice()` accessors.
-These functions are `unsafe` because the slices contain uninitialized and
-dropped values. (Consult the `Storage::mask()` to determine which indices
-are populated.) Slice indices cannot be converted to `Entity` values
-because they lack a generation counter, but they do correspond to
+`VecStorage<T>` provides `as_slice()` and `as_mut_slice()` accessors which
+return `&[MaybeUninit<T>]`. Consult the `Storage::mask()` to determine
+which indices are populated. Slice indices cannot be converted to `Entity`
+values because they lack a generation counter, but they do correspond to
 `Entity::id()`s, so indices can be used to collate between multiple
 `VecStorage`s.
 
@@ -118,8 +117,9 @@ uninitialized, it fills them with the component's default value. This
 requires the component to `impl Default`, and it results in more memory
 writes than `VecStorage`.
 
-`DefaultVecStorage` provides `as_slice()` and `as_mut_slice()` accessors to
-directly access component data. Slice usage is equivalent to `VecStorage`,
-except that every value is initialized so the resulting slice can be safely
-used without checking the mask. `DefaultVecStorage` indices all correspond
-with each other and with `VecStorage` indices.
+`DefaultVecStorage` provides `as_slice()` and `as_mut_slice()` accessors
+which return `&[T]`. `Storage::mask()` can be used to determine which
+indices are in active use, but all indices are fully initialized, so the
+`mask()` is not necessary for safety. `DefaultVecStorage` indices all
+correspond with each other, with `VecStorage` indices, and with
+`Entity::id()`s.

src/storage/mod.rs

@@ -13,7 +13,7 @@ pub use self::{
     track::{ComponentEvent, Tracked},
 };
 
-use self::storages::{SafeSliceAccess, UnsafeSliceAccess};
+use self::storages::SliceAccess;
 
 use std::{
     self,
@@ -265,13 +265,13 @@ impl<'e, T, D> Storage<'e, T, D>
     where
         T: Component,
         D: Deref<Target = MaskedStorage<T>>,
-        T::Storage: SafeSliceAccess<T>
+        T::Storage: SliceAccess<T>
 {
     /// Returns the component data as a slice.
     ///
     /// The indices of this slice may not correspond to anything in particular.
     /// Check the underlying storage documentation for details.
-    pub fn as_slice(&self) -> &[T] {
+    pub fn as_slice(&self) -> &[<T::Storage as SliceAccess<T>>::Element] {
         self.data.inner.as_slice()
     }
 }
@@ -280,55 +280,17 @@ impl<'e, T, D> Storage<'e, T, D>
     where
         T: Component,
         D: DerefMut<Target = MaskedStorage<T>>,
-        T::Storage: SafeSliceAccess<T>
+        T::Storage: SliceAccess<T>
 {
     /// Returns the component data as a slice.
     ///
     /// The indices of this slice may not correspond to anything in particular.
     /// Check the underlying storage documentation for details.
-    pub fn as_mut_slice(&mut self) -> &mut [T] {
+    pub fn as_mut_slice(&mut self) -> &mut [<T::Storage as SliceAccess<T>>::Element] {
         self.data.inner.as_mut_slice()
     }
 }
 
-impl<'e, T, D> Storage<'e, T, D>
-    where
-        T: Component,
-        D: Deref<Target = MaskedStorage<T>>,
-        T::Storage: UnsafeSliceAccess<T>
-{
-    /// Returns the component data as a slice.
-    ///
-    /// The indices of this slice may not correspond to anything in particular.
-    /// Check the underlying storage documentation for details.
-    ///
-    /// # Safety
-    ///
-    /// This slice contains uninitialized or dropped data.
-    pub unsafe fn unsafe_slice(&self) -> &[T] {
-        self.data.inner.unsafe_slice()
-    }
-}
-
-impl<'e, T, D> Storage<'e, T, D>
-    where
-        T: Component,
-        D: DerefMut<Target = MaskedStorage<T>>,
-        T::Storage: UnsafeSliceAccess<T>
-{
-    /// Returns the component data as a slice.
-    ///
-    /// The indices of this slice may not correspond to anything in particular.
-    /// Check the underlying storage documentation for details.
-    ///
-    /// # Safety
-    ///
-    /// This slice contains uninitialized or dropped data.
-    pub unsafe fn unsafe_mut_slice(&mut self) -> &mut [T] {
-        self.data.inner.unsafe_mut_slice()
-    }
-}
-
 impl<'e, T, D> Storage<'e, T, D>
 where
     T: Component,

src/storage/storages.rs

@@ -1,6 +1,7 @@
 //! Different types of storages you can use for your components.
 
 use std::collections::BTreeMap;
+use std::mem::MaybeUninit;
 
 use derivative::Derivative;
 use hashbrown::HashMap;
@@ -11,14 +12,16 @@ use crate::{
     world::Index,
 };
 
-pub trait SafeSliceAccess<T> {
-    fn as_slice(&self) -> &[T];
-    fn as_mut_slice(&mut self) -> &mut [T];
-}
-
-pub trait UnsafeSliceAccess<T> {
-    unsafe fn unsafe_slice(&self) -> &[T];
-    unsafe fn unsafe_mut_slice(&mut self) -> &mut [T];
+/// Some storages can provide slices to access the underlying data.
+///
+/// The underlying data may be of type `T`, or it may be of a type
+/// which wraps `T`. The associated type `Element` identifies what
+/// the slices will contain.
+pub trait SliceAccess<T> {
+    type Element;
+
+    fn as_slice(&self) -> &[Self::Element];
+    fn as_mut_slice(&mut self) -> &mut [Self::Element];
 }
 
 /// BTreeMap-based storage.
@@ -107,13 +110,15 @@ pub struct DenseVecStorage<T> {
     data_id: Vec<Index>,
 }
 
-impl<T> SafeSliceAccess<T> for DenseVecStorage<T> {
+impl<T> SliceAccess<T> for DenseVecStorage<T> {
+    type Element = T;
+
     /// Returns a slice of all the components in this storage.
     ///
     /// Indices inside the slice do not correspond to anything in particular, and
     /// especially do not correspond with entity IDs.
     #[inline]
-    fn as_slice(&self) -> &[T] {
+    fn as_slice(&self) -> &[Self::Element] {
         self.data.as_slice()
     }
 
@@ -122,7 +127,7 @@ impl<T> SafeSliceAccess<T> for DenseVecStorage<T> {
     /// Indices inside the slice do not correspond to anything in particular, and
     /// especially do not correspond with entity IDs.
     #[inline]
-    fn as_mut_slice(&mut self) -> &mut [T] {
+    fn as_mut_slice(&mut self) -> &mut [Self::Element] {
         self.data.as_mut_slice()
     }
 }
@@ -216,34 +221,24 @@ unsafe impl<T> DistinctStorage for NullStorage<T> {}
 /// Vector storage. Uses a simple `Vec`. Supposed to have maximum
 /// performance for the components mostly present in entities.
 ///
-/// `unsafe_slice()` and `unsafe_mut_slice()` indices correspond to
+/// `as_slice()` and `as_mut_slice()` indices correspond to
 /// entity IDs. These can be compared to other `VecStorage`s, to
 /// other `DefaultVecStorage`s, and to `Entity::id()`s for live
 /// entities.
 #[derive(Derivative)]
 #[derivative(Default(bound = ""))]
-pub struct VecStorage<T>(Vec<T>);
+pub struct VecStorage<T>(Vec<MaybeUninit<T>>);
+
+impl<T> SliceAccess<T> for VecStorage<T> {
+    type Element = MaybeUninit<T>;
 
-impl<T> UnsafeSliceAccess<T> for VecStorage<T> {
-    /// Returns a slice of all the components in this storage.
-    ///
-    /// # Safety
-    ///
-    /// This slice contains uninitialized and dropped values. If this is a
-    /// problem, consider using `DefaultVecStorage` or `DenseVecStorage` instead.
     #[inline]
-    unsafe fn unsafe_slice(&self) -> &[T] {
+    fn as_slice(&self) -> &[Self::Element] {
         self.0.as_slice()
     }
 
-    /// Returns a mutable slice of all the components in this storage.
-    ///
-    /// # Safety
-    ///
-    /// This slice contains uninitialized and dropped values. If this is a
-    /// problem, consider using `DefaultVecStorage` or `DenseVecStorage` instead.
     #[inline]
-    unsafe fn unsafe_mut_slice(&mut self) -> &mut [T] {
+    fn as_mut_slice(&mut self) -> &mut [Self::Element] {
         self.0.as_mut_slice()
     }
 }
@@ -256,23 +251,22 @@ impl<T> UnprotectedStorage<T> for VecStorage<T> {
         use std::ptr;
         for (i, v) in self.0.iter_mut().enumerate() {
             if has.contains(i as u32) {
-                ptr::drop_in_place(v);
+                // drop in place
+                ptr::drop_in_place(&mut *v.as_mut_ptr());
             }
         }
         self.0.set_len(0);
     }
 
     unsafe fn get(&self, id: Index) -> &T {
-        self.0.get_unchecked(id as usize)
+        &*self.0.get_unchecked(id as usize).as_ptr()
     }
 
     unsafe fn get_mut(&mut self, id: Index) -> &mut T {
-        self.0.get_unchecked_mut(id as usize)
+        &mut *self.0.get_unchecked_mut(id as usize).as_mut_ptr()
     }
 
     unsafe fn insert(&mut self, id: Index, v: T) {
-        use std::ptr;
-
         let id = id as usize;
         if self.0.len() <= id {
             let delta = id + 1 - self.0.len();
@@ -281,12 +275,11 @@ impl<T> UnprotectedStorage<T> for VecStorage<T> {
         }
         // Write the value without reading or dropping
         // the (currently uninitialized) memory.
-        ptr::write(self.0.get_unchecked_mut(id), v);
+        *self.0.get_unchecked_mut(id as usize) = MaybeUninit::new(v);
     }
 
     unsafe fn remove(&mut self, id: Index) -> T {
         use std::ptr;
-
         ptr::read(self.get(id))
     }
 }
@@ -347,16 +340,18 @@ impl<T> UnprotectedStorage<T> for DefaultVecStorage<T> where T: Default {
 
 unsafe impl<T> DistinctStorage for DefaultVecStorage<T> {}
 
-impl<T> SafeSliceAccess<T> for DefaultVecStorage<T> {
+impl<T> SliceAccess<T> for DefaultVecStorage<T> {
+    type Element = T;
+
     /// Returns a slice of all the components in this storage.
     #[inline]
-    fn as_slice(&self) -> &[T] {
+    fn as_slice(&self) -> &[Self::Element] {
         self.0.as_slice()
     }
 
     /// Returns a mutable slice of all the components in this storage.
     #[inline]
-    fn as_mut_slice(&mut self) -> &mut [T] {
+    fn as_mut_slice(&mut self) -> &mut [Self::Element] {
         self.0.as_mut_slice()
     }
 }