From c1d8a96093712b47b2c171e605e17f42a4757338 Mon Sep 17 00:00:00 2001 From: Will Glynn Date: Thu, 29 Aug 2019 12:45:33 -0500 Subject: [PATCH 1/3] Add slice access --- docs/tutorials/src/05_storages.md | 47 +++++++++++-- examples/slices.rs | 107 +++++++++++++++++++++++++++++ src/lib.rs | 4 +- src/prelude.rs | 4 +- src/storage/mod.rs | 34 ++++++++- src/storage/storages.rs | 110 ++++++++++++++++++++++++++++++ src/storage/tests.rs | 98 ++++++++++++++++++++++++++ 7 files changed, 392 insertions(+), 12 deletions(-) create mode 100644 examples/slices.rs diff --git a/docs/tutorials/src/05_storages.md b/docs/tutorials/src/05_storages.md index 1a23843b0..a16439bd7 100644 --- a/docs/tutorials/src/05_storages.md +++ b/docs/tutorials/src/05_storages.md @@ -30,19 +30,34 @@ has 4 layers). Here a list of the storages with a short description and a link to the corresponding heading. -|Storage Type |Description |Optimized for | -|:-------------------:|--------------------------|------------------------------| -| [`BTreeStorage`] | Works with a `BTreeMap` | no particular case | -| [`DenseVecStorage`] | Uses a redirection table | fairly often used components | -| [`HashMapStorage`] | Uses a `HashMap` | rare components | -| [`NullStorage`] | Can flag entities | doesn't depend on rarity | -| [`VecStorage`] | Uses a sparse `Vec` | commonly used components | +|Storage Type |Description |Optimized for | +|:----------------------:|----------------------------------------------------|------------------------------| +| [`BTreeStorage`] | Works with a `BTreeMap` | no particular case | +| [`DenseVecStorage`] | Uses a redirection table | fairly often used components | +| [`HashMapStorage`] | Uses a `HashMap` | rare components | +| [`NullStorage`] | Can flag entities | doesn't depend on rarity | +| [`VecStorage`] | Uses a sparse `Vec`, empty slots are uninitialized | commonly used components | +| [`DefaultVecStorage`] | Uses a sparse `Vec`, empty slots contain `Default` | commonly used components | [`BTreeStorage`]: #btreestorage [`DenseVecStorage`]: #densevecstorage [`HashMapStorage`]: #hashmapstorage [`NullStorage`]: #nullstorage [`VecStorage`]: #vecstorage +[`DefaultVecStorage`]: #defaultvecstorage + +## Slices + +Certain storages provide access to component slices: + +|Storage Type | Slice type | Density | Indices | +|:----------------------:|---------------------|---------|---------------| +| [`DenseVecStorage`] | `&[T]` | Dense | Arbitrary | +| [`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 +many of the usual abstractions which makes them more difficult to use. ## `BTreeStorage` @@ -57,6 +72,11 @@ 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. +`DefaultVecStorage` 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` This should be used for components which are associated with very few entities, @@ -81,3 +101,16 @@ just leaves uninitialized gaps where we don't have any component. 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). + +## `DefaultVecStorage` + +This storage works exactly like `VecStorage`, but instead of leaving gaps +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 +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 and with `Entity::id()`s. diff --git a/examples/slices.rs b/examples/slices.rs new file mode 100644 index 000000000..6f81557f3 --- /dev/null +++ b/examples/slices.rs @@ -0,0 +1,107 @@ +extern crate specs; + +use specs::prelude::*; + +// A component contains data which is associated with an entity. + +#[derive(Debug)] +struct Vel(f32); + +impl Component for Vel { + type Storage = DefaultVecStorage; +} + +impl Default for Vel { + fn default() -> Self { + Self(0.0) + } +} + +#[derive(Debug)] +struct Pos(f32); + +impl Component for Pos { + type Storage = DefaultVecStorage; +} + +impl Default for Pos { + fn default() -> Self { + Self(0.0) + } +} + +struct SysA; + +impl<'a> System<'a> for SysA { + // These are the resources required for execution. + // You can also define a struct and `#[derive(SystemData)]`, + // see the `full` example. + type SystemData = (WriteStorage<'a, Pos>, ReadStorage<'a, Vel>); + + fn run(&mut self, (mut pos, vel): Self::SystemData) { + // Both the `Pos` and `Vel` components use `DefaultVecStorage`, which supports + // `as_slice()` and `as_mut_slice()`. This lets us access components without + // indirection. + let mut pos_slice = pos.as_mut_slice(); + let vel_slice = vel.as_slice(); + + // Note that an entity which has position but not velocity will still have + // an entry in both slices. `DefaultVecStorage` is sparse, and here is where + // that matters: the storage has space for many entities, but not all of them + // contain meaningful values. These slices may be a mix of present and absent + // (`Default`) data. + // + // We could check the `mask()` before reading the velocity and updating the + // position, and this is what `.join()` normally does. However, because: + // + // 1. `Vel` uses `DefaultVecStorage`, + // 2. `Vel`'s default is 0.0, and + // 3. `Pos` += 0.0 is a no-op, + // + // we can unconditionally add `Vel` to `Pos` without reading the `mask()`! + // This results in a tight inner loop of known size, which is especially + // suitable for SIMD, OpenCL, CUDA, and other accelerator technologies. + // + // Finally, note that `DefaultVecStorage` and `VecStorage` slice indices + // always agree. If an entity is at location `i` in one `VecStorage`, it + // will be at location `i` in every other `VecStorage`. (By contrast, + // `DenseVecStorage` uses unpredictable indices and cannot be used in + // this way.) We need only worry about handling slices of different + // lengths. + let len = pos_slice.len().min(vel_slice.len()); + for i in 0..len { + pos_slice[i].0 += vel_slice[i].0; + } + } +} + +fn main() { + // The `World` is our + // container for components + // and other resources. + + let mut world = World::new(); + + // This builds a dispatcher. + // The third parameter of `add` specifies + // logical dependencies on other systems. + // Since we only have one, we don't depend on anything. + // See the `full` example for dependencies. + let mut dispatcher = DispatcherBuilder::new().with(SysA, "sys_a", &[]).build(); + + // setup() must be called before creating any entity, it will register + // all Components and Resources that Systems depend on + dispatcher.setup(&mut world); + + // An entity may or may not contain some component. + + world.create_entity().with(Vel(2.0)).with(Pos(0.0)).build(); + world.create_entity().with(Vel(4.0)).with(Pos(1.6)).build(); + world.create_entity().with(Vel(1.5)).with(Pos(5.4)).build(); + + // This entity does not have `Vel`, so it won't be dispatched. + world.create_entity().with(Pos(2.0)).build(); + + // This dispatches all the systems in parallel (but blocking). + dispatcher.dispatch(&world); +} diff --git a/src/lib.rs b/src/lib.rs index f1cf1da4b..c2b74a3b9 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -224,8 +224,8 @@ pub use crate::{ changeset::ChangeSet, join::Join, storage::{ - DenseVecStorage, FlaggedStorage, HashMapStorage, NullStorage, ReadStorage, Storage, - Tracked, VecStorage, WriteStorage, + DenseVecStorage, DefaultVecStorage, FlaggedStorage, HashMapStorage, NullStorage, + ReadStorage, Storage, Tracked, VecStorage, WriteStorage, }, world::{Builder, Component, Entities, Entity, EntityBuilder, LazyUpdate, WorldExt}, }; diff --git a/src/prelude.rs b/src/prelude.rs index eee349fd2..d7ec22c06 100644 --- a/src/prelude.rs +++ b/src/prelude.rs @@ -20,8 +20,8 @@ pub use shred::AsyncDispatcher; pub use crate::{ changeset::ChangeSet, storage::{ - ComponentEvent, DenseVecStorage, FlaggedStorage, HashMapStorage, NullStorage, ReadStorage, - Storage, Tracked, VecStorage, WriteStorage, + ComponentEvent, DenseVecStorage, DefaultVecStorage, FlaggedStorage, HashMapStorage, + NullStorage, ReadStorage, Storage, Tracked, VecStorage, WriteStorage, }, world::{Builder, Component, Entities, Entity, EntityBuilder, LazyUpdate, WorldExt}, }; diff --git a/src/storage/mod.rs b/src/storage/mod.rs index d6ccc2887..13404f2af 100644 --- a/src/storage/mod.rs +++ b/src/storage/mod.rs @@ -9,10 +9,12 @@ pub use self::{ ImmutableParallelRestriction, MutableParallelRestriction, RestrictedStorage, SequentialRestriction, }, - storages::{BTreeStorage, DenseVecStorage, HashMapStorage, NullStorage, VecStorage}, + storages::{BTreeStorage, DenseVecStorage, DefaultVecStorage, HashMapStorage, NullStorage, VecStorage}, track::{ComponentEvent, Tracked}, }; +use self::storages::SliceAccess; + use std::{ self, marker::PhantomData, @@ -259,6 +261,36 @@ where } } +impl<'e, T, D> Storage<'e, T, D> + where + T: Component, + D: Deref>, + T::Storage: SliceAccess +{ + /// 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) -> &[>::Element] { + self.data.inner.as_slice() + } +} + +impl<'e, T, D> Storage<'e, T, D> + where + T: Component, + D: DerefMut>, + T::Storage: SliceAccess +{ + /// 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 [>::Element] { + self.data.inner.as_mut_slice() + } +} + impl<'e, T, D> Storage<'e, T, D> where T: Component, diff --git a/src/storage/storages.rs b/src/storage/storages.rs index e3793973e..95985c44f 100644 --- a/src/storage/storages.rs +++ b/src/storage/storages.rs @@ -11,6 +11,18 @@ use crate::{ world::Index, }; +/// 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 { + type Element; + + fn as_slice(&self) -> &[Self::Element]; + fn as_mut_slice(&mut self) -> &mut [Self::Element]; +} + /// BTreeMap-based storage. #[derive(Derivative)] #[derivative(Default(bound = ""))] @@ -83,6 +95,12 @@ unsafe impl DistinctStorage for HashMapStorage {} /// /// Note that this only stores the data (`T`) densely; indices /// to the data are stored in a sparse `Vec`. +/// +/// `as_slice()` and `as_mut_slice()` indices are local to this +/// `DenseVecStorage` at this particular moment. These indices +/// cannot be compared with indices from any other storage, and +/// a particular entity's position within this slice may change +/// over time. #[derive(Derivative)] #[derivative(Default(bound = ""))] pub struct DenseVecStorage { @@ -91,6 +109,28 @@ pub struct DenseVecStorage { data_id: Vec, } +impl SliceAccess for DenseVecStorage { + 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) -> &[Self::Element] { + self.data.as_slice() + } + + /// Returns a mutable 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_mut_slice(&mut self) -> &mut [Self::Element] { + self.data.as_mut_slice() + } +} + impl UnprotectedStorage for DenseVecStorage { unsafe fn clean(&mut self, _has: B) where @@ -227,3 +267,73 @@ impl UnprotectedStorage for VecStorage { } unsafe impl DistinctStorage for VecStorage {} + +/// Vector storage, like `VecStorage`, but allows safe access to the +/// interior slices because unused slots are always initialized. +/// +/// Requires the component to implement `Default`. +/// +/// `as_slice()` and `as_mut_slice()` indices correspond to entity IDs. +/// These can be compared to other `DefaultVecStorage`s, and to +/// `Entity::id()`s for live entities. +#[derive(Derivative)] +#[derivative(Default(bound = ""))] +pub struct DefaultVecStorage(Vec); + +impl UnprotectedStorage for DefaultVecStorage where T: Default { + unsafe fn clean(&mut self, _has: B) + where + B: BitSetLike, + { + self.0.clear(); + } + + unsafe fn get(&self, id: Index) -> &T { + self.0.get_unchecked(id as usize) + } + + unsafe fn get_mut(&mut self, id: Index) -> &mut T { + self.0.get_unchecked_mut(id as usize) + } + + unsafe fn insert(&mut self, id: Index, v: T) { + let id = id as usize; + + if self.0.len() <= id { + // fill all the empty slots with default values + self.0.resize_with(id, Default::default); + // store the desired value + self.0.push(v) + } else { + // store the desired value directly + self.0[id] = v; + } + } + + unsafe fn remove(&mut self, id: Index) -> T { + // make a new default value + let mut v = T::default(); + // swap it into the vec + std::ptr::swap(self.0.get_unchecked_mut(id as usize), &mut v); + // return the old value + return v; + } +} + +unsafe impl DistinctStorage for DefaultVecStorage {} + +impl SliceAccess for DefaultVecStorage { + type Element = T; + + /// Returns a slice of all the components in this storage. + #[inline] + 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 [Self::Element] { + self.0.as_mut_slice() + } +} diff --git a/src/storage/tests.rs b/src/storage/tests.rs index b87158acd..b6ee6493f 100644 --- a/src/storage/tests.rs +++ b/src/storage/tests.rs @@ -225,6 +225,20 @@ mod test { type Storage = VecStorage; } + #[derive(PartialEq, Eq, Debug, Default)] + struct CdefaultVec(u32); + impl From for CdefaultVec { + fn from(v: u32) -> CdefaultVec { CdefaultVec(v) } + } + impl AsMut for CdefaultVec { + fn as_mut(&mut self) -> &mut u32 { + &mut self.0 + } + } + impl Component for CdefaultVec { + type Storage = DefaultVecStorage; + } + fn test_add + Debug + Eq>() where T::Storage: Default, @@ -414,6 +428,26 @@ mod test { } } + fn test_slice_access + Debug + Eq>() + where + T::Storage: Default + SliceAccess, + { + let mut w = World::new(); + let mut s: Storage = create(&mut w); + + for i in 0..1_000 { + if let Err(err) = s.insert(Entity::new(i, Generation::new(1)), (i + 2718).into()) { + panic!("Failed to insert component into entity! {:?}", err); + } + } + + let slice = s.as_slice(); + assert_eq!(slice.len(), 1_000); + for (i, v) in slice.iter().enumerate() { + assert_eq!(v, &(i as u32 + 2718).into()); + } + } + #[test] fn vec_test_add() { test_add::(); @@ -466,6 +500,70 @@ mod test { } } + #[test] + fn default_vec_test_add() { + test_add::(); + } + #[test] + fn default_vec_test_sub() { + test_sub::(); + } + #[test] + fn default_vec_test_get_mut() { + test_get_mut::(); + } + #[test] + fn default_vec_test_get_mut_or_default() { + test_get_mut_or_default::(); + } + #[test] + fn default_vec_test_add_gen() { + test_add_gen::(); + } + #[test] + fn default_vec_test_sub_gen() { + test_sub_gen::(); + } + #[test] + fn default_vec_test_clear() { + test_clear::(); + } + #[test] + fn default_vec_test_anti() { + test_anti::(); + } + #[test] + fn default_vec_test_slice_access() { + test_slice_access::(); + } + + #[test] + fn default_vec_test_defaults() { + let mut w = World::new(); + let mut s: Storage = create(&mut w); + + // insert 1 and 3 at 1 and 3 + s.insert(Entity::new(1, Generation::new(1)), 1.into()).unwrap(); + s.insert(Entity::new(3, Generation::new(1)), 3.into()).unwrap(); + + // should contain default values at other locations + assert_eq!(s.as_slice(), &[ + CdefaultVec(0), + CdefaultVec(1), + CdefaultVec(0), + CdefaultVec(3), + ]); + + // deleting the record 3 should swap in the default but not shrink + s.remove(Entity::new(3, Generation::new(1))); + assert_eq!(s.as_slice(), &[ + CdefaultVec(0), + CdefaultVec(1), + CdefaultVec(0), + CdefaultVec(0), + ]); + } + #[test] fn hash_test_add() { test_add::(); From 20103792b93f78803e0c4bac060b4406902e81c0 Mon Sep 17 00:00:00 2001 From: Will Glynn Date: Thu, 29 Aug 2019 12:45:54 -0500 Subject: [PATCH 2/3] Switch VecStorage to MaybeUninit and add slice access --- .travis.yml | 4 +-- README.md | 2 +- docs/tutorials/src/05_storages.md | 11 +++++++- src/storage/storages.rs | 42 ++++++++++++++++++++++--------- src/storage/tests.rs | 26 +++++++++++++++++++ 5 files changed, 69 insertions(+), 16 deletions(-) diff --git a/.travis.yml b/.travis.yml index fc5b79967..01a183f5b 100644 --- a/.travis.yml +++ b/.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"; diff --git a/README.md b/README.md index 147308fc6..872ea1baf 100644 --- a/README.md +++ b/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] diff --git a/docs/tutorials/src/05_storages.md b/docs/tutorials/src/05_storages.md index a16439bd7..a0aaf94f5 100644 --- a/docs/tutorials/src/05_storages.md +++ b/docs/tutorials/src/05_storages.md @@ -53,6 +53,7 @@ Certain storages provide access to component slices: |Storage Type | Slice type | Density | Indices | |:----------------------:|---------------------|---------|---------------| | [`DenseVecStorage`] | `&[T]` | Dense | Arbitrary | +| [`VecStorage`] | `&[MaybeUninit]` | Sparse | Entity `id()` | | [`DefaultVecStorage`] | `&[T]` | Sparse | Entity `id()` | This is intended as an advanced technique. Component slices provide @@ -102,6 +103,13 @@ 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 `as_slice()` and `as_mut_slice()` accessors which +return `&[MaybeUninit]`. 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. + ## `DefaultVecStorage` This storage works exactly like `VecStorage`, but instead of leaving gaps @@ -113,4 +121,5 @@ writes than `VecStorage`. 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 and with `Entity::id()`s. +correspond with each other, with `VecStorage` indices, and with +`Entity::id()`s. diff --git a/src/storage/storages.rs b/src/storage/storages.rs index 95985c44f..206af5dc0 100644 --- a/src/storage/storages.rs +++ b/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; @@ -219,35 +220,53 @@ unsafe impl DistinctStorage for NullStorage {} /// Vector storage. Uses a simple `Vec`. Supposed to have maximum /// performance for the components mostly present in entities. +/// +/// `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(Vec); +pub struct VecStorage(Vec>); + +impl SliceAccess for VecStorage { + type Element = MaybeUninit; + + #[inline] + fn as_slice(&self) -> &[Self::Element] { + self.0.as_slice() + } + + #[inline] + fn as_mut_slice(&mut self) -> &mut [Self::Element] { + self.0.as_mut_slice() + } +} impl UnprotectedStorage for VecStorage { unsafe fn clean(&mut self, has: B) - where - B: BitSetLike, + where + B: BitSetLike, { 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(); @@ -256,12 +275,11 @@ impl UnprotectedStorage for VecStorage { } // 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)) } } @@ -274,8 +292,8 @@ unsafe impl DistinctStorage for VecStorage {} /// Requires the component to implement `Default`. /// /// `as_slice()` and `as_mut_slice()` indices correspond to entity IDs. -/// These can be compared to other `DefaultVecStorage`s, and to -/// `Entity::id()`s for live entities. +/// These can be compared to other `DefaultVecStorage`s, to other +/// `VecStorage`s, and to `Entity::id()`s for live entities. #[derive(Derivative)] #[derivative(Default(bound = ""))] pub struct DefaultVecStorage(Vec); diff --git a/src/storage/tests.rs b/src/storage/tests.rs index b6ee6493f..b59a1fcd1 100644 --- a/src/storage/tests.rs +++ b/src/storage/tests.rs @@ -3,6 +3,7 @@ use std::any::Any; use super::*; use crate::world::{Component, Entity, Generation, Index, WorldExt}; use shred::World; +use std::mem::MaybeUninit; fn create(world: &mut World) -> WriteStorage where @@ -448,6 +449,27 @@ mod test { } } + fn test_maybeuninit_slice + Debug + Eq>() + where + T::Storage: Default + SliceAccess>, + { + let mut w = World::new(); + let mut s: Storage = create(&mut w); + + for i in 0..1_000 { + if let Err(err) = s.insert(Entity::new(i, Generation::new(1)), (i + 2718).into()) { + panic!("Failed to insert component into entity! {:?}", err); + } + } + + let slice = s.as_slice(); + assert_eq!(slice.len(), 1_000); + for (i, v) in slice.iter().enumerate() { + let v = unsafe { &*v.as_ptr() }; + assert_eq!(v, &(i as u32 + 2718).into()); + } + } + #[test] fn vec_test_add() { test_add::(); @@ -480,6 +502,10 @@ mod test { fn vec_test_anti() { test_anti::(); } + #[test] + fn vec_test_maybeuninit_slice() { + test_maybeuninit_slice::(); + } #[test] fn vec_arc() { From d79e603740e30cb7b8cd84a42e2727aedc45b69f Mon Sep 17 00:00:00 2001 From: Will Glynn Date: Fri, 27 Sep 2019 14:22:53 -0500 Subject: [PATCH 3/3] Switch DenseVecStorage::data_id to Vec> This vector contains uninitialized data, so it should use MaybeUninit. --- src/storage/storages.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/storage/storages.rs b/src/storage/storages.rs index 206af5dc0..f65638546 100644 --- a/src/storage/storages.rs +++ b/src/storage/storages.rs @@ -107,7 +107,7 @@ unsafe impl DistinctStorage for HashMapStorage {} pub struct DenseVecStorage { data: Vec, entity_id: Vec, - data_id: Vec, + data_id: Vec>, } impl SliceAccess for DenseVecStorage { @@ -141,12 +141,12 @@ impl UnprotectedStorage for DenseVecStorage { } unsafe fn get(&self, id: Index) -> &T { - let did = *self.data_id.get_unchecked(id as usize); + let did = self.data_id.get_unchecked(id as usize).assume_init(); self.data.get_unchecked(did as usize) } unsafe fn get_mut(&mut self, id: Index) -> &mut T { - let did = *self.data_id.get_unchecked(id as usize); + let did = self.data_id.get_unchecked(id as usize).assume_init(); self.data.get_unchecked_mut(did as usize) } @@ -157,15 +157,15 @@ impl UnprotectedStorage for DenseVecStorage { self.data_id.reserve(delta); self.data_id.set_len(id + 1); } - *self.data_id.get_unchecked_mut(id) = self.data.len() as Index; + self.data_id.get_unchecked_mut(id).as_mut_ptr().write(self.data.len() as Index); self.entity_id.push(id as Index); self.data.push(v); } unsafe fn remove(&mut self, id: Index) -> T { - let did = *self.data_id.get_unchecked(id as usize); + let did = self.data_id.get_unchecked(id as usize).assume_init(); let last = *self.entity_id.last().unwrap(); - *self.data_id.get_unchecked_mut(last as usize) = did; + self.data_id.get_unchecked_mut(last as usize).as_mut_ptr().write(did); self.entity_id.swap_remove(did as usize); self.data.swap_remove(did as usize) }