From 6946b37e805616d39e2664f51890c3144ff24724 Mon Sep 17 00:00:00 2001 From: Ian Chamberlain Date: Sun, 10 Apr 2022 10:43:35 -0400 Subject: [PATCH 1/2] Use 32-bit cursor on platforms without AtomicI64 --- crates/bevy_ecs/src/entity/mod.rs | 42 ++++++++++++++++++------------- 1 file changed, 24 insertions(+), 18 deletions(-) diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs index be142a594cd1d..cb3dae90d5be3 100644 --- a/crates/bevy_ecs/src/entity/mod.rs +++ b/crates/bevy_ecs/src/entity/mod.rs @@ -29,11 +29,17 @@ pub use self::serde::*; pub use map_entities::*; use crate::{archetype::ArchetypeId, storage::SparseSetIndex}; -use std::{ - convert::TryFrom, - fmt, mem, - sync::atomic::{AtomicI64, Ordering}, -}; +use std::{convert::TryFrom, fmt, mem, sync::atomic::Ordering}; + +#[cfg(target_has_atomic = "64")] +use std::sync::atomic::AtomicI64 as AtomicInt; +#[cfg(target_has_atomic = "64")] +type Int = i64; + +#[cfg(not(target_has_atomic = "64"))] +use std::sync::atomic::AtomicI32 as AtomicInt; +#[cfg(not(target_has_atomic = "64"))] +type Int = i32; /// Lightweight unique ID of an entity. /// @@ -237,7 +243,7 @@ pub struct Entities { /// /// Once `flush()` is done, `free_cursor` will equal `pending.len()`. pending: Vec, - free_cursor: AtomicI64, + free_cursor: AtomicInt, /// Stores the number of free entities for [`len`](Entities::len) len: u32, } @@ -250,8 +256,8 @@ impl Entities { // Use one atomic subtract to grab a range of new IDs. The range might be // entirely nonnegative, meaning all IDs come from the freelist, or entirely // negative, meaning they are all new IDs to allocate, or a mix of both. - let range_end = self.free_cursor.fetch_sub(count as i64, Ordering::Relaxed); - let range_start = range_end - count as i64; + let range_end = self.free_cursor.fetch_sub(count as Int, Ordering::Relaxed); + let range_start = range_end - count as Int; let freelist_range = range_start.max(0) as usize..range_end.max(0) as usize; @@ -268,7 +274,7 @@ impl Entities { // In this example, we truncate the end to 0, leaving us with `-3..0`. // Then we negate these values to indicate how far beyond the end of `meta.end()` // to go, yielding `meta.len()+0 .. meta.len()+3`. - let base = self.meta.len() as i64; + let base = self.meta.len() as Int; let new_id_end = u32::try_from(base - range_start).expect("too many entities"); @@ -305,7 +311,7 @@ impl Entities { // and farther beyond `meta.len()`. Entity { generation: 0, - id: u32::try_from(self.meta.len() as i64 - n).expect("too many entities"), + id: u32::try_from(self.meta.len() as Int - n).expect("too many entities"), } } } @@ -323,7 +329,7 @@ impl Entities { self.verify_flushed(); self.len += 1; if let Some(id) = self.pending.pop() { - let new_free_cursor = self.pending.len() as i64; + let new_free_cursor = self.pending.len() as Int; *self.free_cursor.get_mut() = new_free_cursor; Entity { generation: self.meta[id as usize].generation, @@ -345,14 +351,14 @@ impl Entities { let loc = if entity.id as usize >= self.meta.len() { self.pending.extend((self.meta.len() as u32)..entity.id); - let new_free_cursor = self.pending.len() as i64; + let new_free_cursor = self.pending.len() as Int; *self.free_cursor.get_mut() = new_free_cursor; self.meta.resize(entity.id as usize + 1, EntityMeta::EMPTY); self.len += 1; None } else if let Some(index) = self.pending.iter().position(|item| *item == entity.id) { self.pending.swap_remove(index); - let new_free_cursor = self.pending.len() as i64; + let new_free_cursor = self.pending.len() as Int; *self.free_cursor.get_mut() = new_free_cursor; self.len += 1; None @@ -376,14 +382,14 @@ impl Entities { let result = if entity.id as usize >= self.meta.len() { self.pending.extend((self.meta.len() as u32)..entity.id); - let new_free_cursor = self.pending.len() as i64; + let new_free_cursor = self.pending.len() as Int; *self.free_cursor.get_mut() = new_free_cursor; self.meta.resize(entity.id as usize + 1, EntityMeta::EMPTY); self.len += 1; AllocAtWithoutReplacement::DidNotExist } else if let Some(index) = self.pending.iter().position(|item| *item == entity.id) { self.pending.swap_remove(index); - let new_free_cursor = self.pending.len() as i64; + let new_free_cursor = self.pending.len() as Int; *self.free_cursor.get_mut() = new_free_cursor; self.len += 1; AllocAtWithoutReplacement::DidNotExist @@ -418,7 +424,7 @@ impl Entities { self.pending.push(entity.id); - let new_free_cursor = self.pending.len() as i64; + let new_free_cursor = self.pending.len() as Int; *self.free_cursor.get_mut() = new_free_cursor; self.len -= 1; Some(loc) @@ -429,7 +435,7 @@ impl Entities { self.verify_flushed(); let freelist_size = *self.free_cursor.get_mut(); - let shortfall = additional as i64 - freelist_size; + let shortfall = additional as Int - freelist_size; if shortfall > 0 { self.meta.reserve(shortfall as usize); } @@ -486,7 +492,7 @@ impl Entities { } fn needs_flush(&mut self) -> bool { - *self.free_cursor.get_mut() != self.pending.len() as i64 + *self.free_cursor.get_mut() != self.pending.len() as Int } /// Allocates space for entities previously reserved with `reserve_entity` or From 58646055daf39898847864731ab32398741e1c9d Mon Sep 17 00:00:00 2001 From: Ian Chamberlain Date: Mon, 11 Apr 2022 22:29:20 -0400 Subject: [PATCH 2/2] More robust AtomicIdCursor behavior - Rename to [Atomic]IdCursor - Add comments documentating behavior on non-64-bit-atomic platforms - try_from().unwrap() in cases that can only fail with 32-bit AtomicIsize --- crates/bevy_ecs/src/entity/mod.rs | 43 +++++++++++++++++++------------ 1 file changed, 26 insertions(+), 17 deletions(-) diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs index cb3dae90d5be3..335f0f5201aa7 100644 --- a/crates/bevy_ecs/src/entity/mod.rs +++ b/crates/bevy_ecs/src/entity/mod.rs @@ -32,14 +32,17 @@ use crate::{archetype::ArchetypeId, storage::SparseSetIndex}; use std::{convert::TryFrom, fmt, mem, sync::atomic::Ordering}; #[cfg(target_has_atomic = "64")] -use std::sync::atomic::AtomicI64 as AtomicInt; +use std::sync::atomic::AtomicI64 as AtomicIdCursor; #[cfg(target_has_atomic = "64")] -type Int = i64; +type IdCursor = i64; +/// Most modern platforms support 64-bit atomics, but some less-common platforms +/// do not. This fallback allows compilation using a 32-bit cursor instead, with +/// the caveat that some conversions may fail (and panic) at runtime. #[cfg(not(target_has_atomic = "64"))] -use std::sync::atomic::AtomicI32 as AtomicInt; +use std::sync::atomic::AtomicIsize as AtomicIdCursor; #[cfg(not(target_has_atomic = "64"))] -type Int = i32; +type IdCursor = isize; /// Lightweight unique ID of an entity. /// @@ -243,7 +246,7 @@ pub struct Entities { /// /// Once `flush()` is done, `free_cursor` will equal `pending.len()`. pending: Vec, - free_cursor: AtomicInt, + free_cursor: AtomicIdCursor, /// Stores the number of free entities for [`len`](Entities::len) len: u32, } @@ -256,8 +259,12 @@ impl Entities { // Use one atomic subtract to grab a range of new IDs. The range might be // entirely nonnegative, meaning all IDs come from the freelist, or entirely // negative, meaning they are all new IDs to allocate, or a mix of both. - let range_end = self.free_cursor.fetch_sub(count as Int, Ordering::Relaxed); - let range_start = range_end - count as Int; + let range_end = self + .free_cursor + // Unwrap: these conversions can only fail on platforms that don't support 64-bit atomics + // and use AtomicIsize instead (see note on `IdCursor`). + .fetch_sub(IdCursor::try_from(count).unwrap(), Ordering::Relaxed); + let range_start = range_end - IdCursor::try_from(count).unwrap(); let freelist_range = range_start.max(0) as usize..range_end.max(0) as usize; @@ -274,7 +281,7 @@ impl Entities { // In this example, we truncate the end to 0, leaving us with `-3..0`. // Then we negate these values to indicate how far beyond the end of `meta.end()` // to go, yielding `meta.len()+0 .. meta.len()+3`. - let base = self.meta.len() as Int; + let base = self.meta.len() as IdCursor; let new_id_end = u32::try_from(base - range_start).expect("too many entities"); @@ -311,7 +318,7 @@ impl Entities { // and farther beyond `meta.len()`. Entity { generation: 0, - id: u32::try_from(self.meta.len() as Int - n).expect("too many entities"), + id: u32::try_from(self.meta.len() as IdCursor - n).expect("too many entities"), } } } @@ -329,7 +336,7 @@ impl Entities { self.verify_flushed(); self.len += 1; if let Some(id) = self.pending.pop() { - let new_free_cursor = self.pending.len() as Int; + let new_free_cursor = self.pending.len() as IdCursor; *self.free_cursor.get_mut() = new_free_cursor; Entity { generation: self.meta[id as usize].generation, @@ -351,14 +358,14 @@ impl Entities { let loc = if entity.id as usize >= self.meta.len() { self.pending.extend((self.meta.len() as u32)..entity.id); - let new_free_cursor = self.pending.len() as Int; + let new_free_cursor = self.pending.len() as IdCursor; *self.free_cursor.get_mut() = new_free_cursor; self.meta.resize(entity.id as usize + 1, EntityMeta::EMPTY); self.len += 1; None } else if let Some(index) = self.pending.iter().position(|item| *item == entity.id) { self.pending.swap_remove(index); - let new_free_cursor = self.pending.len() as Int; + let new_free_cursor = self.pending.len() as IdCursor; *self.free_cursor.get_mut() = new_free_cursor; self.len += 1; None @@ -382,14 +389,14 @@ impl Entities { let result = if entity.id as usize >= self.meta.len() { self.pending.extend((self.meta.len() as u32)..entity.id); - let new_free_cursor = self.pending.len() as Int; + let new_free_cursor = self.pending.len() as IdCursor; *self.free_cursor.get_mut() = new_free_cursor; self.meta.resize(entity.id as usize + 1, EntityMeta::EMPTY); self.len += 1; AllocAtWithoutReplacement::DidNotExist } else if let Some(index) = self.pending.iter().position(|item| *item == entity.id) { self.pending.swap_remove(index); - let new_free_cursor = self.pending.len() as Int; + let new_free_cursor = self.pending.len() as IdCursor; *self.free_cursor.get_mut() = new_free_cursor; self.len += 1; AllocAtWithoutReplacement::DidNotExist @@ -424,7 +431,7 @@ impl Entities { self.pending.push(entity.id); - let new_free_cursor = self.pending.len() as Int; + let new_free_cursor = self.pending.len() as IdCursor; *self.free_cursor.get_mut() = new_free_cursor; self.len -= 1; Some(loc) @@ -435,7 +442,9 @@ impl Entities { self.verify_flushed(); let freelist_size = *self.free_cursor.get_mut(); - let shortfall = additional as Int - freelist_size; + // Unwrap: these conversions can only fail on platforms that don't support 64-bit atomics + // and use AtomicIsize instead (see note on `IdCursor`). + let shortfall = IdCursor::try_from(additional).unwrap() - freelist_size; if shortfall > 0 { self.meta.reserve(shortfall as usize); } @@ -492,7 +501,7 @@ impl Entities { } fn needs_flush(&mut self) -> bool { - *self.free_cursor.get_mut() != self.pending.len() as Int + *self.free_cursor.get_mut() != self.pending.len() as IdCursor } /// Allocates space for entities previously reserved with `reserve_entity` or