From 0fac8eadfc65882e4bc23964a6096f22d57f6a8d Mon Sep 17 00:00:00 2001 From: Markus Ineichen Date: Mon, 6 Jan 2025 10:03:33 +0100 Subject: [PATCH] Avoid allocations for loader cache lookup (#5584) [ x ] I have ~~followed~~ _read_ the instructions in the PR template Unfortunately i had several issues: - Some snapshot-tests didn't run successfully on osx. diff shows errors around fonts or missing menu items) - cargo clippy doesn't run successfully (egui_kittest cannot find `wgpu` and `image`) - ./scripts/check.sh had other issues on my system (env: python: No such file or directory), even if python3 can be called via python in my shell Is there a system independent, standard way to run these tools (e.g. via Docker?) I submit the pr anyway, because there changes are very simple and shouldn't cause issues. --- crates/egui/src/load/texture_loader.rs | 8 +++++--- crates/egui_extras/src/loaders/svg_loader.rs | 12 +++++------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/crates/egui/src/load/texture_loader.rs b/crates/egui/src/load/texture_loader.rs index 6845e86ff82..a1170257cad 100644 --- a/crates/egui/src/load/texture_loader.rs +++ b/crates/egui/src/load/texture_loader.rs @@ -1,3 +1,5 @@ +use std::borrow::Cow; + use super::{ BytesLoader, Context, HashMap, ImagePoll, Mutex, SizeHint, SizedTexture, TextureHandle, TextureLoadResult, TextureLoader, TextureOptions, TexturePoll, @@ -5,7 +7,7 @@ use super::{ #[derive(Default)] pub struct DefaultTextureLoader { - cache: Mutex>, + cache: Mutex, TextureOptions), TextureHandle>>, } impl TextureLoader for DefaultTextureLoader { @@ -21,7 +23,7 @@ impl TextureLoader for DefaultTextureLoader { size_hint: SizeHint, ) -> TextureLoadResult { let mut cache = self.cache.lock(); - if let Some(handle) = cache.get(&(uri.into(), texture_options)) { + if let Some(handle) = cache.get(&(Cow::Borrowed(uri), texture_options)) { let texture = SizedTexture::from_handle(handle); Ok(TexturePoll::Ready { texture }) } else { @@ -30,7 +32,7 @@ impl TextureLoader for DefaultTextureLoader { ImagePoll::Ready { image } => { let handle = ctx.load_texture(uri, image, texture_options); let texture = SizedTexture::from_handle(&handle); - cache.insert((uri.into(), texture_options), handle); + cache.insert((Cow::Owned(uri.to_owned()), texture_options), handle); let reduce_texture_memory = ctx.options(|o| o.reduce_texture_memory); if reduce_texture_memory { let loaders = ctx.loaders(); diff --git a/crates/egui_extras/src/loaders/svg_loader.rs b/crates/egui_extras/src/loaders/svg_loader.rs index 67b59540c69..2a3f155695d 100644 --- a/crates/egui_extras/src/loaders/svg_loader.rs +++ b/crates/egui_extras/src/loaders/svg_loader.rs @@ -1,4 +1,4 @@ -use std::{mem::size_of, path::Path, sync::Arc}; +use std::{borrow::Cow, mem::size_of, path::Path, sync::Arc}; use ahash::HashMap; @@ -12,7 +12,7 @@ type Entry = Result, String>; #[derive(Default)] pub struct SvgLoader { - cache: Mutex>, + cache: Mutex, SizeHint), Entry>>, } impl SvgLoader { @@ -37,23 +37,21 @@ impl ImageLoader for SvgLoader { return Err(LoadError::NotSupported); } - let uri = uri.to_owned(); - let mut cache = self.cache.lock(); // We can't avoid the `uri` clone here without unsafe code. - if let Some(entry) = cache.get(&(uri.clone(), size_hint)).cloned() { + if let Some(entry) = cache.get(&(Cow::Borrowed(uri), size_hint)).cloned() { match entry { Ok(image) => Ok(ImagePoll::Ready { image }), Err(err) => Err(LoadError::Loading(err)), } } else { - match ctx.try_load_bytes(&uri) { + match ctx.try_load_bytes(uri) { Ok(BytesPoll::Ready { bytes, .. }) => { log::trace!("started loading {uri:?}"); let result = crate::image::load_svg_bytes_with_size(&bytes, Some(size_hint)) .map(Arc::new); log::trace!("finished loading {uri:?}"); - cache.insert((uri, size_hint), result.clone()); + cache.insert((Cow::Owned(uri.to_owned()), size_hint), result.clone()); match result { Ok(image) => Ok(ImagePoll::Ready { image }), Err(err) => Err(LoadError::Loading(err)),