Skip to content

Commit

Permalink
Replace backtrace crate with stabilized std::backtrace implementa…
Browse files Browse the repository at this point in the history
…tion

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
  • Loading branch information
MarijnS95 committed Nov 2, 2023
1 parent 22aa817 commit 20ff10c
Show file tree
Hide file tree
Showing 9 changed files with 64 additions and 64 deletions.
1 change: 0 additions & 1 deletion .cargo/config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -19,7 +20,6 @@ include = [
]

[dependencies]
backtrace = "0.3"
log = "0.4"
thiserror = "1.0"
presser = { version = "0.3" }
Expand Down
17 changes: 10 additions & 7 deletions src/allocator/dedicated_block_allocator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<String>,
backtrace: Option<backtrace::Backtrace>,
backtrace: Arc<Backtrace>,
}

impl DedicatedBlockAllocator {
Expand All @@ -21,7 +25,7 @@ impl DedicatedBlockAllocator {
size,
allocated: 0,
name: None,
backtrace: None,
backtrace: Arc::new(Backtrace::disabled()),
}
}
}
Expand All @@ -35,7 +39,7 @@ impl SubAllocator for DedicatedBlockAllocator {
_allocation_type: AllocationType,
_granularity: u64,
name: &str,
backtrace: Option<backtrace::Backtrace>,
backtrace: Arc<Backtrace>,
) -> Result<(u64, std::num::NonZeroU64)> {
if self.allocated != 0 {
return Err(AllocationError::OutOfMemory);
Expand Down Expand Up @@ -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,
Expand All @@ -103,7 +106,7 @@ impl SubAllocator for DedicatedBlockAllocator {
memory_block_index,
self.size,
name,
backtrace
self.backtrace
)
}

Expand Down
23 changes: 14 additions & 9 deletions src/allocator/free_list_allocator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -26,7 +31,8 @@ pub(crate) struct MemoryChunk {
pub(crate) offset: u64,
pub(crate) allocation_type: AllocationType,
pub(crate) name: Option<String>,
pub(crate) backtrace: Option<backtrace::Backtrace>, // Only used if STORE_STACK_TRACES is true
/// Only used if [`crate::AllocatorDebugSettings::store_stack_traces`] is [`true`]
pub(crate) backtrace: Arc<Backtrace>,
next: Option<std::num::NonZeroU64>,
prev: Option<std::num::NonZeroU64>,
}
Expand Down Expand Up @@ -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,
},
Expand Down Expand Up @@ -156,7 +162,7 @@ impl SubAllocator for FreeListAllocator {
allocation_type: AllocationType,
granularity: u64,
name: &str,
backtrace: Option<backtrace::Backtrace>,
backtrace: Arc<Backtrace>,
) -> Result<(u64, std::num::NonZeroU64)> {
let free_size = self.size - self.allocated;
if size > free_size {
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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,
Expand All @@ -379,7 +384,7 @@ impl SubAllocator for FreeListAllocator {
chunk.offset,
chunk.allocation_type,
name,
backtrace
chunk.backtrace
);
}
}
Expand Down
21 changes: 6 additions & 15 deletions src/allocator/mod.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
use std::{backtrace::Backtrace, sync::Arc};

use log::*;

use crate::result::*;

pub(crate) mod dedicated_block_allocator;
Expand All @@ -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 {
Expand All @@ -32,18 +34,7 @@ pub(crate) struct AllocationReport {
pub(crate) name: String,
pub(crate) size: u64,
#[cfg(feature = "visualizer")]
pub(crate) backtrace: Option<backtrace::Backtrace>,
}

pub(crate) fn resolve_backtrace(backtrace: &Option<backtrace::Backtrace>) -> String {
backtrace.as_ref().map_or_else(
|| "".to_owned(),
|bt| {
let mut bt = bt.clone();
bt.resolve();
format!("{:?}", bt)
},
)
pub(crate) backtrace: Arc<Backtrace>,
}

#[cfg(feature = "visualizer")]
Expand All @@ -59,7 +50,7 @@ pub(crate) trait SubAllocator: SubAllocatorBase + std::fmt::Debug + Sync + Send
allocation_type: AllocationType,
granularity: u64,
name: &str,
backtrace: Option<backtrace::Backtrace>,
backtrace: Arc<Backtrace>,
) -> Result<(u64, std::num::NonZeroU64)>;

fn free(&mut self, chunk_id: Option<std::num::NonZeroU64>) -> Result<()>;
Expand Down
20 changes: 10 additions & 10 deletions src/d3d12/mod.rs
Original file line number Diff line number Diff line change
@@ -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};

Expand Down Expand Up @@ -421,7 +421,7 @@ impl MemoryType {
&mut self,
device: &ID3D12DeviceVersion,
desc: &AllocationCreateDesc<'_>,
backtrace: Option<backtrace::Backtrace>,
backtrace: Arc<Backtrace>,
allocation_sizes: &AllocationSizes,
) -> Result<Allocation> {
let allocation_type = AllocationType::Linear;
Expand Down Expand Up @@ -717,20 +717,20 @@ 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!(
"Allocating `{}` of {} bytes with an alignment of {}.",
&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);
}
}

Expand Down Expand Up @@ -761,8 +761,8 @@ impl Allocator {
let name = allocation.name.as_deref().unwrap_or("<null>");
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);
}
}

Expand Down
8 changes: 5 additions & 3 deletions src/visualizer/allocation_reports.rs
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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());
});
}

Expand Down
13 changes: 7 additions & 6 deletions src/visualizer/memory_chunks.rs
Original file line number Diff line number Diff line change
@@ -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;

Expand Down Expand Up @@ -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());
}
});
}
Expand Down
23 changes: 11 additions & 12 deletions src/vulkan/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -456,7 +455,7 @@ impl MemoryType {
device: &ash::Device,
desc: &AllocationCreateDesc<'_>,
granularity: u64,
backtrace: Option<backtrace::Backtrace>,
backtrace: Arc<Backtrace>,
allocation_sizes: &AllocationSizes,
) -> Result<Allocation> {
let allocation_type = if desc.linear {
Expand Down Expand Up @@ -812,20 +811,20 @@ 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!(
"Allocating `{}` of {} bytes with an alignment of {}.",
&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);
}
}

Expand Down Expand Up @@ -915,7 +914,7 @@ impl Allocator {
let name = allocation.name.as_deref().unwrap_or("<null>");
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);
}
}
Expand Down

0 comments on commit 20ff10c

Please sign in to comment.