Skip to content

Commit

Permalink
Use TypeIdMap whenever possible (bevyengine#11684)
Browse files Browse the repository at this point in the history
Use `TypeIdMap<T>` instead of `HashMap<TypeId, T>`

- ~~`TypeIdMap` was in `bevy_ecs`. I've kept it there because of
bevyengine#11478~~
- ~~I haven't swapped `bevy_reflect` over because it doesn't depend on
`bevy_ecs`, but I'd also be happy with moving `TypeIdMap` to
`bevy_utils` and then adding a dependency to that~~
- ~~this is a slight change in the public API of
`DrawFunctionsInternal`, does this need to go in the changelog?~~

## Changelog
- moved `TypeIdMap` to `bevy_utils`
- changed `DrawFunctionsInternal::indices` to `TypeIdMap`

## Migration Guide

- `TypeIdMap` now lives in `bevy_utils`
- `DrawFunctionsInternal::indices` now uses a `TypeIdMap`.

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
  • Loading branch information
2 people authored and tjamaan committed Feb 6, 2024
1 parent 485f7b6 commit f2a8400
Show file tree
Hide file tree
Showing 13 changed files with 79 additions and 81 deletions.
4 changes: 2 additions & 2 deletions crates/bevy_app/src/plugin_group.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::{App, AppError, Plugin};
use bevy_utils::{tracing::debug, tracing::warn, HashMap};
use bevy_utils::{tracing::debug, tracing::warn, TypeIdMap};
use std::any::TypeId;

/// Combines multiple [`Plugin`]s into a single unit.
Expand Down Expand Up @@ -33,7 +33,7 @@ impl PluginGroup for PluginGroupBuilder {
/// can be disabled, enabled or reordered.
pub struct PluginGroupBuilder {
group_name: String,
plugins: HashMap<TypeId, PluginEntry>,
plugins: TypeIdMap<PluginEntry>,
order: Vec<TypeId>,
}

Expand Down
18 changes: 9 additions & 9 deletions crates/bevy_asset/src/server/info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::{
};
use bevy_ecs::world::World;
use bevy_log::warn;
use bevy_utils::{Entry, HashMap, HashSet};
use bevy_utils::{Entry, HashMap, HashSet, TypeIdMap};
use crossbeam_channel::Sender;
use std::{
any::TypeId,
Expand Down Expand Up @@ -61,7 +61,7 @@ impl AssetInfo {

#[derive(Default)]
pub(crate) struct AssetInfos {
path_to_id: HashMap<AssetPath<'static>, HashMap<TypeId, UntypedAssetId>>,
path_to_id: HashMap<AssetPath<'static>, TypeIdMap<UntypedAssetId>>,
infos: HashMap<UntypedAssetId, AssetInfo>,
/// If set to `true`, this informs [`AssetInfos`] to track data relevant to watching for changes (such as `load_dependants`)
/// This should only be set at startup.
Expand All @@ -72,10 +72,10 @@ pub(crate) struct AssetInfos {
/// Tracks living labeled assets for a given source asset.
/// This should only be set when watching for changes to avoid unnecessary work.
pub(crate) living_labeled_assets: HashMap<AssetPath<'static>, HashSet<String>>,
pub(crate) handle_providers: HashMap<TypeId, AssetHandleProvider>,
pub(crate) dependency_loaded_event_sender: HashMap<TypeId, fn(&mut World, UntypedAssetId)>,
pub(crate) handle_providers: TypeIdMap<AssetHandleProvider>,
pub(crate) dependency_loaded_event_sender: TypeIdMap<fn(&mut World, UntypedAssetId)>,
pub(crate) dependency_failed_event_sender:
HashMap<TypeId, fn(&mut World, UntypedAssetId, AssetPath<'static>, AssetLoadError)>,
TypeIdMap<fn(&mut World, UntypedAssetId, AssetPath<'static>, AssetLoadError)>,
}

impl std::fmt::Debug for AssetInfos {
Expand Down Expand Up @@ -112,7 +112,7 @@ impl AssetInfos {
#[allow(clippy::too_many_arguments)]
fn create_handle_internal(
infos: &mut HashMap<UntypedAssetId, AssetInfo>,
handle_providers: &HashMap<TypeId, AssetHandleProvider>,
handle_providers: &TypeIdMap<AssetHandleProvider>,
living_labeled_assets: &mut HashMap<AssetPath<'static>, HashSet<String>>,
watching_for_changes: bool,
type_id: TypeId,
Expand Down Expand Up @@ -205,7 +205,7 @@ impl AssetInfos {
.ok_or(GetOrCreateHandleInternalError::HandleMissingButTypeIdNotSpecified)?;

match handles.entry(type_id) {
Entry::Occupied(entry) => {
bevy_utils::hashbrown::hash_map::Entry::Occupied(entry) => {
let id = *entry.get();
// if there is a path_to_id entry, info always exists
let info = self.infos.get_mut(&id).unwrap();
Expand Down Expand Up @@ -246,7 +246,7 @@ impl AssetInfos {
}
}
// The entry does not exist, so this is a "fresh" asset load. We must create a new handle
Entry::Vacant(entry) => {
bevy_utils::hashbrown::hash_map::Entry::Vacant(entry) => {
let should_load = match loading_mode {
HandleLoadingMode::NotLoading => false,
HandleLoadingMode::Request | HandleLoadingMode::Force => true,
Expand Down Expand Up @@ -640,7 +640,7 @@ impl AssetInfos {

fn process_handle_drop_internal(
infos: &mut HashMap<UntypedAssetId, AssetInfo>,
path_to_id: &mut HashMap<AssetPath<'static>, HashMap<TypeId, UntypedAssetId>>,
path_to_id: &mut HashMap<AssetPath<'static>, TypeIdMap<UntypedAssetId>>,
loader_dependants: &mut HashMap<AssetPath<'static>, HashSet<AssetPath<'static>>>,
living_labeled_assets: &mut HashMap<AssetPath<'static>, HashSet<String>>,
watching_for_changes: bool,
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_asset/src/server/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use crate::{
use bevy_ecs::prelude::*;
use bevy_log::{error, info, warn};
use bevy_tasks::IoTaskPool;
use bevy_utils::{CowArc, HashMap, HashSet};
use bevy_utils::{CowArc, HashMap, HashSet, TypeIdMap};
use crossbeam_channel::{Receiver, Sender};
use futures_lite::StreamExt;
use info::*;
Expand Down Expand Up @@ -1238,7 +1238,7 @@ pub fn handle_internal_asset_events(world: &mut World) {

#[derive(Default)]
pub(crate) struct AssetLoaders {
type_id_to_loader: HashMap<TypeId, MaybeAssetLoader>,
type_id_to_loader: TypeIdMap<MaybeAssetLoader>,
extension_to_type_id: HashMap<String, TypeId>,
type_name_to_type_id: HashMap<&'static str, TypeId>,
preregistered_loaders: HashMap<&'static str, TypeId>,
Expand Down
3 changes: 1 addition & 2 deletions crates/bevy_ecs/src/bundle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
//! This module contains the [`Bundle`] trait and some other helper types.

pub use bevy_ecs_macros::Bundle;
use bevy_utils::{HashMap, HashSet};
use bevy_utils::{HashMap, HashSet, TypeIdMap};

use crate::{
archetype::{
Expand All @@ -14,7 +14,6 @@ use crate::{
entity::{Entities, Entity, EntityLocation},
query::DebugCheckedUnwrap,
storage::{SparseSetIndex, SparseSets, Storages, Table, TableRow},
TypeIdMap,
};
use bevy_ptr::OwningPtr;
use bevy_utils::all_tuples;
Expand Down
2 changes: 1 addition & 1 deletion crates/bevy_ecs/src/component.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@ use crate::{
storage::{SparseSetIndex, Storages},
system::{Local, Resource, SystemParam},
world::{FromWorld, World},
TypeIdMap,
};
pub use bevy_ecs_macros::Component;
use bevy_ptr::{OwningPtr, UnsafeCellDeref};
#[cfg(feature = "bevy_reflect")]
use bevy_reflect::Reflect;
use bevy_utils::TypeIdMap;
use std::cell::UnsafeCell;
use std::{
alloc::Layout,
Expand Down
47 changes: 0 additions & 47 deletions crates/bevy_ecs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ pub mod storage;
pub mod system;
pub mod world;

use std::any::TypeId;

pub use bevy_ptr as ptr;

/// Most commonly used re-exported types.
Expand Down Expand Up @@ -56,34 +54,6 @@ pub mod prelude {

pub use bevy_utils::all_tuples;

/// A specialized hashmap type with Key of [`TypeId`]
type TypeIdMap<V> =
std::collections::HashMap<TypeId, V, std::hash::BuildHasherDefault<NoOpTypeIdHasher>>;

#[doc(hidden)]
#[derive(Default)]
struct NoOpTypeIdHasher(u64);

// TypeId already contains a high-quality hash, so skip re-hashing that hash.
impl std::hash::Hasher for NoOpTypeIdHasher {
fn finish(&self) -> u64 {
self.0
}

fn write(&mut self, bytes: &[u8]) {
// This will never be called: TypeId always just calls write_u64 once!
// This is a known trick and unlikely to change, but isn't officially guaranteed.
// Don't break applications (slower fallback, just check in test):
self.0 = bytes.iter().fold(self.0, |hash, b| {
hash.rotate_left(8).wrapping_add(*b as u64)
});
}

fn write_u64(&mut self, i: u64) {
self.0 = i;
}
}

#[cfg(test)]
mod tests {
use crate as bevy_ecs;
Expand Down Expand Up @@ -1755,23 +1725,6 @@ mod tests {
);
}

#[test]
fn fast_typeid_hash() {
struct Hasher;

impl std::hash::Hasher for Hasher {
fn finish(&self) -> u64 {
0
}
fn write(&mut self, _: &[u8]) {
panic!("Hashing of std::any::TypeId changed");
}
fn write_u64(&mut self, _: u64) {}
}

std::hash::Hash::hash(&TypeId::of::<()>(), &mut Hasher);
}

#[derive(Component)]
struct ComponentA(u32);

Expand Down
3 changes: 2 additions & 1 deletion crates/bevy_ecs/src/schedule/stepping.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use crate::{
use bevy_utils::{
thiserror::Error,
tracing::{error, info, warn},
TypeIdMap,
};

#[cfg(test)]
Expand Down Expand Up @@ -617,7 +618,7 @@ struct ScheduleState {

/// changes to system behavior that should be applied the next time
/// [`ScheduleState::skipped_systems()`] is called
behavior_updates: HashMap<TypeId, Option<SystemBehavior>>,
behavior_updates: TypeIdMap<Option<SystemBehavior>>,

/// This field contains the first steppable system in the schedule.
first: Option<usize>,
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_gizmos/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ pub use bevy_gizmos_macros::GizmoConfigGroup;
use bevy_ecs::{component::Component, system::Resource};
use bevy_reflect::{Reflect, TypePath};
use bevy_render::view::RenderLayers;
use bevy_utils::HashMap;
use bevy_utils::TypeIdMap;
use core::panic;
use std::{
any::TypeId,
Expand All @@ -30,7 +30,7 @@ pub struct DefaultGizmoConfigGroup;
#[derive(Resource, Default)]
pub struct GizmoConfigStore {
// INVARIANT: must map TypeId::of::<T>() to correct type T
store: HashMap<TypeId, (GizmoConfig, Box<dyn Reflect>)>,
store: TypeIdMap<(GizmoConfig, Box<dyn Reflect>)>,
}

impl GizmoConfigStore {
Expand Down
6 changes: 3 additions & 3 deletions crates/bevy_gizmos/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ use bevy_render::{
renderer::RenderDevice,
Extract, ExtractSchedule, Render, RenderApp, RenderSet,
};
use bevy_utils::HashMap;
use bevy_utils::TypeIdMap;
use config::{
DefaultGizmoConfigGroup, GizmoConfig, GizmoConfigGroup, GizmoConfigStore, GizmoMeshConfig,
};
Expand Down Expand Up @@ -206,8 +206,8 @@ impl AppGizmoBuilder for App {

#[derive(Resource, Default)]
struct LineGizmoHandles {
list: HashMap<TypeId, Handle<LineGizmo>>,
strip: HashMap<TypeId, Handle<LineGizmo>>,
list: TypeIdMap<Handle<LineGizmo>>,
strip: TypeIdMap<Handle<LineGizmo>>,
}

fn update_gizmo_meshes<T: GizmoConfigGroup>(
Expand Down
10 changes: 5 additions & 5 deletions crates/bevy_reflect/src/type_registry.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::{serde::Serializable, Reflect, TypeInfo, TypePath, Typed};
use bevy_ptr::{Ptr, PtrMut};
use bevy_utils::{HashMap, HashSet};
use bevy_utils::{HashMap, HashSet, TypeIdMap};
use downcast_rs::{impl_downcast, Downcast};
use serde::Deserialize;
use std::{
Expand All @@ -22,7 +22,7 @@ use std::{
/// [Registering]: TypeRegistry::register
/// [crate-level documentation]: crate
pub struct TypeRegistry {
registrations: HashMap<TypeId, TypeRegistration>,
registrations: TypeIdMap<TypeRegistration>,
short_path_to_id: HashMap<&'static str, TypeId>,
type_path_to_id: HashMap<&'static str, TypeId>,
ambiguous_names: HashSet<&'static str>,
Expand Down Expand Up @@ -318,7 +318,7 @@ impl TypeRegistryArc {
///
/// [crate-level documentation]: crate
pub struct TypeRegistration {
data: HashMap<TypeId, Box<dyn TypeData>>,
data: TypeIdMap<Box<dyn TypeData>>,
type_info: &'static TypeInfo,
}

Expand Down Expand Up @@ -373,15 +373,15 @@ impl TypeRegistration {
/// Creates type registration information for `T`.
pub fn of<T: Reflect + Typed + TypePath>() -> Self {
Self {
data: HashMap::default(),
data: Default::default(),
type_info: T::type_info(),
}
}
}

impl Clone for TypeRegistration {
fn clone(&self) -> Self {
let mut data = HashMap::default();
let mut data = TypeIdMap::default();
for (id, type_data) in &self.data {
data.insert(*id, (*type_data).clone_type_data());
}
Expand Down
6 changes: 3 additions & 3 deletions crates/bevy_render/src/render_phase/draw.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use bevy_ecs::{
system::{ReadOnlySystemParam, Resource, SystemParam, SystemParamItem, SystemState},
world::World,
};
use bevy_utils::{all_tuples, HashMap};
use bevy_utils::{all_tuples, TypeIdMap};
use std::{
any::TypeId,
fmt::Debug,
Expand Down Expand Up @@ -47,7 +47,7 @@ pub struct DrawFunctionId(u32);
/// For retrieval, the [`Draw`] functions are mapped to their respective [`TypeId`]s.
pub struct DrawFunctionsInternal<P: PhaseItem> {
pub draw_functions: Vec<Box<dyn Draw<P>>>,
pub indices: HashMap<TypeId, DrawFunctionId>,
pub indices: TypeIdMap<DrawFunctionId>,
}

impl<P: PhaseItem> DrawFunctionsInternal<P> {
Expand Down Expand Up @@ -111,7 +111,7 @@ impl<P: PhaseItem> Default for DrawFunctions<P> {
Self {
internal: RwLock::new(DrawFunctionsInternal {
draw_functions: Vec::new(),
indices: HashMap::default(),
indices: Default::default(),
}),
}
}
Expand Down
7 changes: 3 additions & 4 deletions crates/bevy_scene/src/dynamic_scene.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,7 @@ use bevy_ecs::{
world::World,
};
use bevy_reflect::{Reflect, TypePath, TypeRegistryArc};
use bevy_utils::{EntityHashMap, HashMap};
use std::any::TypeId;
use bevy_utils::{EntityHashMap, TypeIdMap};

#[cfg(feature = "serialize")]
use crate::serde::SceneSerializer;
Expand Down Expand Up @@ -97,7 +96,7 @@ impl DynamicScene {
// of which entities in the scene use that component.
// This is so we can update the scene-internal references to references
// of the actual entities in the world.
let mut scene_mappings: HashMap<TypeId, Vec<Entity>> = HashMap::default();
let mut scene_mappings: TypeIdMap<Vec<Entity>> = Default::default();

for scene_entity in &self.entities {
// Fetch the entity with the given entity id from the `entity_map`
Expand Down Expand Up @@ -132,7 +131,7 @@ impl DynamicScene {
if registration.data::<ReflectMapEntities>().is_some() {
scene_mappings
.entry(registration.type_id())
.or_insert(Vec::new())
.or_default()
.push(entity);
}

Expand Down
Loading

0 comments on commit f2a8400

Please sign in to comment.