Use immutable key for HashMap and HashSet (#12086)

# Objective Memory usage optimisation ## Solution `HashMap` and `HashSet`'s keys are immutable. So using mutable types like `String`, `Vec<T>`, or `PathBuf` as a key is a waste of memory: they have an extra `usize` for their capacity and may have spare capacity. This PR replaces these types by their immutable equivalents `Box<str>`, `Box<[T]>`, and `Box<Path>`. For more context, I recommend watching the [Use Arc Instead of Vec](https://www.youtube.com/watch?v=A4cKi7PTJSs) video. --------- Co-authored-by: James Liu <[email protected]>
bevyengine · Feb 26, 2024 · 1cded6a · 1cded6a
1 parent c97d010
commit 1cded6a
Show file tree

Hide file tree

Showing 17 changed files with 72 additions and 79 deletions.
diff --git a/crates/bevy_app/src/app.rs b/crates/bevy_app/src/app.rs
@@ -78,7 +78,7 @@ pub struct App {
     pub main_schedule_label: InternedScheduleLabel,
     sub_apps: HashMap<InternedAppLabel, SubApp>,
     plugin_registry: Vec<Box<dyn Plugin>>,
-    plugin_name_added: HashSet<String>,
+    plugin_name_added: HashSet<Box<str>>,
     /// A private counter to prevent incorrect calls to `App::run()` from `Plugin::build()`
     building_plugin_depth: usize,
     plugins_state: PluginsState,
@@ -642,7 +642,7 @@ impl App {
         plugin: Box<dyn Plugin>,
     ) -> Result<&mut Self, AppError> {
         debug!("added plugin: {}", plugin.name());
-        if plugin.is_unique() && !self.plugin_name_added.insert(plugin.name().to_string()) {
+        if plugin.is_unique() && !self.plugin_name_added.insert(plugin.name().into()) {
             Err(AppError::DuplicatePlugin {
                 plugin_name: plugin.name().to_string(),
             })?;

diff --git a/crates/bevy_asset/src/io/embedded/embedded_watcher.rs b/crates/bevy_asset/src/io/embedded/embedded_watcher.rs
@@ -25,7 +25,7 @@ pub struct EmbeddedWatcher {
 impl EmbeddedWatcher {
     pub fn new(
         dir: Dir,
-        root_paths: Arc<RwLock<HashMap<PathBuf, PathBuf>>>,
+        root_paths: Arc<RwLock<HashMap<Box<Path>, PathBuf>>>,
         sender: crossbeam_channel::Sender<AssetSourceEvent>,
         debounce_wait_time: Duration,
     ) -> Self {
@@ -49,7 +49,7 @@ impl AssetWatcher for EmbeddedWatcher {}
 /// the initial static bytes from the file embedded in the binary.
 pub(crate) struct EmbeddedEventHandler {
     sender: crossbeam_channel::Sender<AssetSourceEvent>,
-    root_paths: Arc<RwLock<HashMap<PathBuf, PathBuf>>>,
+    root_paths: Arc<RwLock<HashMap<Box<Path>, PathBuf>>>,
     root: PathBuf,
     dir: Dir,
     last_event: Option<AssetSourceEvent>,
@@ -61,7 +61,7 @@ impl FilesystemEventHandler for EmbeddedEventHandler {
 
     fn get_path(&self, absolute_path: &Path) -> Option<(PathBuf, bool)> {
         let (local_path, is_meta) = get_asset_path(&self.root, absolute_path);
-        let final_path = self.root_paths.read().get(&local_path)?.clone();
+        let final_path = self.root_paths.read().get(local_path.as_path())?.clone();
         if is_meta {
             warn!("Meta file asset hot-reloading is not supported yet: {final_path:?}");
         }

diff --git a/crates/bevy_asset/src/io/embedded/mod.rs b/crates/bevy_asset/src/io/embedded/mod.rs
@@ -22,7 +22,7 @@ pub const EMBEDDED: &str = "embedded";
 pub struct EmbeddedAssetRegistry {
     dir: Dir,
     #[cfg(feature = "embedded_watcher")]
-    root_paths: std::sync::Arc<parking_lot::RwLock<bevy_utils::HashMap<PathBuf, PathBuf>>>,
+    root_paths: std::sync::Arc<parking_lot::RwLock<bevy_utils::HashMap<Box<Path>, PathBuf>>>,
 }
 
 impl EmbeddedAssetRegistry {
@@ -35,7 +35,7 @@ impl EmbeddedAssetRegistry {
         #[cfg(feature = "embedded_watcher")]
         self.root_paths
             .write()
-            .insert(full_path.to_owned(), asset_path.to_owned());
+            .insert(full_path.into(), asset_path.to_owned());
         self.dir.insert_asset(asset_path, value);
     }
 
@@ -48,7 +48,7 @@ impl EmbeddedAssetRegistry {
         #[cfg(feature = "embedded_watcher")]
         self.root_paths
             .write()
-            .insert(full_path.to_owned(), asset_path.to_owned());
+            .insert(full_path.into(), asset_path.to_owned());
         self.dir.insert_meta(asset_path, value);
     }
 

diff --git a/crates/bevy_asset/src/io/gated.rs b/crates/bevy_asset/src/io/gated.rs
@@ -2,18 +2,15 @@ use crate::io::{AssetReader, AssetReaderError, PathStream, Reader};
 use bevy_utils::{BoxedFuture, HashMap};
 use crossbeam_channel::{Receiver, Sender};
 use parking_lot::RwLock;
-use std::{
-    path::{Path, PathBuf},
-    sync::Arc,
-};
+use std::{path::Path, sync::Arc};
 
 /// A "gated" reader that will prevent asset reads from returning until
 /// a given path has been "opened" using [`GateOpener`].
 ///
 /// This is built primarily for unit tests.
 pub struct GatedReader<R: AssetReader> {
     reader: R,
-    gates: Arc<RwLock<HashMap<PathBuf, (Sender<()>, Receiver<()>)>>>,
+    gates: Arc<RwLock<HashMap<Box<Path>, (Sender<()>, Receiver<()>)>>>,
 }
 
 impl<R: AssetReader + Clone> Clone for GatedReader<R> {
@@ -27,7 +24,7 @@ impl<R: AssetReader + Clone> Clone for GatedReader<R> {
 
 /// Opens path "gates" for a [`GatedReader`].
 pub struct GateOpener {
-    gates: Arc<RwLock<HashMap<PathBuf, (Sender<()>, Receiver<()>)>>>,
+    gates: Arc<RwLock<HashMap<Box<Path>, (Sender<()>, Receiver<()>)>>>,
 }
 
 impl GateOpener {
@@ -36,7 +33,7 @@ impl GateOpener {
     pub fn open<P: AsRef<Path>>(&self, path: P) {
         let mut gates = self.gates.write();
         let gates = gates
-            .entry(path.as_ref().to_path_buf())
+            .entry_ref(path.as_ref())
             .or_insert_with(crossbeam_channel::unbounded);
         gates.0.send(()).unwrap();
     }
@@ -65,7 +62,7 @@ impl<R: AssetReader> AssetReader for GatedReader<R> {
         let receiver = {
             let mut gates = self.gates.write();
             let gates = gates
-                .entry(path.to_path_buf())
+                .entry_ref(path.as_ref())
                 .or_insert_with(crossbeam_channel::unbounded);
             gates.1.clone()
         };

diff --git a/crates/bevy_asset/src/io/memory.rs b/crates/bevy_asset/src/io/memory.rs
@@ -12,9 +12,9 @@ use std::{
 
 #[derive(Default, Debug)]
 struct DirInternal {
-    assets: HashMap<String, Data>,
-    metadata: HashMap<String, Data>,
-    dirs: HashMap<String, Dir>,
+    assets: HashMap<Box<str>, Data>,
+    metadata: HashMap<Box<str>, Data>,
+    dirs: HashMap<Box<str>, Dir>,
     path: PathBuf,
 }
 
@@ -46,7 +46,7 @@ impl Dir {
             dir = self.get_or_insert_dir(parent);
         }
         dir.0.write().assets.insert(
-            path.file_name().unwrap().to_string_lossy().to_string(),
+            path.file_name().unwrap().to_string_lossy().into(),
             Data {
                 value: value.into(),
                 path: path.to_owned(),
@@ -60,7 +60,7 @@ impl Dir {
             dir = self.get_or_insert_dir(parent);
         }
         dir.0.write().metadata.insert(
-            path.file_name().unwrap().to_string_lossy().to_string(),
+            path.file_name().unwrap().to_string_lossy().into(),
             Data {
                 value: value.into(),
                 path: path.to_owned(),
@@ -73,7 +73,7 @@ impl Dir {
         let mut full_path = PathBuf::new();
         for c in path.components() {
             full_path.push(c);
-            let name = c.as_os_str().to_string_lossy().to_string();
+            let name = c.as_os_str().to_string_lossy().into();
             dir = {
                 let dirs = &mut dir.0.write().dirs;
                 dirs.entry(name)
@@ -147,7 +147,12 @@ impl Stream for DirStream {
         let dir = this.dir.0.read();
 
         let dir_index = this.dir_index;
-        if let Some(dir_path) = dir.dirs.keys().nth(dir_index).map(|d| dir.path.join(d)) {
+        if let Some(dir_path) = dir
+            .dirs
+            .keys()
+            .nth(dir_index)
+            .map(|d| dir.path.join(d.as_ref()))
+        {
             this.dir_index += 1;
             Poll::Ready(Some(dir_path))
         } else {

diff --git a/crates/bevy_asset/src/lib.rs b/crates/bevy_asset/src/lib.rs
@@ -451,10 +451,7 @@ mod tests {
     use bevy_utils::{BoxedFuture, Duration, HashMap};
     use futures_lite::AsyncReadExt;
     use serde::{Deserialize, Serialize};
-    use std::{
-        path::{Path, PathBuf},
-        sync::Arc,
-    };
+    use std::{path::Path, sync::Arc};
     use thiserror::Error;
 
     #[derive(Asset, TypePath, Debug, Default)]
@@ -545,7 +542,7 @@ mod tests {
     /// A dummy [`CoolText`] asset reader that only succeeds after `failure_count` times it's read from for each asset.
     #[derive(Default, Clone)]
     pub struct UnstableMemoryAssetReader {
-        pub attempt_counters: Arc<std::sync::Mutex<HashMap<PathBuf, usize>>>,
+        pub attempt_counters: Arc<std::sync::Mutex<HashMap<Box<Path>, usize>>>,
         pub load_delay: Duration,
         memory_reader: MemoryAssetReader,
         failure_count: usize,
@@ -589,13 +586,12 @@ mod tests {
             Result<Box<bevy_asset::io::Reader<'a>>, bevy_asset::io::AssetReaderError>,
         > {
             let attempt_number = {
-                let key = PathBuf::from(path);
                 let mut attempt_counters = self.attempt_counters.lock().unwrap();
-                if let Some(existing) = attempt_counters.get_mut(&key) {
+                if let Some(existing) = attempt_counters.get_mut(path) {
                     *existing += 1;
                     *existing
                 } else {
-                    attempt_counters.insert(key, 1);
+                    attempt_counters.insert(path.into(), 1);
                     1
                 }
             };

diff --git a/crates/bevy_asset/src/processor/mod.rs b/crates/bevy_asset/src/processor/mod.rs
@@ -56,7 +56,7 @@ pub struct AssetProcessorData {
     log: async_lock::RwLock<Option<ProcessorTransactionLog>>,
     processors: RwLock<HashMap<&'static str, Arc<dyn ErasedProcessor>>>,
     /// Default processors for file extensions
-    default_processors: RwLock<HashMap<String, &'static str>>,
+    default_processors: RwLock<HashMap<Box<str>, &'static str>>,
     state: async_lock::RwLock<ProcessorState>,
     sources: AssetSources,
     initialized_sender: async_broadcast::Sender<()>,
@@ -482,7 +482,7 @@ impl AssetProcessor {
     /// Set the default processor for the given `extension`. Make sure `P` is registered with [`AssetProcessor::register_processor`].
     pub fn set_default_processor<P: Process>(&self, extension: &str) {
         let mut default_processors = self.data.default_processors.write();
-        default_processors.insert(extension.to_string(), std::any::type_name::<P>());
+        default_processors.insert(extension.into(), std::any::type_name::<P>());
     }
 
     /// Returns the default processor for the given `extension`, if it exists.

diff --git a/crates/bevy_asset/src/server/info.rs b/crates/bevy_asset/src/server/info.rs
@@ -71,7 +71,7 @@ pub(crate) struct AssetInfos {
     pub(crate) loader_dependants: HashMap<AssetPath<'static>, HashSet<AssetPath<'static>>>,
     /// 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) living_labeled_assets: HashMap<AssetPath<'static>, HashSet<Box<str>>>,
     pub(crate) handle_providers: TypeIdMap<AssetHandleProvider>,
     pub(crate) dependency_loaded_event_sender: TypeIdMap<fn(&mut World, UntypedAssetId)>,
     pub(crate) dependency_failed_event_sender:
@@ -113,7 +113,7 @@ impl AssetInfos {
     fn create_handle_internal(
         infos: &mut HashMap<UntypedAssetId, AssetInfo>,
         handle_providers: &TypeIdMap<AssetHandleProvider>,
-        living_labeled_assets: &mut HashMap<AssetPath<'static>, HashSet<String>>,
+        living_labeled_assets: &mut HashMap<AssetPath<'static>, HashSet<Box<str>>>,
         watching_for_changes: bool,
         type_id: TypeId,
         path: Option<AssetPath<'static>>,
@@ -129,7 +129,7 @@ impl AssetInfos {
                 let mut without_label = path.to_owned();
                 if let Some(label) = without_label.take_label() {
                     let labels = living_labeled_assets.entry(without_label).or_default();
-                    labels.insert(label.to_string());
+                    labels.insert(label.as_ref().into());
                 }
             }
         }
@@ -613,7 +613,7 @@ impl AssetInfos {
         info: &AssetInfo,
         loader_dependants: &mut HashMap<AssetPath<'static>, HashSet<AssetPath<'static>>>,
         path: &AssetPath<'static>,
-        living_labeled_assets: &mut HashMap<AssetPath<'static>, HashSet<String>>,
+        living_labeled_assets: &mut HashMap<AssetPath<'static>, HashSet<Box<str>>>,
     ) {
         for loader_dependency in info.loader_dependencies.keys() {
             if let Some(dependants) = loader_dependants.get_mut(loader_dependency) {
@@ -642,7 +642,7 @@ impl AssetInfos {
         infos: &mut HashMap<UntypedAssetId, AssetInfo>,
         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>>,
+        living_labeled_assets: &mut HashMap<AssetPath<'static>, HashSet<Box<str>>>,
         watching_for_changes: bool,
         id: UntypedAssetId,
     ) -> bool {

diff --git a/crates/bevy_asset/src/server/loaders.rs b/crates/bevy_asset/src/server/loaders.rs
@@ -13,7 +13,7 @@ use thiserror::Error;
 pub(crate) struct AssetLoaders {
     loaders: Vec<MaybeAssetLoader>,
     type_id_to_loaders: TypeIdMap<Vec<usize>>,
-    extension_to_loaders: HashMap<String, Vec<usize>>,
+    extension_to_loaders: HashMap<Box<str>, Vec<usize>>,
     type_name_to_loader: HashMap<&'static str, usize>,
     preregistered_loaders: HashMap<&'static str, usize>,
 }
@@ -44,7 +44,7 @@ impl AssetLoaders {
             for extension in loader.extensions() {
                 let list = self
                     .extension_to_loaders
-                    .entry(extension.to_string())
+                    .entry((*extension).into())
                     .or_default();
 
                 if !list.is_empty() {
@@ -105,7 +105,7 @@ impl AssetLoaders {
         for extension in extensions {
             let list = self
                 .extension_to_loaders
-                .entry(extension.to_string())
+                .entry((*extension).into())
                 .or_default();
 
             if !list.is_empty() {

diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs
@@ -806,7 +806,7 @@ pub struct Bundles {
     /// Cache static [`BundleId`]
     bundle_ids: TypeIdMap<BundleId>,
     /// Cache dynamic [`BundleId`] with multiple components
-    dynamic_bundle_ids: HashMap<Vec<ComponentId>, (BundleId, Vec<StorageType>)>,
+    dynamic_bundle_ids: HashMap<Box<[ComponentId]>, (BundleId, Vec<StorageType>)>,
     /// Cache optimized dynamic [`BundleId`] with single component
     dynamic_component_bundle_ids: HashMap<ComponentId, (BundleId, StorageType)>,
 }
@@ -871,7 +871,7 @@ impl Bundles {
             .from_key(component_ids)
             .or_insert_with(|| {
                 (
-                    Vec::from(component_ids),
+                    component_ids.into(),
                     initialize_dynamic_bundle(bundle_infos, components, Vec::from(component_ids)),
                 )
             });

diff --git a/crates/bevy_ecs/src/storage/table.rs b/crates/bevy_ecs/src/storage/table.rs
@@ -796,7 +796,7 @@ impl Table {
 /// Can be accessed via [`Storages`](crate::storage::Storages)
 pub struct Tables {
     tables: Vec<Table>,
-    table_ids: HashMap<Vec<ComponentId>, TableId>,
+    table_ids: HashMap<Box<[ComponentId]>, TableId>,
 }
 
 impl Default for Tables {
@@ -872,10 +872,7 @@ impl Tables {
                     table = table.add_column(components.get_info_unchecked(*component_id));
                 }
                 tables.push(table.build());
-                (
-                    component_ids.to_vec(),
-                    TableId::from_usize(tables.len() - 1),
-                )
+                (component_ids.into(), TableId::from_usize(tables.len() - 1))
             });
 
         *value

diff --git a/crates/bevy_gltf/src/lib.rs b/crates/bevy_gltf/src/lib.rs
@@ -26,7 +26,7 @@ use bevy_scene::Scene;
 /// Adds support for glTF file loading to the app.
 #[derive(Default)]
 pub struct GltfPlugin {
-    custom_vertex_attributes: HashMap<String, MeshVertexAttribute>,
+    custom_vertex_attributes: HashMap<Box<str>, MeshVertexAttribute>,
 }
 
 impl GltfPlugin {
@@ -40,8 +40,7 @@ impl GltfPlugin {
         name: &str,
         attribute: MeshVertexAttribute,
     ) -> Self {
-        self.custom_vertex_attributes
-            .insert(name.to_string(), attribute);
+        self.custom_vertex_attributes.insert(name.into(), attribute);
         self
     }
 }
@@ -75,27 +74,27 @@ pub struct Gltf {
     /// All scenes loaded from the glTF file.
     pub scenes: Vec<Handle<Scene>>,
     /// Named scenes loaded from the glTF file.
-    pub named_scenes: HashMap<String, Handle<Scene>>,
+    pub named_scenes: HashMap<Box<str>, Handle<Scene>>,
     /// All meshes loaded from the glTF file.
     pub meshes: Vec<Handle<GltfMesh>>,
     /// Named meshes loaded from the glTF file.
-    pub named_meshes: HashMap<String, Handle<GltfMesh>>,
+    pub named_meshes: HashMap<Box<str>, Handle<GltfMesh>>,
     /// All materials loaded from the glTF file.
     pub materials: Vec<Handle<StandardMaterial>>,
     /// Named materials loaded from the glTF file.
-    pub named_materials: HashMap<String, Handle<StandardMaterial>>,
+    pub named_materials: HashMap<Box<str>, Handle<StandardMaterial>>,
     /// All nodes loaded from the glTF file.
     pub nodes: Vec<Handle<GltfNode>>,
     /// Named nodes loaded from the glTF file.
-    pub named_nodes: HashMap<String, Handle<GltfNode>>,
+    pub named_nodes: HashMap<Box<str>, Handle<GltfNode>>,
     /// Default scene to be displayed.
     pub default_scene: Option<Handle<Scene>>,
     /// All animations loaded from the glTF file.
     #[cfg(feature = "bevy_animation")]
     pub animations: Vec<Handle<AnimationClip>>,
     /// Named animations loaded from the glTF file.
     #[cfg(feature = "bevy_animation")]
-    pub named_animations: HashMap<String, Handle<AnimationClip>>,
+    pub named_animations: HashMap<Box<str>, Handle<AnimationClip>>,
     /// The gltf root of the gltf asset, see <https://docs.rs/gltf/latest/gltf/struct.Gltf.html>. Only has a value when `GltfLoaderSettings::include_source` is true.
     pub source: Option<gltf::Gltf>,
 }