From 999cfde14f5ef86cb1e4ad77740cb05a3f009da4 Mon Sep 17 00:00:00 2001 From: Tomasz Stachowiak Date: Mon, 8 Jul 2024 00:11:36 +0200 Subject: [PATCH] `rename_allocation` without mutable self --- Cargo.toml | 46 +++++++++++++------ .../dedicated_block_allocator/mod.rs | 27 +++++------ src/allocator/free_list_allocator/mod.rs | 34 +++++++------- src/allocator/mod.rs | 6 +-- src/d3d12/mod.rs | 20 ++++---- src/visualizer/memory_chunks.rs | 2 +- src/vulkan/mod.rs | 22 +++++---- 7 files changed, 84 insertions(+), 73 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index cf2ab9fb..3a3f0306 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -12,49 +12,65 @@ keywords = ["vulkan", "memory", "allocator"] documentation = "https://docs.rs/gpu-allocator/" rust-version = "1.69" -include = [ - "/README.md", - "/LICENSE-*", - "/src", - "/examples", -] +include = ["/README.md", "/LICENSE-*", "/src", "/examples"] [package.metadata.docs.rs] all-features = true [dependencies] +arc-swap = "1.7" log = "0.4" thiserror = "1.0" presser = { version = "0.3" } # Only needed for Vulkan. Disable all default features as good practice, # such as the ability to link/load a Vulkan library. -ash = { version = "0.38", optional = true, default-features = false, features = ["debug"] } +ash = { version = "0.38", optional = true, default-features = false, features = [ + "debug", +] } # Only needed for visualizer. egui = { version = ">=0.24, <=0.27", optional = true, default-features = false } egui_extras = { version = ">=0.24, <=0.27", optional = true, default-features = false } [target.'cfg(any(target_os = "macos", target_os = "ios"))'.dependencies] -metal = { version = "0.28.0", git = "https://github.com/gfx-rs/metal-rs", rev = "0d6214f", default-features = false, features = ["link", "dispatch"], optional = true } +metal = { version = "0.28.0", git = "https://github.com/gfx-rs/metal-rs", rev = "0d6214f", default-features = false, features = [ + "link", + "dispatch", +], optional = true } [target.'cfg(windows)'.dependencies] # Only needed for public-winapi interop helpers -winapi = { version = "0.3.9", features = ["d3d12", "winerror", "impl-default", "impl-debug"], optional = true } +winapi = { version = "0.3.9", features = [ + "d3d12", + "winerror", + "impl-default", + "impl-debug", +], optional = true } [target.'cfg(windows)'.dependencies.windows] version = ">=0.53,<=0.56" -features = [ - "Win32_Graphics_Direct3D12", - "Win32_Graphics_Dxgi_Common", -] +features = ["Win32_Graphics_Direct3D12", "Win32_Graphics_Dxgi_Common"] optional = true [dev-dependencies] # Enable the "loaded" feature to be able to access the Vulkan entrypoint. -ash = { version = "0.38", default-features = false, features = ["debug", "loaded"] } +ash = { version = "0.38", default-features = false, features = [ + "debug", + "loaded", +] } env_logger = "0.10" [target.'cfg(windows)'.dev-dependencies] -winapi = { version = "0.3.9", features = ["d3d12", "d3d12sdklayers", "dxgi1_6", "winerror", "impl-default", "impl-debug", "winuser", "windowsx", "libloaderapi"] } +winapi = { version = "0.3.9", features = [ + "d3d12", + "d3d12sdklayers", + "dxgi1_6", + "winerror", + "impl-default", + "impl-debug", + "winuser", + "windowsx", + "libloaderapi", +] } [target.'cfg(windows)'.dev-dependencies.windows] version = ">=0.53,<=0.56" diff --git a/src/allocator/dedicated_block_allocator/mod.rs b/src/allocator/dedicated_block_allocator/mod.rs index 746b4dcd..feeeb120 100644 --- a/src/allocator/dedicated_block_allocator/mod.rs +++ b/src/allocator/dedicated_block_allocator/mod.rs @@ -5,6 +5,7 @@ pub(crate) mod visualizer; use std::{backtrace::Backtrace, sync::Arc}; +use arc_swap::ArcSwapOption; use log::{log, Level}; use super::{AllocationReport, AllocationType, SubAllocator, SubAllocatorBase}; @@ -15,7 +16,7 @@ pub(crate) struct DedicatedBlockAllocator { size: u64, allocated: u64, /// Only used if [`crate::AllocatorDebugSettings::store_stack_traces`] is [`true`] - name: Option, + name: ArcSwapOption, backtrace: Arc, } @@ -24,7 +25,7 @@ impl DedicatedBlockAllocator { Self { size, allocated: 0, - name: None, + name: ArcSwapOption::empty(), backtrace: Arc::new(Backtrace::disabled()), } } @@ -52,7 +53,7 @@ impl SubAllocator for DedicatedBlockAllocator { } self.allocated = size; - self.name = Some(name.to_string()); + self.name.swap(Some(Arc::new(name.to_string()))); self.backtrace = backtrace; #[allow(clippy::unwrap_used)] @@ -69,15 +70,11 @@ impl SubAllocator for DedicatedBlockAllocator { } } - fn rename_allocation( - &mut self, - chunk_id: Option, - name: &str, - ) -> Result<()> { + fn rename_allocation(&self, chunk_id: Option, name: &str) -> Result<()> { if chunk_id != std::num::NonZeroU64::new(1) { Err(AllocationError::Internal("Chunk ID must be 1.".into())) } else { - self.name = Some(name.into()); + self.name.swap(Some(Arc::new(name.into()))); Ok(()) } } @@ -88,8 +85,8 @@ impl SubAllocator for DedicatedBlockAllocator { memory_type_index: usize, memory_block_index: usize, ) { - let empty = "".to_string(); - let name = self.name.as_ref().unwrap_or(&empty); + let name = self.name.load(); + let name = (*name).as_ref().map_or("", |name| name); log!( log_level, @@ -112,10 +109,10 @@ impl SubAllocator for DedicatedBlockAllocator { fn report_allocations(&self) -> Vec { vec![AllocationReport { - name: self - .name - .clone() - .unwrap_or_else(|| "".to_owned()), + name: self.name.load().as_ref().map_or_else( + || "".to_owned(), + |s: &Arc| (**s).clone(), + ), offset: 0, size: self.size, #[cfg(feature = "visualizer")] diff --git a/src/allocator/free_list_allocator/mod.rs b/src/allocator/free_list_allocator/mod.rs index d7cde2e8..fa238118 100644 --- a/src/allocator/free_list_allocator/mod.rs +++ b/src/allocator/free_list_allocator/mod.rs @@ -9,6 +9,7 @@ use std::{ sync::Arc, }; +use arc_swap::ArcSwapOption; use log::{log, Level}; use super::{AllocationReport, AllocationType, SubAllocator, SubAllocatorBase}; @@ -30,7 +31,7 @@ pub(crate) struct MemoryChunk { pub(crate) size: u64, pub(crate) offset: u64, pub(crate) allocation_type: AllocationType, - pub(crate) name: Option, + pub(crate) name: ArcSwapOption, /// Only used if [`crate::AllocatorDebugSettings::store_stack_traces`] is [`true`] pub(crate) backtrace: Arc, next: Option, @@ -78,7 +79,7 @@ impl FreeListAllocator { size, offset: 0, allocation_type: AllocationType::Free, - name: None, + name: ArcSwapOption::empty(), backtrace: Arc::new(Backtrace::disabled()), prev: None, next: None, @@ -248,7 +249,7 @@ impl SubAllocator for FreeListAllocator { size: best_aligned_size, offset: free_chunk.offset, allocation_type, - name: Some(name.to_string()), + name: ArcSwapOption::from_pointee(name.to_string()), backtrace, prev: free_chunk.prev, next: Some(first_fit_id), @@ -277,7 +278,7 @@ impl SubAllocator for FreeListAllocator { .ok_or_else(|| AllocationError::Internal("Invalid chunk reference.".into()))?; chunk.allocation_type = allocation_type; - chunk.name = Some(name.to_string()); + chunk.name.swap(Some(Arc::new(name.to_string()))); chunk.backtrace = backtrace; self.remove_id_from_free_list(first_fit_id); @@ -301,7 +302,7 @@ impl SubAllocator for FreeListAllocator { ) })?; chunk.allocation_type = AllocationType::Free; - chunk.name = None; + chunk.name.swap(None); chunk.backtrace = Arc::new(Backtrace::disabled()); self.allocated -= chunk.size; @@ -325,15 +326,11 @@ impl SubAllocator for FreeListAllocator { Ok(()) } - fn rename_allocation( - &mut self, - chunk_id: Option, - name: &str, - ) -> Result<()> { + fn rename_allocation(&self, chunk_id: Option, name: &str) -> Result<()> { let chunk_id = chunk_id .ok_or_else(|| AllocationError::Internal("Chunk ID must be a valid value.".into()))?; - let chunk = self.chunks.get_mut(&chunk_id).ok_or_else(|| { + let chunk = self.chunks.get(&chunk_id).ok_or_else(|| { AllocationError::Internal( "Attempting to rename chunk that is not in chunk list.".into(), ) @@ -345,7 +342,7 @@ impl SubAllocator for FreeListAllocator { )); } - chunk.name = Some(name.into()); + chunk.name.swap(Some(Arc::new(name.into()))); Ok(()) } @@ -360,8 +357,9 @@ impl SubAllocator for FreeListAllocator { if chunk.allocation_type == AllocationType::Free { continue; } - let empty = "".to_string(); - let name = chunk.name.as_ref().unwrap_or(&empty); + + let name = chunk.name.load(); + let name = (*name).as_ref().map_or("", |name| name); log!( log_level, @@ -394,10 +392,10 @@ impl SubAllocator for FreeListAllocator { .iter() .filter(|(_key, chunk)| chunk.allocation_type != AllocationType::Free) .map(|(_key, chunk)| AllocationReport { - name: chunk - .name - .clone() - .unwrap_or_else(|| "".to_owned()), + name: chunk.name.load().as_ref().map_or_else( + || "".to_owned(), + |s: &Arc| (**s).clone(), + ), offset: chunk.offset, size: chunk.size, #[cfg(feature = "visualizer")] diff --git a/src/allocator/mod.rs b/src/allocator/mod.rs index 5d40bbf6..2fc9404c 100644 --- a/src/allocator/mod.rs +++ b/src/allocator/mod.rs @@ -118,11 +118,7 @@ pub(crate) trait SubAllocator: SubAllocatorBase + fmt::Debug + Sync + Send { fn free(&mut self, chunk_id: Option) -> Result<()>; - fn rename_allocation( - &mut self, - chunk_id: Option, - name: &str, - ) -> Result<()>; + fn rename_allocation(&self, chunk_id: Option, name: &str) -> Result<()>; fn report_memory_leaks( &self, diff --git a/src/d3d12/mod.rs b/src/d3d12/mod.rs index 6236fd90..0e9ea363 100644 --- a/src/d3d12/mod.rs +++ b/src/d3d12/mod.rs @@ -1,5 +1,6 @@ use std::{backtrace::Backtrace, fmt, sync::Arc}; +use arc_swap::ArcSwapOption; use log::{debug, warn, Level}; use windows::Win32::{ Foundation::E_OUTOFMEMORY, @@ -320,7 +321,7 @@ pub struct Allocation { memory_type_index: usize, heap: ID3D12Heap, - name: Option>, + name: ArcSwapOption, } impl Allocation { @@ -485,7 +486,7 @@ impl MemoryType { memory_block_index: block_index, memory_type_index: self.memory_type_index, heap: mem_block.heap.clone(), - name: Some(desc.name.into()), + name: ArcSwapOption::from_pointee(desc.name.to_owned()), }); } @@ -510,7 +511,7 @@ impl MemoryType { memory_block_index: mem_block_i, memory_type_index: self.memory_type_index, heap: mem_block.heap.clone(), - name: Some(desc.name.into()), + name: ArcSwapOption::from_pointee(desc.name.to_owned()), }); } Err(AllocationError::OutOfMemory) => {} // Block is full, continue search. @@ -564,7 +565,7 @@ impl MemoryType { memory_block_index: new_block_index, memory_type_index: self.memory_type_index, heap: mem_block.heap.clone(), - name: Some(desc.name.into()), + name: ArcSwapOption::from_pointee(desc.name.to_owned()), }) } @@ -766,7 +767,8 @@ impl Allocator { pub fn free(&mut self, allocation: Allocation) -> Result<()> { if self.debug_settings.log_frees { - let name = allocation.name.as_deref().unwrap_or(""); + let name = allocation.name.load(); + let name = name.as_deref().map_or("", |name| name); debug!("Freeing `{}`.", name); if self.debug_settings.log_stack_traces { let backtrace = Backtrace::force_capture(); @@ -783,16 +785,16 @@ impl Allocator { Ok(()) } - pub fn rename_allocation(&mut self, allocation: &mut Allocation, name: &str) -> Result<()> { - allocation.name = Some(name.into()); + pub fn rename_allocation(&self, allocation: &Allocation, name: &str) -> Result<()> { + allocation.name.swap(Some(Arc::new(name.into()))); if allocation.is_null() { return Ok(()); } - let mem_type = &mut self.memory_types[allocation.memory_type_index]; + let mem_type = &self.memory_types[allocation.memory_type_index]; let mem_block = mem_type.memory_blocks[allocation.memory_block_index] - .as_mut() + .as_ref() .ok_or_else(|| AllocationError::Internal("Memory block must be Some.".into()))?; mem_block diff --git a/src/visualizer/memory_chunks.rs b/src/visualizer/memory_chunks.rs index ffe2afed..1f450a8a 100644 --- a/src/visualizer/memory_chunks.rs +++ b/src/visualizer/memory_chunks.rs @@ -111,7 +111,7 @@ pub(crate) fn render_memory_chunks_ui<'a>( "allocation_type: {}", chunk.allocation_type.as_str() )); - if let Some(name) = &chunk.name { + if let Some(name) = &*chunk.name.load() { ui.label(format!("name: {}", name)); } if settings.show_backtraces diff --git a/src/vulkan/mod.rs b/src/vulkan/mod.rs index 560d7cbf..73a43959 100644 --- a/src/vulkan/mod.rs +++ b/src/vulkan/mod.rs @@ -4,6 +4,7 @@ mod visualizer; use std::{backtrace::Backtrace, fmt, marker::PhantomData, sync::Arc}; +use arc_swap::ArcSwapOption; use ash::vk; use log::{debug, Level}; #[cfg(feature = "visualizer")] @@ -158,7 +159,7 @@ pub struct Allocation { mapped_ptr: Option, dedicated_allocation: bool, memory_properties: vk::MemoryPropertyFlags, - name: Option>, + name: ArcSwapOption, } impl Allocation { @@ -273,7 +274,7 @@ impl Default for Allocation { device_memory: vk::DeviceMemory::null(), mapped_ptr: None, memory_properties: vk::MemoryPropertyFlags::empty(), - name: None, + name: ArcSwapOption::empty(), dedicated_allocation: false, } } @@ -531,7 +532,7 @@ impl MemoryType { device_memory: mem_block.device_memory, mapped_ptr: mem_block.mapped_ptr, memory_properties: self.memory_properties, - name: Some(desc.name.into()), + name: ArcSwapOption::from_pointee(desc.name.to_owned()), dedicated_allocation, }); } @@ -567,7 +568,7 @@ impl MemoryType { memory_properties: self.memory_properties, mapped_ptr, dedicated_allocation: false, - name: Some(desc.name.into()), + name: ArcSwapOption::from_pointee(desc.name.to_owned()), }); } Err(err) => match err { @@ -640,7 +641,7 @@ impl MemoryType { device_memory: mem_block.device_memory, mapped_ptr, memory_properties: self.memory_properties, - name: Some(desc.name.into()), + name: ArcSwapOption::from_pointee(desc.name.to_owned()), dedicated_allocation: false, }) } @@ -870,7 +871,8 @@ impl Allocator { pub fn free(&mut self, allocation: Allocation) -> Result<()> { if self.debug_settings.log_frees { - let name = allocation.name.as_deref().unwrap_or(""); + let name = allocation.name.load(); + let name = name.as_deref().map_or("", |name| name); debug!("Freeing `{}`.", name); if self.debug_settings.log_stack_traces { let backtrace = Backtrace::force_capture(); @@ -887,16 +889,16 @@ impl Allocator { Ok(()) } - pub fn rename_allocation(&mut self, allocation: &mut Allocation, name: &str) -> Result<()> { - allocation.name = Some(name.into()); + pub fn rename_allocation(&self, allocation: &Allocation, name: &str) -> Result<()> { + allocation.name.swap(Some(Arc::new(name.into()))); if allocation.is_null() { return Ok(()); } - let mem_type = &mut self.memory_types[allocation.memory_type_index]; + let mem_type = &self.memory_types[allocation.memory_type_index]; let mem_block = mem_type.memory_blocks[allocation.memory_block_index] - .as_mut() + .as_ref() .ok_or_else(|| AllocationError::Internal("Memory block must be Some.".into()))?; mem_block