From 20ff10c3f86ef73bd4f54edf2a307b4c1c931e98 Mon Sep 17 00:00:00 2001 From: Marijn Suijten Date: Wed, 30 Nov 2022 17:45:49 +0100 Subject: [PATCH] Replace `backtrace` crate with stabilized `std::backtrace` implementation Rust 1.65 [stabilized `std::backtrace::Backtrace`], which we can use in place of the `backtrace` crate to reduce our dependency stack. This type has builtin support for disabling and enabling backtraces, making extra wrapping in an `Option<>` unnecessary. It is automatically configured using `RUST_BACKTRACE` or `RUST_LIB_BACKTRACE`, requiring us to call `Backtrace::force_backtrace()` to have full control over when we create a backtrace via our boolean feature flags. Unfortuantely the type no longer implements `Clone` like `backtrace::Backtrace`, requiring us to wrap it in an `Arc` (because most of our types are thread-safe) to clone the `Backtrace` around various (sub)allocations and statistics reports. [stabilized `std::backtrace::Backtrace`]: https://blog.rust-lang.org/2022/11/03/Rust-1.65.0.html#stabilized-apis --- .cargo/config.toml | 1 - Cargo.toml | 2 +- .../dedicated_block_allocator/mod.rs | 17 ++++++++------ src/allocator/free_list_allocator/mod.rs | 23 +++++++++++-------- src/allocator/mod.rs | 21 +++++------------ src/d3d12/mod.rs | 20 ++++++++-------- src/visualizer/allocation_reports.rs | 8 ++++--- src/visualizer/memory_chunks.rs | 13 ++++++----- src/vulkan/mod.rs | 23 +++++++++---------- 9 files changed, 64 insertions(+), 64 deletions(-) diff --git a/.cargo/config.toml b/.cargo/config.toml index b73da0e2..76831810 100644 --- a/.cargo/config.toml +++ b/.cargo/config.toml @@ -78,7 +78,6 @@ rustflags = [ # END - Embark standard lints v6 for Rust 1.55+ # Our additional lints - "-Wclippy::clone_on_ref_ptr", "-Wclippy::cognitive_complexity", "-Wclippy::needless_pass_by_value", "-Wclippy::option_if_let_else", diff --git a/Cargo.toml b/Cargo.toml index 26a4254b..462e14ba 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -10,6 +10,7 @@ homepage = "https://github.com/Traverse-Research/gpu-allocator" repository = "https://github.com/Traverse-Research/gpu-allocator" keywords = ["vulkan", "memory", "allocator"] documentation = "https://docs.rs/gpu-allocator/" +rust-version = "1.65" include = [ "/README.md", @@ -19,7 +20,6 @@ include = [ ] [dependencies] -backtrace = "0.3" log = "0.4" thiserror = "1.0" presser = { version = "0.3" } diff --git a/src/allocator/dedicated_block_allocator/mod.rs b/src/allocator/dedicated_block_allocator/mod.rs index e36fd579..79ec8ef8 100644 --- a/src/allocator/dedicated_block_allocator/mod.rs +++ b/src/allocator/dedicated_block_allocator/mod.rs @@ -3,16 +3,20 @@ #[cfg(feature = "visualizer")] pub(crate) mod visualizer; -use super::{resolve_backtrace, AllocationReport, AllocationType, SubAllocator, SubAllocatorBase}; -use crate::{AllocationError, Result}; +use std::{backtrace::Backtrace, sync::Arc}; + use log::{log, Level}; +use super::{AllocationReport, AllocationType, SubAllocator, SubAllocatorBase}; +use crate::{AllocationError, Result}; + #[derive(Debug)] pub(crate) struct DedicatedBlockAllocator { size: u64, allocated: u64, + /// Only used if [`crate::AllocatorDebugSettings::store_stack_traces`] is [`true`] name: Option, - backtrace: Option, + backtrace: Arc, } impl DedicatedBlockAllocator { @@ -21,7 +25,7 @@ impl DedicatedBlockAllocator { size, allocated: 0, name: None, - backtrace: None, + backtrace: Arc::new(Backtrace::disabled()), } } } @@ -35,7 +39,7 @@ impl SubAllocator for DedicatedBlockAllocator { _allocation_type: AllocationType, _granularity: u64, name: &str, - backtrace: Option, + backtrace: Arc, ) -> Result<(u64, std::num::NonZeroU64)> { if self.allocated != 0 { return Err(AllocationError::OutOfMemory); @@ -86,7 +90,6 @@ impl SubAllocator for DedicatedBlockAllocator { ) { let empty = "".to_string(); let name = self.name.as_ref().unwrap_or(&empty); - let backtrace = resolve_backtrace(&self.backtrace); log!( log_level, @@ -103,7 +106,7 @@ impl SubAllocator for DedicatedBlockAllocator { memory_block_index, self.size, name, - backtrace + self.backtrace ) } diff --git a/src/allocator/free_list_allocator/mod.rs b/src/allocator/free_list_allocator/mod.rs index 9b9973e7..33437c85 100644 --- a/src/allocator/free_list_allocator/mod.rs +++ b/src/allocator/free_list_allocator/mod.rs @@ -3,11 +3,16 @@ #[cfg(feature = "visualizer")] pub(crate) mod visualizer; -use super::{resolve_backtrace, AllocationReport, AllocationType, SubAllocator, SubAllocatorBase}; -use crate::{AllocationError, Result}; +use std::{ + backtrace::Backtrace, + collections::{HashMap, HashSet}, + sync::Arc, +}; use log::{log, Level}; -use std::collections::{HashMap, HashSet}; + +use super::{AllocationReport, AllocationType, SubAllocator, SubAllocatorBase}; +use crate::{AllocationError, Result}; const USE_BEST_FIT: bool = true; @@ -26,7 +31,8 @@ pub(crate) struct MemoryChunk { pub(crate) offset: u64, pub(crate) allocation_type: AllocationType, pub(crate) name: Option, - pub(crate) backtrace: Option, // Only used if STORE_STACK_TRACES is true + /// Only used if [`crate::AllocatorDebugSettings::store_stack_traces`] is [`true`] + pub(crate) backtrace: Arc, next: Option, prev: Option, } @@ -73,7 +79,7 @@ impl FreeListAllocator { offset: 0, allocation_type: AllocationType::Free, name: None, - backtrace: None, + backtrace: Arc::new(Backtrace::disabled()), prev: None, next: None, }, @@ -156,7 +162,7 @@ impl SubAllocator for FreeListAllocator { allocation_type: AllocationType, granularity: u64, name: &str, - backtrace: Option, + backtrace: Arc, ) -> Result<(u64, std::num::NonZeroU64)> { let free_size = self.size - self.allocated; if size > free_size { @@ -296,7 +302,7 @@ impl SubAllocator for FreeListAllocator { })?; chunk.allocation_type = AllocationType::Free; chunk.name = None; - chunk.backtrace = None; + chunk.backtrace = Arc::new(Backtrace::disabled()); self.allocated -= chunk.size; @@ -356,7 +362,6 @@ impl SubAllocator for FreeListAllocator { } let empty = "".to_string(); let name = chunk.name.as_ref().unwrap_or(&empty); - let backtrace = resolve_backtrace(&chunk.backtrace); log!( log_level, @@ -379,7 +384,7 @@ impl SubAllocator for FreeListAllocator { chunk.offset, chunk.allocation_type, name, - backtrace + chunk.backtrace ); } } diff --git a/src/allocator/mod.rs b/src/allocator/mod.rs index c251dc31..a84c047b 100644 --- a/src/allocator/mod.rs +++ b/src/allocator/mod.rs @@ -1,3 +1,7 @@ +use std::{backtrace::Backtrace, sync::Arc}; + +use log::*; + use crate::result::*; pub(crate) mod dedicated_block_allocator; @@ -6,8 +10,6 @@ pub(crate) use dedicated_block_allocator::DedicatedBlockAllocator; pub(crate) mod free_list_allocator; pub(crate) use free_list_allocator::FreeListAllocator; -use log::*; - #[derive(PartialEq, Copy, Clone, Debug)] #[repr(u8)] pub(crate) enum AllocationType { @@ -32,18 +34,7 @@ pub(crate) struct AllocationReport { pub(crate) name: String, pub(crate) size: u64, #[cfg(feature = "visualizer")] - pub(crate) backtrace: Option, -} - -pub(crate) fn resolve_backtrace(backtrace: &Option) -> String { - backtrace.as_ref().map_or_else( - || "".to_owned(), - |bt| { - let mut bt = bt.clone(); - bt.resolve(); - format!("{:?}", bt) - }, - ) + pub(crate) backtrace: Arc, } #[cfg(feature = "visualizer")] @@ -59,7 +50,7 @@ pub(crate) trait SubAllocator: SubAllocatorBase + std::fmt::Debug + Sync + Send allocation_type: AllocationType, granularity: u64, name: &str, - backtrace: Option, + backtrace: Arc, ) -> Result<(u64, std::num::NonZeroU64)>; fn free(&mut self, chunk_id: Option) -> Result<()>; diff --git a/src/d3d12/mod.rs b/src/d3d12/mod.rs index 3eecb8c5..d923330a 100644 --- a/src/d3d12/mod.rs +++ b/src/d3d12/mod.rs @@ -1,6 +1,6 @@ #![deny(clippy::unimplemented, clippy::unwrap_used, clippy::ok_expect)] -use std::fmt; +use std::{backtrace::Backtrace, fmt, sync::Arc}; use log::{debug, warn, Level}; @@ -421,7 +421,7 @@ impl MemoryType { &mut self, device: &ID3D12DeviceVersion, desc: &AllocationCreateDesc<'_>, - backtrace: Option, + backtrace: Arc, allocation_sizes: &AllocationSizes, ) -> Result { let allocation_type = AllocationType::Linear; @@ -717,11 +717,11 @@ impl Allocator { let size = desc.size; let alignment = desc.alignment; - let backtrace = if self.debug_settings.store_stack_traces { - Some(backtrace::Backtrace::new_unresolved()) + let backtrace = Arc::new(if self.debug_settings.store_stack_traces { + Backtrace::force_capture() } else { - None - }; + Backtrace::disabled() + }); if self.debug_settings.log_allocations { debug!( @@ -729,8 +729,8 @@ impl Allocator { &desc.name, size, alignment ); if self.debug_settings.log_stack_traces { - let backtrace = backtrace::Backtrace::new(); - debug!("Allocation stack trace: {:?}", &backtrace); + let backtrace = Backtrace::force_capture(); + debug!("Allocation stack trace: {}", backtrace); } } @@ -761,8 +761,8 @@ impl Allocator { let name = allocation.name.as_deref().unwrap_or(""); debug!("Freeing `{}`.", name); if self.debug_settings.log_stack_traces { - let backtrace = backtrace::Backtrace::new(); - debug!("Free stack trace: {:?}", backtrace); + let backtrace = Backtrace::force_capture(); + debug!("Free stack trace: {}", backtrace); } } diff --git a/src/visualizer/allocation_reports.rs b/src/visualizer/allocation_reports.rs index f741a9ce..a57f212b 100644 --- a/src/visualizer/allocation_reports.rs +++ b/src/visualizer/allocation_reports.rs @@ -1,7 +1,9 @@ +use std::backtrace::BacktraceStatus; + use egui::{Label, Response, Sense, Ui, WidgetText}; use egui_extras::{Column, TableBuilder}; -use crate::allocator::{fmt_bytes, resolve_backtrace, AllocationReport}; +use crate::allocator::{fmt_bytes, AllocationReport}; #[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] pub(crate) enum AllocationReportVisualizeSorting { @@ -121,9 +123,9 @@ pub(crate) fn render_allocation_reports_ui( ui.label(name); }); - if backtrace.is_some() { + if backtrace.status() == BacktraceStatus::Captured { resp.1.on_hover_ui(|ui| { - ui.label(resolve_backtrace(&backtrace)); + ui.label(backtrace.to_string()); }); } diff --git a/src/visualizer/memory_chunks.rs b/src/visualizer/memory_chunks.rs index 48ba97ba..3ef3ff22 100644 --- a/src/visualizer/memory_chunks.rs +++ b/src/visualizer/memory_chunks.rs @@ -1,6 +1,8 @@ +use std::backtrace::BacktraceStatus; + use egui::{Color32, DragValue, Rect, ScrollArea, Sense, Ui, Vec2}; -use crate::allocator::{free_list_allocator::MemoryChunk, resolve_backtrace}; +use crate::allocator::free_list_allocator::MemoryChunk; use super::ColorScheme; @@ -113,11 +115,10 @@ pub(crate) fn render_memory_chunks_ui<'a>( if let Some(name) = &chunk.name { ui.label(format!("name: {}", name)); } - if settings.show_backtraces && chunk.backtrace.is_some() { - ui.label(format!( - "backtrace: {}", - resolve_backtrace(&chunk.backtrace) - )); + if settings.show_backtraces + && chunk.backtrace.status() == BacktraceStatus::Captured + { + ui.label(chunk.backtrace.to_string()); } }); } diff --git a/src/vulkan/mod.rs b/src/vulkan/mod.rs index 56cc277b..39344b50 100644 --- a/src/vulkan/mod.rs +++ b/src/vulkan/mod.rs @@ -5,13 +5,12 @@ mod visualizer; #[cfg(feature = "visualizer")] pub use visualizer::AllocatorVisualizer; -use super::allocator; -use super::allocator::AllocationType; +use std::{backtrace::Backtrace, fmt, marker::PhantomData, sync::Arc}; + use ash::vk; -use core::marker::PhantomData; use log::{debug, Level}; -use std::fmt; +use super::allocator::{self, AllocationType}; use crate::{ allocator::fmt_bytes, AllocationError, AllocationSizes, AllocatorDebugSettings, MemoryLocation, Result, @@ -456,7 +455,7 @@ impl MemoryType { device: &ash::Device, desc: &AllocationCreateDesc<'_>, granularity: u64, - backtrace: Option, + backtrace: Arc, allocation_sizes: &AllocationSizes, ) -> Result { let allocation_type = if desc.linear { @@ -812,11 +811,11 @@ impl Allocator { let size = desc.requirements.size; let alignment = desc.requirements.alignment; - let backtrace = if self.debug_settings.store_stack_traces { - Some(backtrace::Backtrace::new_unresolved()) + let backtrace = Arc::new(if self.debug_settings.store_stack_traces { + Backtrace::force_capture() } else { - None - }; + Backtrace::disabled() + }); if self.debug_settings.log_allocations { debug!( @@ -824,8 +823,8 @@ impl Allocator { &desc.name, size, alignment ); if self.debug_settings.log_stack_traces { - let backtrace = backtrace::Backtrace::new(); - debug!("Allocation stack trace: {:?}", backtrace); + let backtrace = Backtrace::force_capture(); + debug!("Allocation stack trace: {}", backtrace); } } @@ -915,7 +914,7 @@ impl Allocator { let name = allocation.name.as_deref().unwrap_or(""); debug!("Freeing `{}`.", name); if self.debug_settings.log_stack_traces { - let backtrace = format!("{:?}", backtrace::Backtrace::new()); + let backtrace = Backtrace::force_capture(); debug!("Free stack trace: {}", backtrace); } }