Skip to content

Commit 6a5ef73

Browse files
chescockkfc35
andauthored
Change ResourceEntities from SparseSet to SparseArray to speed up resource lookups (#23616)
# Objective Reduce memory usage for resources, and maybe improve performance of resource lookups. Related to #23039, but not a solution. ## Solution Change `ResourceEntities` from a `SparseSet` to a `SparseArray`. `SparseArray` is `pub (crate)`, so simply exposing it through `Deref` would cause privacy errors. Instead, remove the `Deref` impl and add wrapper methods for `iter`, `get`, and `remove`. Change the return types from `&Entity` to `Entity` now that they aren't generic. As background: A `SparseArray` is a simple `Vec<Option<V>>`, while a `SparseSet` is a `SparseArray` that maps keys to dense indexes, combined with dense arrays of keys and values. That requires a second array operation to find the actual value, but can be much better for memory usage when the values are large, since missing items only take up space for a single index instead of an entire value. But the values in `ResourceEntities` are `Entity`, which are already small! A `SparseArray` will always be smaller on 64-bit systems, since an `Entity` is the same size as a `usize`, and we don't need to store the additional `dense` and `indices` arrays. So switching to `SparseArray` will save a lookup *and* save memory. One drawback is that we can no longer use the dense lists to iterate all resources, so methods like `iter_resources` now need to scan all component ids. I don't expect this to be a problem in practice, though. `iter_resources` is rarely used, and O(components) isn't all that much worse than O(resources). If it turns out to be an issue, it's also possible to recover this data by querying the `IsResource` component. ## Testing Inconclusive. I attempted to run benchmarks, both `bevymark` as in the linked issue and `cargo bench -p benches --bench ecs`, but the results were too noisy on my machine to reach any conclusions. And now that I look more closely, we don't have many benches that even use resources! --------- Co-authored-by: Kevin Chen <chen.kevin.f@gmail.com>
1 parent a5cbc9e commit 6a5ef73

10 files changed

Lines changed: 72 additions & 51 deletions

File tree

crates/bevy_ecs/src/resource.rs

Lines changed: 28 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,12 @@
11
//! Resources are unique, singleton-like data types that can be accessed from systems and stored in the [`World`](crate::world::World).
22
3-
use core::ops::{Deref, DerefMut};
43
use log::warn;
54

65
use crate::{
76
component::{Component, ComponentId, Mutable},
87
entity::Entity,
98
lifecycle::HookContext,
10-
storage::SparseSet,
9+
storage::SparseArray,
1110
world::DeferredWorld,
1211
};
1312
#[cfg(feature = "bevy_reflect")]
@@ -89,12 +88,33 @@ pub trait Resource: Component<Mutability = Mutable> {}
8988

9089
/// A cache that links each `ComponentId` from a resource to the corresponding entity.
9190
#[derive(Default)]
92-
pub struct ResourceEntities(SyncUnsafeCell<SparseSet<ComponentId, Entity>>);
91+
pub struct ResourceEntities(SyncUnsafeCell<SparseArray<ComponentId, Entity>>);
92+
93+
impl ResourceEntities {
94+
/// Returns an iterator over all registered resource components and their corresponding entity.
95+
///
96+
/// This must scan the entire array of components to find non-empty values,
97+
/// which may be slow even if there are few resources.
98+
#[inline]
99+
pub fn iter(&self) -> impl Iterator<Item = (ComponentId, Entity)> {
100+
self.deref().iter().map(|(id, entity)| (id, *entity))
101+
}
102+
103+
/// Returns the entity for the given resource component, or `None` if there is no entity.
104+
#[inline]
105+
pub fn get(&self, id: ComponentId) -> Option<Entity> {
106+
self.deref().get(id).copied()
107+
}
93108

94-
impl Deref for ResourceEntities {
95-
type Target = SparseSet<ComponentId, Entity>;
109+
/// Removes the entry for the given resource component.
110+
/// Returns the entity that was removed, or `None` if there was no entity.
111+
#[inline]
112+
pub(crate) fn remove(&mut self, id: ComponentId) -> Option<Entity> {
113+
self.0.get_mut().remove(id)
114+
}
96115

97-
fn deref(&self) -> &Self::Target {
116+
#[inline]
117+
fn deref(&self) -> &SparseArray<ComponentId, Entity> {
98118
// SAFETY: There are no other mutable references to the map.
99119
// The underlying `SyncUnsafeCell` is never exposed outside this module,
100120
// so mutable references are only created by the resource hooks.
@@ -104,12 +124,6 @@ impl Deref for ResourceEntities {
104124
}
105125
}
106126

107-
impl DerefMut for ResourceEntities {
108-
fn deref_mut(&mut self) -> &mut Self::Target {
109-
self.0.get_mut()
110-
}
111-
}
112-
113127
/// A marker component for entities that have a Resource component.
114128
#[cfg_attr(feature = "bevy_reflect", derive(Reflect), reflect(Component, Debug))]
115129
#[derive(Component, Debug)]
@@ -134,7 +148,7 @@ impl IsResource {
134148
.unwrap()
135149
.resource_component_id();
136150

137-
if let Some(&original_entity) = world.resource_entities.get(resource_component_id) {
151+
if let Some(original_entity) = world.resource_entities.get(resource_component_id) {
138152
if !world.entities().contains(original_entity) {
139153
let name = world
140154
.components()
@@ -182,7 +196,7 @@ impl IsResource {
182196
.resource_component_id();
183197

184198
if let Some(resource_entity) = world.resource_entities.get(resource_component_id)
185-
&& *resource_entity == context.entity
199+
&& resource_entity == context.entity
186200
{
187201
// SAFETY: We have exclusive world access (as long as we don't make structural changes).
188202
let cache = unsafe { world.as_unsafe_world_cell().resource_entities() };

crates/bevy_ecs/src/storage/sparse_set.rs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,19 @@ impl<I: SparseSetIndex, V> SparseArray<I, V> {
135135
marker: PhantomData,
136136
}
137137
}
138+
139+
/// Returns an iterator over the non-empty values in the array.
140+
///
141+
/// This must scan the entire array to find non-empty values,
142+
/// which may be slow even if the array is sparsely populated.
143+
#[inline]
144+
pub(crate) fn iter(&self) -> impl Iterator<Item = (I, &V)> {
145+
self.values.iter().enumerate().filter_map(|(index, value)| {
146+
value
147+
.as_ref()
148+
.map(|value| (SparseSetIndex::get_sparse_set_index(index), value))
149+
})
150+
}
138151
}
139152

140153
/// A sparse data structure of [`Component`](crate::component::Component)s.

crates/bevy_ecs/src/world/mod.rs

Lines changed: 11 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1860,7 +1860,7 @@ impl World {
18601860
) -> (ComponentId, EntityWorldMut<'_>) {
18611861
let resource_id = self.register_resource::<R>();
18621862

1863-
if let Some(&entity) = self.resource_entities.get(resource_id) {
1863+
if let Some(entity) = self.resource_entities.get(resource_id) {
18641864
let entity_ref = self.get_entity(entity).expect("ResourceCache is in sync");
18651865
if !entity_ref.contains_id(resource_id) {
18661866
let resource = func(self);
@@ -1995,7 +1995,7 @@ impl World {
19951995
#[inline]
19961996
pub fn remove_resource<R: Resource>(&mut self) -> Option<R> {
19971997
let resource_id = self.component_id::<R>()?;
1998-
let entity = *self.resource_entities.get(resource_id)?;
1998+
let entity = self.resource_entities.get(resource_id)?;
19991999
let value = self
20002000
.get_entity_mut(entity)
20012001
.expect("ResourceCache is in sync")
@@ -2040,7 +2040,7 @@ impl World {
20402040
#[inline]
20412041
pub fn contains_resource_by_id(&self, component_id: ComponentId) -> bool {
20422042
if let Some(entity) = self.resource_entities.get(component_id)
2043-
&& let Ok(entity_ref) = self.get_entity(*entity)
2043+
&& let Ok(entity_ref) = self.get_entity(entity)
20442044
{
20452045
return entity_ref.contains_id(component_id);
20462046
}
@@ -2130,7 +2130,7 @@ impl World {
21302130
component_id: ComponentId,
21312131
) -> Option<ComponentTicks> {
21322132
let entity = self.resource_entities.get(component_id)?;
2133-
let entity_ref = self.get_entity(*entity).ok()?;
2133+
let entity_ref = self.get_entity(entity).ok()?;
21342134
entity_ref.get_change_ticks_by_id(component_id)
21352135
}
21362136

@@ -2781,7 +2781,7 @@ impl World {
27812781
let change_tick = self.change_tick();
27822782

27832783
let component_id = self.components.valid_component_id::<R>()?;
2784-
let entity = *self.resource_entities.get(component_id)?;
2784+
let entity = self.resource_entities.get(component_id)?;
27852785
let mut entity_mut = self.get_entity_mut(entity).ok()?;
27862786

27872787
let mut ticks = entity_mut.get_change_ticks::<R>()?;
@@ -2974,7 +2974,7 @@ impl World {
29742974
) {
29752975
// if the resource already exists, we replace it on the same entity
29762976
let mut entity_mut = if let Some(entity) = self.resource_entities.get(component_id) {
2977-
self.get_entity_mut(*entity)
2977+
self.get_entity_mut(entity)
29782978
.expect("ResourceCache is in sync")
29792979
} else {
29802980
self.spawn_empty()
@@ -3303,11 +3303,7 @@ impl World {
33033303
/// This can easily cause systems expecting certain resources to immediately start panicking.
33043304
/// Use with caution.
33053305
pub fn clear_resources(&mut self) {
3306-
let pairs: Vec<(ComponentId, Entity)> = self
3307-
.resource_entities()
3308-
.iter()
3309-
.map(|(id, entity)| (*id, *entity))
3310-
.collect();
3306+
let pairs: Vec<(ComponentId, Entity)> = self.resource_entities().iter().collect();
33113307
for (component_id, entity) in pairs {
33123308
self.entity_mut(entity).remove_by_id(component_id);
33133309
}
@@ -3509,11 +3505,11 @@ impl World {
35093505
#[inline]
35103506
pub fn iter_resources(&self) -> impl Iterator<Item = (&ComponentInfo, Ptr<'_>)> {
35113507
self.resource_entities
3512-
.indices()
35133508
.iter()
3514-
.filter_map(|component_id| {
3515-
let component_info = self.components().get_info(*component_id)?;
3516-
let resource = self.get_resource_by_id(*component_id)?;
3509+
.filter_map(|(component_id, entity)| {
3510+
let component_info = self.components().get_info(component_id)?;
3511+
let entity_cell = self.get_entity(entity).ok()?;
3512+
let resource = entity_cell.get_by_id(component_id).ok()?;
35173513
Some((component_info, resource))
35183514
})
35193515
}
@@ -3591,7 +3587,6 @@ impl World {
35913587

35923588
resource_entities
35933589
.iter()
3594-
.map(|(component_id, entity)| (*component_id, *entity))
35953590
.filter_map(move |(component_id, entity)| {
35963591
// SAFETY: If a resource has been initialized, a corresponding ComponentInfo must exist with its ID.
35973592
let component_info =
@@ -3867,7 +3862,6 @@ impl fmt::Debug for World {
38673862
.field("entity_count", &self.entities.count_spawned())
38683863
.field("archetype_count", &self.archetypes.len())
38693864
.field("component_count", &self.components.len())
3870-
.field("resource_count", &self.resource_entities.len())
38713865
.finish()
38723866
}
38733867
}

crates/bevy_ecs/src/world/reflect.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,7 @@ impl World {
200200
reflected_resource: Box<dyn PartialReflect>,
201201
) {
202202
if let Some(entity) = self.resource_entities().get(resource_id) {
203-
self.entity_mut(*entity).insert_reflect(reflected_resource);
203+
self.entity_mut(entity).insert_reflect(reflected_resource);
204204
} else {
205205
self.spawn_empty().insert_reflect(reflected_resource);
206206
}

crates/bevy_ecs/src/world/unsafe_world_cell.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -466,7 +466,7 @@ impl<'w> UnsafeWorldCell<'w> {
466466
pub unsafe fn get_resource_by_id(self, component_id: ComponentId) -> Option<Ptr<'w>> {
467467
// SAFETY: We have permission to access the resource of `component_id`.
468468
let entity = unsafe { self.resource_entities() }.get(component_id)?;
469-
let entity_cell = self.get_entity(*entity).ok()?;
469+
let entity_cell = self.get_entity(entity).ok()?;
470470
entity_cell.get_by_id(component_id)
471471
}
472472

@@ -575,7 +575,7 @@ impl<'w> UnsafeWorldCell<'w> {
575575
self.assert_allows_mutable_access();
576576
// SAFETY: We have permission to access the resource of `component_id`.
577577
let entity = unsafe { self.resource_entities() }.get(component_id)?;
578-
let entity_cell = self.get_entity(*entity).ok()?;
578+
let entity_cell = self.get_entity(entity).ok()?;
579579
entity_cell.get_mut_by_id(component_id).ok()
580580
}
581581

@@ -680,13 +680,13 @@ impl<'w> UnsafeWorldCell<'w> {
680680
// SAFETY: We have permission to access the resource of `component_id`.
681681
let entity = unsafe { self.resource_entities() }.get(component_id)?;
682682
let storage_type = self.components().get_info(component_id)?.storage_type();
683-
let location = self.get_entity(*entity).ok()?.location();
683+
let location = self.get_entity(entity).ok()?.location();
684684
// SAFETY:
685685
// - caller ensures there is no `&mut World`
686686
// - caller ensures there are no mutable borrows of this resource
687687
// - caller ensures that we have permission to access this resource
688688
// - storage_type and location are valid
689-
get_component_and_ticks(self, component_id, storage_type, *entity, location)
689+
get_component_and_ticks(self, component_id, storage_type, entity, location)
690690
}
691691

692692
// Shorthand helper function for getting the data and change ticks for a resource.

crates/bevy_remote/src/builtin_methods.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1893,7 +1893,7 @@ fn get_resource_entity_pair(
18931893
.resource_entities()
18941894
.get(component_id)
18951895
.ok_or(anyhow!("Resource entity does not exist."))?;
1896-
Ok((*entity, component_id))
1896+
Ok((entity, component_id))
18971897
}
18981898

18991899
#[cfg(test)]

crates/bevy_settings/src/lib.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -326,7 +326,7 @@ fn resources_to_toml(
326326
let Some(res_entity) = world.resource_entities().get(component_id) else {
327327
continue;
328328
};
329-
let res_entity_ref = world.entity(*res_entity);
329+
let res_entity_ref = world.entity(res_entity);
330330
let Some(reflect) = cmp.reflect(res_entity_ref) else {
331331
continue;
332332
};
@@ -441,7 +441,7 @@ fn apply_settings_to_world(
441441

442442
if let Some(res_entity) = res_entity {
443443
// Resource already exists, so apply toml properties to it.
444-
let res_entity_mut = world.entity_mut(*res_entity);
444+
let res_entity_mut = world.entity_mut(res_entity);
445445
let Some(mut reflect) = reflect_component.reflect_mut(res_entity_mut) else {
446446
continue;
447447
};

crates/bevy_world_serialization/src/dynamic_world.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ impl DynamicWorld {
184184

185185
// check if the resource already exists, if not spawn it, otherwise override the value
186186
let entity = if let Some(entity) = world.resource_entities().get(resource_id) {
187-
*entity
187+
entity
188188
} else {
189189
world.spawn_empty().id()
190190
};

crates/bevy_world_serialization/src/dynamic_world_builder.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -378,14 +378,14 @@ impl<'w> DynamicWorldBuilder<'w> {
378378
.get_valid_id(TypeId::of::<DefaultQueryFilters>());
379379

380380
for (component_id, entity) in self.original_world.resource_entities().iter() {
381-
if Some(*component_id) == original_world_dqf_id {
381+
if Some(component_id) == original_world_dqf_id {
382382
continue;
383383
}
384384
let mut extract_and_push = || {
385385
let type_id = self
386386
.original_world
387387
.components()
388-
.get_info(*component_id)?
388+
.get_info(component_id)?
389389
.type_id()?;
390390

391391
let is_denied = self.resource_filter.is_denied_by_id(type_id);
@@ -400,12 +400,12 @@ impl<'w> DynamicWorldBuilder<'w> {
400400
type_registration.data::<ReflectResource>()?;
401401
let component = type_registration
402402
.data::<ReflectComponent>()?
403-
.reflect(self.original_world.entity(*entity))?;
403+
.reflect(self.original_world.entity(entity))?;
404404

405405
let component =
406406
clone_reflect_value(component.as_partial_reflect(), type_registration);
407407

408-
self.extracted_resources.insert(*component_id, component);
408+
self.extracted_resources.insert(component_id, component);
409409
Some(())
410410
};
411411
extract_and_push();

crates/bevy_world_serialization/src/world_asset.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -76,21 +76,21 @@ impl WorldAsset {
7676

7777
// Resources archetype
7878
for (component_id, source_entity) in self.world.resource_entities().iter() {
79-
if Some(*component_id) == self_dqf_id {
79+
if Some(component_id) == self_dqf_id {
8080
continue;
8181
}
8282
if !world
83-
.get_entity(*source_entity)
83+
.get_entity(source_entity)
8484
.ok()
85-
.is_some_and(|entity_ref| entity_ref.contains_id(*component_id))
85+
.is_some_and(|entity_ref| entity_ref.contains_id(component_id))
8686
{
8787
continue;
8888
}
8989

9090
let component_info = self
9191
.world
9292
.components()
93-
.get_info(*component_id)
93+
.get_info(component_id)
9494
.expect("component_ids in archetypes should have ComponentInfo");
9595

9696
let type_id = component_info
@@ -114,16 +114,16 @@ impl WorldAsset {
114114

115115
// check if the resource already exists in the other world, if not spawn it
116116
let destination_entity =
117-
if let Some(entity) = world.resource_entities().get(*component_id) {
118-
*entity
117+
if let Some(entity) = world.resource_entities().get(component_id) {
118+
entity
119119
} else {
120120
world.spawn_empty().id()
121121
};
122122

123123
reflect_component.copy(
124124
&self.world,
125125
world,
126-
*source_entity,
126+
source_entity,
127127
destination_entity,
128128
&type_registry,
129129
);

0 commit comments

Comments
 (0)