Skip to content

Commit 1cded6a

Browse files
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]>
1 parent c97d010 commit 1cded6a

File tree

17 files changed

+72
-79
lines changed

17 files changed

+72
-79
lines changed

crates/bevy_app/src/app.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ pub struct App {
7878
pub main_schedule_label: InternedScheduleLabel,
7979
sub_apps: HashMap<InternedAppLabel, SubApp>,
8080
plugin_registry: Vec<Box<dyn Plugin>>,
81-
plugin_name_added: HashSet<String>,
81+
plugin_name_added: HashSet<Box<str>>,
8282
/// A private counter to prevent incorrect calls to `App::run()` from `Plugin::build()`
8383
building_plugin_depth: usize,
8484
plugins_state: PluginsState,
@@ -642,7 +642,7 @@ impl App {
642642
plugin: Box<dyn Plugin>,
643643
) -> Result<&mut Self, AppError> {
644644
debug!("added plugin: {}", plugin.name());
645-
if plugin.is_unique() && !self.plugin_name_added.insert(plugin.name().to_string()) {
645+
if plugin.is_unique() && !self.plugin_name_added.insert(plugin.name().into()) {
646646
Err(AppError::DuplicatePlugin {
647647
plugin_name: plugin.name().to_string(),
648648
})?;

crates/bevy_asset/src/io/embedded/embedded_watcher.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ pub struct EmbeddedWatcher {
2525
impl EmbeddedWatcher {
2626
pub fn new(
2727
dir: Dir,
28-
root_paths: Arc<RwLock<HashMap<PathBuf, PathBuf>>>,
28+
root_paths: Arc<RwLock<HashMap<Box<Path>, PathBuf>>>,
2929
sender: crossbeam_channel::Sender<AssetSourceEvent>,
3030
debounce_wait_time: Duration,
3131
) -> Self {
@@ -49,7 +49,7 @@ impl AssetWatcher for EmbeddedWatcher {}
4949
/// the initial static bytes from the file embedded in the binary.
5050
pub(crate) struct EmbeddedEventHandler {
5151
sender: crossbeam_channel::Sender<AssetSourceEvent>,
52-
root_paths: Arc<RwLock<HashMap<PathBuf, PathBuf>>>,
52+
root_paths: Arc<RwLock<HashMap<Box<Path>, PathBuf>>>,
5353
root: PathBuf,
5454
dir: Dir,
5555
last_event: Option<AssetSourceEvent>,
@@ -61,7 +61,7 @@ impl FilesystemEventHandler for EmbeddedEventHandler {
6161

6262
fn get_path(&self, absolute_path: &Path) -> Option<(PathBuf, bool)> {
6363
let (local_path, is_meta) = get_asset_path(&self.root, absolute_path);
64-
let final_path = self.root_paths.read().get(&local_path)?.clone();
64+
let final_path = self.root_paths.read().get(local_path.as_path())?.clone();
6565
if is_meta {
6666
warn!("Meta file asset hot-reloading is not supported yet: {final_path:?}");
6767
}

crates/bevy_asset/src/io/embedded/mod.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ pub const EMBEDDED: &str = "embedded";
2222
pub struct EmbeddedAssetRegistry {
2323
dir: Dir,
2424
#[cfg(feature = "embedded_watcher")]
25-
root_paths: std::sync::Arc<parking_lot::RwLock<bevy_utils::HashMap<PathBuf, PathBuf>>>,
25+
root_paths: std::sync::Arc<parking_lot::RwLock<bevy_utils::HashMap<Box<Path>, PathBuf>>>,
2626
}
2727

2828
impl EmbeddedAssetRegistry {
@@ -35,7 +35,7 @@ impl EmbeddedAssetRegistry {
3535
#[cfg(feature = "embedded_watcher")]
3636
self.root_paths
3737
.write()
38-
.insert(full_path.to_owned(), asset_path.to_owned());
38+
.insert(full_path.into(), asset_path.to_owned());
3939
self.dir.insert_asset(asset_path, value);
4040
}
4141

@@ -48,7 +48,7 @@ impl EmbeddedAssetRegistry {
4848
#[cfg(feature = "embedded_watcher")]
4949
self.root_paths
5050
.write()
51-
.insert(full_path.to_owned(), asset_path.to_owned());
51+
.insert(full_path.into(), asset_path.to_owned());
5252
self.dir.insert_meta(asset_path, value);
5353
}
5454

crates/bevy_asset/src/io/gated.rs

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,18 +2,15 @@ use crate::io::{AssetReader, AssetReaderError, PathStream, Reader};
22
use bevy_utils::{BoxedFuture, HashMap};
33
use crossbeam_channel::{Receiver, Sender};
44
use parking_lot::RwLock;
5-
use std::{
6-
path::{Path, PathBuf},
7-
sync::Arc,
8-
};
5+
use std::{path::Path, sync::Arc};
96

107
/// A "gated" reader that will prevent asset reads from returning until
118
/// a given path has been "opened" using [`GateOpener`].
129
///
1310
/// This is built primarily for unit tests.
1411
pub struct GatedReader<R: AssetReader> {
1512
reader: R,
16-
gates: Arc<RwLock<HashMap<PathBuf, (Sender<()>, Receiver<()>)>>>,
13+
gates: Arc<RwLock<HashMap<Box<Path>, (Sender<()>, Receiver<()>)>>>,
1714
}
1815

1916
impl<R: AssetReader + Clone> Clone for GatedReader<R> {
@@ -27,7 +24,7 @@ impl<R: AssetReader + Clone> Clone for GatedReader<R> {
2724

2825
/// Opens path "gates" for a [`GatedReader`].
2926
pub struct GateOpener {
30-
gates: Arc<RwLock<HashMap<PathBuf, (Sender<()>, Receiver<()>)>>>,
27+
gates: Arc<RwLock<HashMap<Box<Path>, (Sender<()>, Receiver<()>)>>>,
3128
}
3229

3330
impl GateOpener {
@@ -36,7 +33,7 @@ impl GateOpener {
3633
pub fn open<P: AsRef<Path>>(&self, path: P) {
3734
let mut gates = self.gates.write();
3835
let gates = gates
39-
.entry(path.as_ref().to_path_buf())
36+
.entry_ref(path.as_ref())
4037
.or_insert_with(crossbeam_channel::unbounded);
4138
gates.0.send(()).unwrap();
4239
}
@@ -65,7 +62,7 @@ impl<R: AssetReader> AssetReader for GatedReader<R> {
6562
let receiver = {
6663
let mut gates = self.gates.write();
6764
let gates = gates
68-
.entry(path.to_path_buf())
65+
.entry_ref(path.as_ref())
6966
.or_insert_with(crossbeam_channel::unbounded);
7067
gates.1.clone()
7168
};

crates/bevy_asset/src/io/memory.rs

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,9 @@ use std::{
1212

1313
#[derive(Default, Debug)]
1414
struct DirInternal {
15-
assets: HashMap<String, Data>,
16-
metadata: HashMap<String, Data>,
17-
dirs: HashMap<String, Dir>,
15+
assets: HashMap<Box<str>, Data>,
16+
metadata: HashMap<Box<str>, Data>,
17+
dirs: HashMap<Box<str>, Dir>,
1818
path: PathBuf,
1919
}
2020

@@ -46,7 +46,7 @@ impl Dir {
4646
dir = self.get_or_insert_dir(parent);
4747
}
4848
dir.0.write().assets.insert(
49-
path.file_name().unwrap().to_string_lossy().to_string(),
49+
path.file_name().unwrap().to_string_lossy().into(),
5050
Data {
5151
value: value.into(),
5252
path: path.to_owned(),
@@ -60,7 +60,7 @@ impl Dir {
6060
dir = self.get_or_insert_dir(parent);
6161
}
6262
dir.0.write().metadata.insert(
63-
path.file_name().unwrap().to_string_lossy().to_string(),
63+
path.file_name().unwrap().to_string_lossy().into(),
6464
Data {
6565
value: value.into(),
6666
path: path.to_owned(),
@@ -73,7 +73,7 @@ impl Dir {
7373
let mut full_path = PathBuf::new();
7474
for c in path.components() {
7575
full_path.push(c);
76-
let name = c.as_os_str().to_string_lossy().to_string();
76+
let name = c.as_os_str().to_string_lossy().into();
7777
dir = {
7878
let dirs = &mut dir.0.write().dirs;
7979
dirs.entry(name)
@@ -147,7 +147,12 @@ impl Stream for DirStream {
147147
let dir = this.dir.0.read();
148148

149149
let dir_index = this.dir_index;
150-
if let Some(dir_path) = dir.dirs.keys().nth(dir_index).map(|d| dir.path.join(d)) {
150+
if let Some(dir_path) = dir
151+
.dirs
152+
.keys()
153+
.nth(dir_index)
154+
.map(|d| dir.path.join(d.as_ref()))
155+
{
151156
this.dir_index += 1;
152157
Poll::Ready(Some(dir_path))
153158
} else {

crates/bevy_asset/src/lib.rs

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -451,10 +451,7 @@ mod tests {
451451
use bevy_utils::{BoxedFuture, Duration, HashMap};
452452
use futures_lite::AsyncReadExt;
453453
use serde::{Deserialize, Serialize};
454-
use std::{
455-
path::{Path, PathBuf},
456-
sync::Arc,
457-
};
454+
use std::{path::Path, sync::Arc};
458455
use thiserror::Error;
459456

460457
#[derive(Asset, TypePath, Debug, Default)]
@@ -545,7 +542,7 @@ mod tests {
545542
/// A dummy [`CoolText`] asset reader that only succeeds after `failure_count` times it's read from for each asset.
546543
#[derive(Default, Clone)]
547544
pub struct UnstableMemoryAssetReader {
548-
pub attempt_counters: Arc<std::sync::Mutex<HashMap<PathBuf, usize>>>,
545+
pub attempt_counters: Arc<std::sync::Mutex<HashMap<Box<Path>, usize>>>,
549546
pub load_delay: Duration,
550547
memory_reader: MemoryAssetReader,
551548
failure_count: usize,
@@ -589,13 +586,12 @@ mod tests {
589586
Result<Box<bevy_asset::io::Reader<'a>>, bevy_asset::io::AssetReaderError>,
590587
> {
591588
let attempt_number = {
592-
let key = PathBuf::from(path);
593589
let mut attempt_counters = self.attempt_counters.lock().unwrap();
594-
if let Some(existing) = attempt_counters.get_mut(&key) {
590+
if let Some(existing) = attempt_counters.get_mut(path) {
595591
*existing += 1;
596592
*existing
597593
} else {
598-
attempt_counters.insert(key, 1);
594+
attempt_counters.insert(path.into(), 1);
599595
1
600596
}
601597
};

crates/bevy_asset/src/processor/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ pub struct AssetProcessorData {
5656
log: async_lock::RwLock<Option<ProcessorTransactionLog>>,
5757
processors: RwLock<HashMap<&'static str, Arc<dyn ErasedProcessor>>>,
5858
/// Default processors for file extensions
59-
default_processors: RwLock<HashMap<String, &'static str>>,
59+
default_processors: RwLock<HashMap<Box<str>, &'static str>>,
6060
state: async_lock::RwLock<ProcessorState>,
6161
sources: AssetSources,
6262
initialized_sender: async_broadcast::Sender<()>,
@@ -482,7 +482,7 @@ impl AssetProcessor {
482482
/// Set the default processor for the given `extension`. Make sure `P` is registered with [`AssetProcessor::register_processor`].
483483
pub fn set_default_processor<P: Process>(&self, extension: &str) {
484484
let mut default_processors = self.data.default_processors.write();
485-
default_processors.insert(extension.to_string(), std::any::type_name::<P>());
485+
default_processors.insert(extension.into(), std::any::type_name::<P>());
486486
}
487487

488488
/// Returns the default processor for the given `extension`, if it exists.

crates/bevy_asset/src/server/info.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ pub(crate) struct AssetInfos {
7171
pub(crate) loader_dependants: HashMap<AssetPath<'static>, HashSet<AssetPath<'static>>>,
7272
/// Tracks living labeled assets for a given source asset.
7373
/// This should only be set when watching for changes to avoid unnecessary work.
74-
pub(crate) living_labeled_assets: HashMap<AssetPath<'static>, HashSet<String>>,
74+
pub(crate) living_labeled_assets: HashMap<AssetPath<'static>, HashSet<Box<str>>>,
7575
pub(crate) handle_providers: TypeIdMap<AssetHandleProvider>,
7676
pub(crate) dependency_loaded_event_sender: TypeIdMap<fn(&mut World, UntypedAssetId)>,
7777
pub(crate) dependency_failed_event_sender:
@@ -113,7 +113,7 @@ impl AssetInfos {
113113
fn create_handle_internal(
114114
infos: &mut HashMap<UntypedAssetId, AssetInfo>,
115115
handle_providers: &TypeIdMap<AssetHandleProvider>,
116-
living_labeled_assets: &mut HashMap<AssetPath<'static>, HashSet<String>>,
116+
living_labeled_assets: &mut HashMap<AssetPath<'static>, HashSet<Box<str>>>,
117117
watching_for_changes: bool,
118118
type_id: TypeId,
119119
path: Option<AssetPath<'static>>,
@@ -129,7 +129,7 @@ impl AssetInfos {
129129
let mut without_label = path.to_owned();
130130
if let Some(label) = without_label.take_label() {
131131
let labels = living_labeled_assets.entry(without_label).or_default();
132-
labels.insert(label.to_string());
132+
labels.insert(label.as_ref().into());
133133
}
134134
}
135135
}
@@ -613,7 +613,7 @@ impl AssetInfos {
613613
info: &AssetInfo,
614614
loader_dependants: &mut HashMap<AssetPath<'static>, HashSet<AssetPath<'static>>>,
615615
path: &AssetPath<'static>,
616-
living_labeled_assets: &mut HashMap<AssetPath<'static>, HashSet<String>>,
616+
living_labeled_assets: &mut HashMap<AssetPath<'static>, HashSet<Box<str>>>,
617617
) {
618618
for loader_dependency in info.loader_dependencies.keys() {
619619
if let Some(dependants) = loader_dependants.get_mut(loader_dependency) {
@@ -642,7 +642,7 @@ impl AssetInfos {
642642
infos: &mut HashMap<UntypedAssetId, AssetInfo>,
643643
path_to_id: &mut HashMap<AssetPath<'static>, TypeIdMap<UntypedAssetId>>,
644644
loader_dependants: &mut HashMap<AssetPath<'static>, HashSet<AssetPath<'static>>>,
645-
living_labeled_assets: &mut HashMap<AssetPath<'static>, HashSet<String>>,
645+
living_labeled_assets: &mut HashMap<AssetPath<'static>, HashSet<Box<str>>>,
646646
watching_for_changes: bool,
647647
id: UntypedAssetId,
648648
) -> bool {

crates/bevy_asset/src/server/loaders.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ use thiserror::Error;
1313
pub(crate) struct AssetLoaders {
1414
loaders: Vec<MaybeAssetLoader>,
1515
type_id_to_loaders: TypeIdMap<Vec<usize>>,
16-
extension_to_loaders: HashMap<String, Vec<usize>>,
16+
extension_to_loaders: HashMap<Box<str>, Vec<usize>>,
1717
type_name_to_loader: HashMap<&'static str, usize>,
1818
preregistered_loaders: HashMap<&'static str, usize>,
1919
}
@@ -44,7 +44,7 @@ impl AssetLoaders {
4444
for extension in loader.extensions() {
4545
let list = self
4646
.extension_to_loaders
47-
.entry(extension.to_string())
47+
.entry((*extension).into())
4848
.or_default();
4949

5050
if !list.is_empty() {
@@ -105,7 +105,7 @@ impl AssetLoaders {
105105
for extension in extensions {
106106
let list = self
107107
.extension_to_loaders
108-
.entry(extension.to_string())
108+
.entry((*extension).into())
109109
.or_default();
110110

111111
if !list.is_empty() {

crates/bevy_ecs/src/bundle.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -806,7 +806,7 @@ pub struct Bundles {
806806
/// Cache static [`BundleId`]
807807
bundle_ids: TypeIdMap<BundleId>,
808808
/// Cache dynamic [`BundleId`] with multiple components
809-
dynamic_bundle_ids: HashMap<Vec<ComponentId>, (BundleId, Vec<StorageType>)>,
809+
dynamic_bundle_ids: HashMap<Box<[ComponentId]>, (BundleId, Vec<StorageType>)>,
810810
/// Cache optimized dynamic [`BundleId`] with single component
811811
dynamic_component_bundle_ids: HashMap<ComponentId, (BundleId, StorageType)>,
812812
}
@@ -871,7 +871,7 @@ impl Bundles {
871871
.from_key(component_ids)
872872
.or_insert_with(|| {
873873
(
874-
Vec::from(component_ids),
874+
component_ids.into(),
875875
initialize_dynamic_bundle(bundle_infos, components, Vec::from(component_ids)),
876876
)
877877
});

crates/bevy_ecs/src/storage/table.rs

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -796,7 +796,7 @@ impl Table {
796796
/// Can be accessed via [`Storages`](crate::storage::Storages)
797797
pub struct Tables {
798798
tables: Vec<Table>,
799-
table_ids: HashMap<Vec<ComponentId>, TableId>,
799+
table_ids: HashMap<Box<[ComponentId]>, TableId>,
800800
}
801801

802802
impl Default for Tables {
@@ -872,10 +872,7 @@ impl Tables {
872872
table = table.add_column(components.get_info_unchecked(*component_id));
873873
}
874874
tables.push(table.build());
875-
(
876-
component_ids.to_vec(),
877-
TableId::from_usize(tables.len() - 1),
878-
)
875+
(component_ids.into(), TableId::from_usize(tables.len() - 1))
879876
});
880877

881878
*value

crates/bevy_gltf/src/lib.rs

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ use bevy_scene::Scene;
2626
/// Adds support for glTF file loading to the app.
2727
#[derive(Default)]
2828
pub struct GltfPlugin {
29-
custom_vertex_attributes: HashMap<String, MeshVertexAttribute>,
29+
custom_vertex_attributes: HashMap<Box<str>, MeshVertexAttribute>,
3030
}
3131

3232
impl GltfPlugin {
@@ -40,8 +40,7 @@ impl GltfPlugin {
4040
name: &str,
4141
attribute: MeshVertexAttribute,
4242
) -> Self {
43-
self.custom_vertex_attributes
44-
.insert(name.to_string(), attribute);
43+
self.custom_vertex_attributes.insert(name.into(), attribute);
4544
self
4645
}
4746
}
@@ -75,27 +74,27 @@ pub struct Gltf {
7574
/// All scenes loaded from the glTF file.
7675
pub scenes: Vec<Handle<Scene>>,
7776
/// Named scenes loaded from the glTF file.
78-
pub named_scenes: HashMap<String, Handle<Scene>>,
77+
pub named_scenes: HashMap<Box<str>, Handle<Scene>>,
7978
/// All meshes loaded from the glTF file.
8079
pub meshes: Vec<Handle<GltfMesh>>,
8180
/// Named meshes loaded from the glTF file.
82-
pub named_meshes: HashMap<String, Handle<GltfMesh>>,
81+
pub named_meshes: HashMap<Box<str>, Handle<GltfMesh>>,
8382
/// All materials loaded from the glTF file.
8483
pub materials: Vec<Handle<StandardMaterial>>,
8584
/// Named materials loaded from the glTF file.
86-
pub named_materials: HashMap<String, Handle<StandardMaterial>>,
85+
pub named_materials: HashMap<Box<str>, Handle<StandardMaterial>>,
8786
/// All nodes loaded from the glTF file.
8887
pub nodes: Vec<Handle<GltfNode>>,
8988
/// Named nodes loaded from the glTF file.
90-
pub named_nodes: HashMap<String, Handle<GltfNode>>,
89+
pub named_nodes: HashMap<Box<str>, Handle<GltfNode>>,
9190
/// Default scene to be displayed.
9291
pub default_scene: Option<Handle<Scene>>,
9392
/// All animations loaded from the glTF file.
9493
#[cfg(feature = "bevy_animation")]
9594
pub animations: Vec<Handle<AnimationClip>>,
9695
/// Named animations loaded from the glTF file.
9796
#[cfg(feature = "bevy_animation")]
98-
pub named_animations: HashMap<String, Handle<AnimationClip>>,
97+
pub named_animations: HashMap<Box<str>, Handle<AnimationClip>>,
9998
/// 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.
10099
pub source: Option<gltf::Gltf>,
101100
}

0 commit comments

Comments
 (0)