Skip to content

Commit b3a1db6

Browse files
committed
Proper prehashing (#3963)
For some keys, it is too expensive to hash them on every lookup. Historically in Bevy, we have regrettably done the "wrong" thing in these cases (pre-computing hashes, then re-hashing them) because Rust's built in hashed collections don't give us the tools we need to do otherwise. Doing this is "wrong" because two different values can result in the same hash. Hashed collections generally get around this by falling back to equality checks on hash collisions. You can't do that if the key _is_ the hash. Additionally, re-hashing a hash increase the odds of collision! #3959 needs pre-hashing to be viable, so I decided to finally properly solve the problem. The solution involves two different changes: 1. A new generalized "pre-hashing" solution in bevy_utils: `Hashed<T>` types, which store a value alongside a pre-computed hash. And `PreHashMap<K, V>` (which uses `Hashed<T>` internally) . `PreHashMap` is just an alias for a normal HashMap that uses `Hashed<T>` as the key and a new `PassHash` implementation as the Hasher. 2. Replacing the `std::collections` re-exports in `bevy_utils` with equivalent `hashbrown` impls. Avoiding re-hashes requires the `raw_entry_mut` api, which isn't stabilized yet (and may never be ... `entry_ref` has favor now, but also isn't available yet). If std's HashMap ever provides the tools we need, we can move back to that. The latest version of `hashbrown` adds support for the `entity_ref` api, so we can move to that in preparation for an std migration, if thats the direction they seem to be going in. Note that adding hashbrown doesn't increase our dependency count because it was already in our tree. In addition to providing these core tools, I also ported the "table identity hashing" in `bevy_ecs` to `raw_entry_mut`, which was a particularly egregious case. The biggest outstanding case is `AssetPathId`, which stores a pre-hash. We need AssetPathId to be cheaply clone-able (and ideally Copy), but `Hashed<AssetPath>` requires ownership of the AssetPath, which makes cloning ids way more expensive. We could consider doing `Hashed<Arc<AssetPath>>`, but cloning an arc is still a non-trivial expensive that needs to be considered. I would like to handle this in a separate PR. And given that we will be re-evaluating the Bevy Assets implementation in the very near future, I'd prefer to hold off until after that conversation is concluded.
1 parent c4f132a commit b3a1db6

File tree

12 files changed

+186
-181
lines changed

12 files changed

+186
-181
lines changed

crates/bevy_asset/src/asset_server.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,10 @@ use anyhow::Result;
88
use bevy_ecs::system::{Res, ResMut};
99
use bevy_log::warn;
1010
use bevy_tasks::TaskPool;
11-
use bevy_utils::{HashMap, Uuid};
11+
use bevy_utils::{Entry, HashMap, Uuid};
1212
use crossbeam_channel::TryRecvError;
1313
use parking_lot::{Mutex, RwLock};
14-
use std::{collections::hash_map::Entry, path::Path, sync::Arc};
14+
use std::{path::Path, sync::Arc};
1515
use thiserror::Error;
1616

1717
/// Errors that occur while loading assets with an `AssetServer`

crates/bevy_ecs/src/entity/map_entities.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
use crate::entity::Entity;
2-
use bevy_utils::HashMap;
3-
use std::collections::hash_map::Entry;
2+
use bevy_utils::{Entry, HashMap};
43
use thiserror::Error;
54

65
#[derive(Error, Debug)]

crates/bevy_ecs/src/storage/table.rs

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,9 @@ use crate::{
33
entity::Entity,
44
storage::{BlobVec, SparseSet},
55
};
6-
use bevy_utils::{AHasher, HashMap};
6+
use bevy_utils::HashMap;
77
use std::{
88
cell::UnsafeCell,
9-
hash::{Hash, Hasher},
109
ops::{Index, IndexMut},
1110
ptr::NonNull,
1211
};
@@ -415,7 +414,7 @@ impl Table {
415414
/// Can be accessed via [`Storages`](crate::storage::Storages)
416415
pub struct Tables {
417416
tables: Vec<Table>,
418-
table_ids: HashMap<u64, TableId>,
417+
table_ids: HashMap<Vec<ComponentId>, TableId>,
419418
}
420419

421420
impl Default for Tables {
@@ -472,18 +471,21 @@ impl Tables {
472471
component_ids: &[ComponentId],
473472
components: &Components,
474473
) -> TableId {
475-
let mut hasher = AHasher::default();
476-
component_ids.hash(&mut hasher);
477-
let hash = hasher.finish();
478474
let tables = &mut self.tables;
479-
*self.table_ids.entry(hash).or_insert_with(move || {
480-
let mut table = Table::with_capacity(0, component_ids.len());
481-
for component_id in component_ids.iter() {
482-
table.add_column(components.get_info_unchecked(*component_id));
483-
}
484-
tables.push(table);
485-
TableId(tables.len() - 1)
486-
})
475+
let (_key, value) = self
476+
.table_ids
477+
.raw_entry_mut()
478+
.from_key(component_ids)
479+
.or_insert_with(|| {
480+
let mut table = Table::with_capacity(0, component_ids.len());
481+
for component_id in component_ids.iter() {
482+
table.add_column(components.get_info_unchecked(*component_id));
483+
}
484+
tables.push(table);
485+
(component_ids.to_vec(), TableId(tables.len() - 1))
486+
});
487+
488+
*value
487489
}
488490

489491
pub fn iter(&self) -> std::slice::Iter<'_, Table> {

crates/bevy_input/src/input.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,13 @@ use bevy_ecs::schedule::State;
2929
/// * Call the [`Input::release`] method for each release event.
3030
/// * Call the [`Input::clear`] method at each frame start, before processing events.
3131
#[derive(Debug, Clone)]
32-
pub struct Input<T> {
32+
pub struct Input<T: Eq + Hash> {
3333
pressed: HashSet<T>,
3434
just_pressed: HashSet<T>,
3535
just_released: HashSet<T>,
3636
}
3737

38-
impl<T> Default for Input<T> {
38+
impl<T: Eq + Hash> Default for Input<T> {
3939
fn default() -> Self {
4040
Self {
4141
pressed: Default::default(),

crates/bevy_reflect/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ thiserror = "1.0"
2525
serde = "1"
2626
smallvec = { version = "1.6", features = ["serde", "union", "const_generics"], optional = true }
2727
glam = { version = "0.20.0", features = ["serde"], optional = true }
28+
hashbrown = { version = "0.11", features = ["serde"], optional = true }
2829

2930
[dev-dependencies]
3031
ron = "0.7.0"

crates/bevy_reflect/src/impls/std.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use crate::{
66
};
77

88
use bevy_reflect_derive::{impl_from_reflect_value, impl_reflect_value};
9-
use bevy_utils::{AHashExt, Duration, HashMap, HashSet};
9+
use bevy_utils::{Duration, HashMap, HashSet};
1010
use serde::{Deserialize, Serialize};
1111
use std::{
1212
any::Any,

crates/bevy_reflect/src/map.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
use std::{any::Any, collections::hash_map::Entry};
1+
use std::any::Any;
22

3-
use bevy_utils::HashMap;
3+
use bevy_utils::{Entry, HashMap};
44

55
use crate::{serde::Serializable, Reflect, ReflectMut, ReflectRef};
66

crates/bevy_reflect/src/struct_trait.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use crate::{serde::Serializable, Reflect, ReflectMut, ReflectRef};
2-
use bevy_utils::HashMap;
3-
use std::{any::Any, borrow::Cow, collections::hash_map::Entry};
2+
use bevy_utils::{Entry, HashMap};
3+
use std::{any::Any, borrow::Cow};
44

55
/// A reflected Rust regular struct type.
66
///

crates/bevy_render/src/render_resource/pipeline_cache.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@ use crate::{
1010
use bevy_app::EventReader;
1111
use bevy_asset::{AssetEvent, Assets, Handle};
1212
use bevy_ecs::system::{Res, ResMut};
13-
use bevy_utils::{tracing::error, HashMap, HashSet};
14-
use std::{collections::hash_map::Entry, hash::Hash, ops::Deref, sync::Arc};
13+
use bevy_utils::{tracing::error, Entry, HashMap, HashSet};
14+
use std::{hash::Hash, ops::Deref, sync::Arc};
1515
use thiserror::Error;
1616
use wgpu::{PipelineLayoutDescriptor, ShaderModule, VertexBufferLayout};
1717

crates/bevy_render/src/texture/texture_cache.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use crate::{
33
renderer::RenderDevice,
44
};
55
use bevy_ecs::prelude::ResMut;
6-
use bevy_utils::HashMap;
6+
use bevy_utils::{Entry, HashMap};
77
use wgpu::{TextureDescriptor, TextureViewDescriptor};
88

99
/// The internal representation of a [`CachedTexture`] used to track whether it was recently used
@@ -39,7 +39,7 @@ impl TextureCache {
3939
descriptor: TextureDescriptor<'static>,
4040
) -> CachedTexture {
4141
match self.textures.entry(descriptor) {
42-
std::collections::hash_map::Entry::Occupied(mut entry) => {
42+
Entry::Occupied(mut entry) => {
4343
for texture in entry.get_mut().iter_mut() {
4444
if !texture.taken {
4545
texture.frames_since_last_use = 0;
@@ -64,7 +64,7 @@ impl TextureCache {
6464
default_view,
6565
}
6666
}
67-
std::collections::hash_map::Entry::Vacant(entry) => {
67+
Entry::Vacant(entry) => {
6868
let texture = render_device.create_texture(entry.key());
6969
let default_view = texture.create_view(&TextureViewDescriptor::default());
7070
entry.insert(vec![CachedTextureMeta {

crates/bevy_utils/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ ahash = "0.7.0"
1414
tracing = {version = "0.1", features = ["release_max_level_info"]}
1515
instant = { version = "0.1", features = ["wasm-bindgen"] }
1616
uuid = { version = "0.8", features = ["v4", "serde"] }
17+
hashbrown = { version = "0.11", features = ["serde"] }
1718

1819
[target.'cfg(target_arch = "wasm32")'.dependencies]
1920
getrandom = {version = "0.2.0", features = ["js"]}

0 commit comments

Comments
 (0)