Minor speed up for tiles (#432)

We currently do lots of redundant hash calculations while changing a
tile in vram. We can cache the value and then use the entry API to reuse
it.

- [x] no changelog update needed
This commit is contained in:
Gwilym Inzani 2023-05-23 21:41:25 +01:00 committed by GitHub
commit 98484783b4
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 24 additions and 28 deletions

View file

@ -338,23 +338,12 @@ where
} }
} }
fn insert_and_get(&mut self, key: K, value: V) -> &'_ mut V { unsafe fn insert_new_and_get(&mut self, key: K, value: V, hash: HashType) -> &'_ mut V {
let hash = self.hash(&key); if self.nodes.capacity() <= self.len() {
self.resize(self.nodes.backing_vec_size() * 2);
}
let location = if let Some(location) = self.nodes.location(&key, hash) { let location = self.nodes.insert_new(key, value, hash);
// SAFETY: location is valid due to the above
unsafe {
self.nodes
.replace_at_location_unchecked(location, key, value);
}
location
} else {
if self.nodes.capacity() <= self.len() {
self.resize(self.nodes.backing_vec_size() * 2);
}
self.nodes.insert_new(key, value, hash)
};
// SAFETY: location is always valid // SAFETY: location is always valid
unsafe { unsafe {
@ -599,7 +588,7 @@ impl<K, V, ALLOCATOR: ClonableAllocator> IntoIterator for HashMap<K, V, ALLOCATO
mod entries { mod entries {
use core::{alloc::Allocator, hash::Hash}; use core::{alloc::Allocator, hash::Hash};
use super::{ClonableAllocator, HashMap}; use super::{ClonableAllocator, HashMap, HashType};
/// A view into an occupied entry in a `HashMap`. This is part of the [`crate::Entry`] enum. /// A view into an occupied entry in a `HashMap`. This is part of the [`crate::Entry`] enum.
pub struct OccupiedEntry<'a, K: 'a, V: 'a, ALLOCATOR: Allocator> { pub struct OccupiedEntry<'a, K: 'a, V: 'a, ALLOCATOR: Allocator> {
@ -695,11 +684,16 @@ mod entries {
pub struct VacantEntry<'a, K: 'a, V: 'a, ALLOCATOR: Allocator> { pub struct VacantEntry<'a, K: 'a, V: 'a, ALLOCATOR: Allocator> {
key: K, key: K,
map: &'a mut HashMap<K, V, ALLOCATOR>, map: &'a mut HashMap<K, V, ALLOCATOR>,
hash: HashType,
} }
impl<'a, K: 'a, V: 'a, ALLOCATOR: ClonableAllocator> VacantEntry<'a, K, V, ALLOCATOR> { impl<'a, K: 'a, V: 'a, ALLOCATOR: ClonableAllocator> VacantEntry<'a, K, V, ALLOCATOR> {
pub(crate) fn new(key: K, map: &'a mut HashMap<K, V, ALLOCATOR>) -> Self { pub(crate) unsafe fn new(
Self { key, map } key: K,
hash: HashType,
map: &'a mut HashMap<K, V, ALLOCATOR>,
) -> Self {
Self { key, map, hash }
} }
/// Gets a reference to the key that would be used when inserting a value through `VacantEntry` /// Gets a reference to the key that would be used when inserting a value through `VacantEntry`
@ -717,7 +711,8 @@ mod entries {
where where
K: Hash + Eq, K: Hash + Eq,
{ {
self.map.insert_and_get(self.key, value) // SAFETY: by construction, this doesn't already exist in the hashmap and we were given the hash and key
unsafe { self.map.insert_new_and_get(self.key, value, self.hash) }
} }
} }
} }
@ -832,7 +827,10 @@ where
unsafe { OccupiedEntry::new(key, self, location) }, unsafe { OccupiedEntry::new(key, self, location) },
) )
} else { } else {
Entry::Vacant(VacantEntry::new(key, self)) Entry::Vacant(
// SAFETY: item doesn't exist yet and the hash is correct here
unsafe { VacantEntry::new(key, hash, self) },
)
} }
} }
} }

View file

@ -7,7 +7,7 @@ use crate::{
agb_alloc::{block_allocator::BlockAllocator, bump_allocator::StartEnd}, agb_alloc::{block_allocator::BlockAllocator, bump_allocator::StartEnd},
display::palette16, display::palette16,
dma::dma_copy16, dma::dma_copy16,
hash_map::HashMap, hash_map::{Entry, HashMap},
memory_mapped::MemoryMapped1DArray, memory_mapped::MemoryMapped1DArray,
}; };
@ -291,10 +291,10 @@ impl VRamManager {
pub(crate) fn add_tile(&mut self, tile_set: &TileSet<'_>, tile: u16) -> TileIndex { pub(crate) fn add_tile(&mut self, tile_set: &TileSet<'_>, tile: u16) -> TileIndex {
let reference = self let reference = self
.tile_set_to_vram .tile_set_to_vram
.get(&TileInTileSetReference::new(tile_set, tile)); .entry(TileInTileSetReference::new(tile_set, tile));
if let Some(reference) = reference { if let Entry::Occupied(reference) = reference {
let tile_index = Self::index_from_reference(*reference, tile_set.format); let tile_index = Self::index_from_reference(*reference.get(), tile_set.format);
let key = tile_index.refcount_key(); let key = tile_index.refcount_key();
self.reference_counts[key].increment_reference_count(); self.reference_counts[key].increment_reference_count();
return tile_index; return tile_index;
@ -305,15 +305,13 @@ impl VRamManager {
.unwrap() .unwrap()
.cast(); .cast();
let tile_reference = TileReference(new_reference); let tile_reference = TileReference(new_reference);
reference.or_insert(tile_reference);
self.copy_tile_to_location(tile_set, tile, tile_reference); self.copy_tile_to_location(tile_set, tile, tile_reference);
let index = Self::index_from_reference(tile_reference, tile_set.format); let index = Self::index_from_reference(tile_reference, tile_set.format);
let key = index.refcount_key(); let key = index.refcount_key();
self.tile_set_to_vram
.insert(TileInTileSetReference::new(tile_set, tile), tile_reference);
self.reference_counts self.reference_counts
.resize(self.reference_counts.len().max(key + 1), Default::default()); .resize(self.reference_counts.len().max(key + 1), Default::default());