Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Merged by Bors] - Remove redundant table and sparse set component IDs from Archetype #4927

Closed
wants to merge 4 commits into from
Closed

[Merged by Bors] - Remove redundant table and sparse set component IDs from Archetype #4927

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5c598c2
Remove table and sparse set component IDs from Archetype
james7132 Jun 4, 2022
c410d58
Merge branch 'main' into archetype-component-id-removal
james7132 Oct 28, 2022
c2cf143
Merge branch 'main' into archetype-component-id-removal
james7132 Oct 28, 2022
71262b2
Use the correct size_hint
james7132 Nov 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 34 additions & 43 deletions crates/bevy_ecs/src/archetype.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,40 +151,32 @@ pub struct Archetype {
table_id: TableId,
edges: Edges,
entities: Vec<ArchetypeEntity>,
table_components: Box<[ComponentId]>,
sparse_set_components: Box<[ComponentId]>,
components: SparseSet<ComponentId, ArchetypeComponentInfo>,
}

impl Archetype {
pub fn new(
id: ArchetypeId,
table_id: TableId,
table_components: Box<[ComponentId]>,
sparse_set_components: Box<[ComponentId]>,
table_archetype_components: Vec<ArchetypeComponentId>,
sparse_set_archetype_components: Vec<ArchetypeComponentId>,
table_components: impl Iterator<Item = (ComponentId, ArchetypeComponentId)>,
sparse_set_components: impl Iterator<Item = (ComponentId, ArchetypeComponentId)>,
) -> Self {
let mut components =
SparseSet::with_capacity(table_components.len() + sparse_set_components.len());
for (component_id, archetype_component_id) in
table_components.iter().zip(table_archetype_components)
{
let (min_table, _) = table_components.size_hint();
let (min_sparse, _) = table_components.size_hint();
james7132 marked this conversation as resolved.
Show resolved Hide resolved
let mut components = SparseSet::with_capacity(min_table + min_sparse);
for (component_id, archetype_component_id) in table_components {
components.insert(
*component_id,
component_id,
ArchetypeComponentInfo {
storage_type: StorageType::Table,
archetype_component_id,
},
);
}

for (component_id, archetype_component_id) in sparse_set_components
.iter()
.zip(sparse_set_archetype_components)
{
for (component_id, archetype_component_id) in sparse_set_components {
components.insert(
*component_id,
component_id,
ArchetypeComponentInfo {
storage_type: StorageType::SparseSet,
archetype_component_id,
Expand All @@ -194,10 +186,8 @@ impl Archetype {
Self {
id,
table_id,
entities: Vec::new(),
components,
table_components,
sparse_set_components,
entities: Default::default(),
edges: Default::default(),
}
}
Expand All @@ -218,13 +208,19 @@ impl Archetype {
}

#[inline]
pub fn table_components(&self) -> &[ComponentId] {
&self.table_components
pub fn table_components(&self) -> impl Iterator<Item = ComponentId> + '_ {
self.components
.iter()
.filter(|(_, component)| component.storage_type == StorageType::Table)
.map(|(id, _)| *id)
}

#[inline]
pub fn sparse_set_components(&self) -> &[ComponentId] {
&self.sparse_set_components
pub fn sparse_set_components(&self) -> impl Iterator<Item = ComponentId> + '_ {
self.components
.iter()
.filter(|(_, component)| component.storage_type == StorageType::SparseSet)
.map(|(id, _)| *id)
}

#[inline]
Expand Down Expand Up @@ -453,38 +449,33 @@ impl Archetypes {
table_components: Vec<ComponentId>,
sparse_set_components: Vec<ComponentId>,
) -> ArchetypeId {
let table_components = table_components.into_boxed_slice();
let sparse_set_components = sparse_set_components.into_boxed_slice();
let archetype_identity = ArchetypeIdentity {
sparse_set_components: sparse_set_components.clone(),
table_components: table_components.clone(),
sparse_set_components: sparse_set_components.clone().into_boxed_slice(),
table_components: table_components.clone().into_boxed_slice(),
};

let archetypes = &mut self.archetypes;
let archetype_component_count = &mut self.archetype_component_count;
let mut next_archetype_component_id = move || {
let id = ArchetypeComponentId(*archetype_component_count);
*archetype_component_count += 1;
id
};
*self
.archetype_ids
.entry(archetype_identity)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not something that needs to be done here, but I think we can completely eliminate the redundant clones / allocations here related to ArchetypeIdentity:

  1. Use Hashbrown's raw_entry API to remove the need for an "owning" ArchetypeIdentity when the archetype already exists
  2. When the archetype does not exist, create the Archetype first using a non-owning iterator over sparse_set_components and table_components, then use the same allocated lists to construct the final owning ArchetypeIdentity.

.or_insert_with(move || {
let id = ArchetypeId(archetypes.len());
let table_archetype_components = (0..table_components.len())
.map(|_| next_archetype_component_id())
.collect();
let sparse_set_archetype_components = (0..sparse_set_components.len())
.map(|_| next_archetype_component_id())
.collect();
let table_start = *archetype_component_count;
*archetype_component_count += table_components.len();
let table_archetype_components =
(table_start..*archetype_component_count).map(ArchetypeComponentId);
let sparse_start = *archetype_component_count;
*archetype_component_count += sparse_set_components.len();
let sparse_set_archetype_components =
(sparse_start..*archetype_component_count).map(ArchetypeComponentId);
archetypes.push(Archetype::new(
id,
table_id,
table_components,
sparse_set_components,
table_archetype_components,
sparse_set_archetype_components,
table_components.into_iter().zip(table_archetype_components),
sparse_set_components
.into_iter()
.zip(sparse_set_archetype_components),
));
id
})
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_ecs/src/bundle.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -447,7 +447,7 @@ impl BundleInfo {
table_components = if new_table_components.is_empty() {
// if there are no new table components, we can keep using this table
table_id = current_archetype.table_id();
current_archetype.table_components().to_vec()
current_archetype.table_components().collect()
} else {
new_table_components.extend(current_archetype.table_components());
// sort to ignore order while hashing
Expand All @@ -463,7 +463,7 @@ impl BundleInfo {
};

sparse_set_components = if new_sparse_set_components.is_empty() {
current_archetype.sparse_set_components().to_vec()
current_archetype.sparse_set_components().collect()
} else {
new_sparse_set_components.extend(current_archetype.sparse_set_components());
// sort to ignore order while hashing
Expand Down
6 changes: 3 additions & 3 deletions crates/bevy_ecs/src/world/entity_ref.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -503,7 +503,7 @@ impl<'w> EntityMut<'w> {
table_row = remove_result.table_row;

for component_id in archetype.sparse_set_components() {
let sparse_set = world.storages.sparse_sets.get_mut(*component_id).unwrap();
let sparse_set = world.storages.sparse_sets.get_mut(component_id).unwrap();
sparse_set.remove(self.entity);
}
// SAFETY: table rows stored in archetypes always exist
Expand Down Expand Up @@ -835,8 +835,8 @@ unsafe fn remove_bundle_from_archetype(
// components are already sorted
removed_table_components.sort();
removed_sparse_set_components.sort();
next_table_components = current_archetype.table_components().to_vec();
next_sparse_set_components = current_archetype.sparse_set_components().to_vec();
next_table_components = current_archetype.table_components().collect();
next_sparse_set_components = current_archetype.sparse_set_components().collect();
sorted_remove(&mut next_table_components, &removed_table_components);
sorted_remove(
&mut next_sparse_set_components,
Expand Down