Skip to content

Commit

Permalink
Turn on allocation tracker at run-time and for web (#1591)
Browse files Browse the repository at this point in the history
* Fix copy/paste in web viewer

* Memory tracker callstack: shorten file paths

* Fix typo

* clean up copy-pasta comment

* code refactor

* allow turning on allocation tracking at runtime from the memory panel

* nicer formatting in memory panel

* Notice text layout allocations in the summaries

* Notice failures to copy the backtrace

* Notice empty backtraces

* Fix stacktraces on web

* wasm_bindgen_check: build to target_wasm

* Always use target_wasm as build-dir when building wasm32

* Trim callstacks on web

* typo fixe

Co-authored-by: Andreas Reich <[email protected]>

* Use a checkbox for the detailed allocation tracking

* Make web callstack even nicer

* Even nicer callstacks

---------

Co-authored-by: Andreas Reich <[email protected]>
  • Loading branch information
emilk and Wumpf authored Mar 16, 2023
1 parent 38f4fd6 commit 364f86d
Show file tree
Hide file tree
Showing 13 changed files with 286 additions and 88 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/rust.yml
Original file line number Diff line number Diff line change
Expand Up @@ -218,13 +218,13 @@ jobs:
uses: actions-rs/cargo@v1
with:
command: check
args: --locked --all-features --lib --target wasm32-unknown-unknown -p re_viewer
args: --locked --all-features --lib --target wasm32-unknown-unknown --target-dir target_wasm -p re_viewer

- name: Check re_renderer examples wasm32
uses: actions-rs/cargo@v1
with:
command: check
args: --locked --target wasm32-unknown-unknown -p re_renderer --examples
args: --locked --target wasm32-unknown-unknown --target-dir target_wasm -p re_renderer --examples

- run: ./scripts/wasm_bindgen_check.sh --skip-setup

Expand Down
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions crates/re_build_web_viewer/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,13 @@ pub fn build(release: bool) {
let mut cmd = std::process::Command::new("cargo");
cmd.args([
"build",
"--target-dir",
target_wasm_dir.as_str(),
"-p",
"--package",
crate_name,
"--lib",
"--target",
"wasm32-unknown-unknown",
"--target-dir",
target_wasm_dir.as_str(),
]);
if release {
cmd.arg("--release");
Expand All @@ -78,7 +78,7 @@ pub fn build(release: bool) {
// These pre-encoded flags are generally generated by Cargo itself when loading its
// configuration from e.g. `$CARGO_HOME/config.toml`; which means they will contain
// values that only make sense for the native target host, not for a wasm build.
cmd.env("CARGO_ENCODED_RUSTFLAGS", "");
cmd.env("CARGO_ENCODED_RUSTFLAGS", "--cfg=web_sys_unstable_apis");

eprintln!("> {cmd:?}");
let status = cmd
Expand Down
7 changes: 6 additions & 1 deletion crates/re_memory/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,19 @@ re_format.workspace = true
re_log.workspace = true

ahash.workspace = true
backtrace = { version = "0.3" }
emath.workspace = true
instant = { version = "0.1", features = ["wasm-bindgen"] }
itertools = "0.10"
nohash-hasher = "0.2"
once_cell = "1.16"
parking_lot.workspace = true
smallvec = "1.10"

# native dependencies:
[target.'cfg(not(target_arch = "wasm32"))'.dependencies]
backtrace = "0.3"
memory-stats = "1.0"

# web dependencies:
[target.'cfg(target_arch = "wasm32")'.dependencies]
wasm-bindgen = "=0.2.84"
8 changes: 4 additions & 4 deletions crates/re_memory/src/accounting_allocator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ thread_local! {
///
/// Tracking an allocation (taking its backtrace etc) can itself create allocations.
/// We don't want to track those allocations, or we will have infinite recursion.
static IS_TRHEAD_IN_ALLOCATION_TRACKER: std::cell::Cell<bool> = std::cell::Cell::new(false);
static IS_THREAD_IN_ALLOCATION_TRACKER: std::cell::Cell<bool> = std::cell::Cell::new(false);
}

// ----------------------------------------------------------------------------
Expand Down Expand Up @@ -188,7 +188,7 @@ pub fn tracking_stats() -> Option<TrackingStatistics> {
}

GLOBAL_STATS.track_callstacks.load(Relaxed).then(|| {
IS_TRHEAD_IN_ALLOCATION_TRACKER.with(|is_thread_in_allocation_tracker| {
IS_THREAD_IN_ALLOCATION_TRACKER.with(|is_thread_in_allocation_tracker| {
// prevent double-lock of ALLOCATION_TRACKER:
is_thread_in_allocation_tracker.set(true);
let mut top_big_callstacks = tracker_stats(&BIG_ALLOCATION_TRACKER.lock());
Expand Down Expand Up @@ -303,7 +303,7 @@ fn note_alloc(ptr: *mut u8, size: usize) {
// Big enough to track - but make sure we don't create a deadlock by trying to
// track the allocations made by the allocation tracker:

IS_TRHEAD_IN_ALLOCATION_TRACKER.with(|is_thread_in_allocation_tracker| {
IS_THREAD_IN_ALLOCATION_TRACKER.with(|is_thread_in_allocation_tracker| {
if !is_thread_in_allocation_tracker.get() {
is_thread_in_allocation_tracker.set(true);

Expand Down Expand Up @@ -337,7 +337,7 @@ fn note_dealloc(ptr: *mut u8, size: usize) {
} else {
// Big enough to track - but make sure we don't create a deadlock by trying to
// track the allocations made by the allocation tracker:
IS_TRHEAD_IN_ALLOCATION_TRACKER.with(|is_thread_in_allocation_tracker| {
IS_THREAD_IN_ALLOCATION_TRACKER.with(|is_thread_in_allocation_tracker| {
if !is_thread_in_allocation_tracker.get() {
is_thread_in_allocation_tracker.set(true);

Expand Down
47 changes: 6 additions & 41 deletions crates/re_memory/src/allocation_tracker.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::{hash::Hash, sync::Arc};
use std::sync::Arc;

use backtrace::Backtrace;
use crate::{Backtrace, BacktraceHash};

use crate::CountAndSize;

Expand All @@ -22,25 +22,6 @@ impl PtrHash {

// ----------------------------------------------------------------------------

#[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)]
struct BacktraceHash(u64);

impl nohash_hasher::IsEnabled for BacktraceHash {}

fn hash_backtrace(backtrace: &Backtrace) -> BacktraceHash {
use std::hash::Hasher as _;
let mut hasher =
std::hash::BuildHasher::build_hasher(&ahash::RandomState::with_seeds(0, 1, 2, 3));

for frame in backtrace.frames() {
frame.ip().hash(&mut hasher);
}

BacktraceHash(hasher.finish())
}

// ----------------------------------------------------------------------------

/// Formatted [`Backtrace`].
///
/// Clones without allocating.
Expand All @@ -58,26 +39,10 @@ impl std::fmt::Display for ReadableBacktrace {

impl ReadableBacktrace {
fn new(mut backtrace: Backtrace) -> Self {
backtrace.resolve();
let readable = format_backtrace(&backtrace);
Self { readable }
}
}

fn format_backtrace(backtrace: &Backtrace) -> Arc<str> {
let stack = format!("{backtrace:?}");
let mut stack = stack.as_str();
let start_pattern = "re_memory::accounting_allocator::note_alloc\n";
if let Some(start_offset) = stack.find(start_pattern) {
stack = &stack[start_offset + start_pattern.len()..];
}

if let Some(end_offset) = stack.find("std::sys_common::backtrace::__rust_begin_short_backtrace")
{
stack = &stack[..end_offset];
Self {
readable: backtrace.format(),
}
}

stack.into()
}

// ----------------------------------------------------------------------------
Expand Down Expand Up @@ -137,7 +102,7 @@ impl AllocationTracker {
}

let unresolved_backtrace = Backtrace::new_unresolved();
let hash = hash_backtrace(&unresolved_backtrace);
let hash = BacktraceHash::new(&unresolved_backtrace);

self.readable_backtraces
.entry(hash)
Expand Down
120 changes: 120 additions & 0 deletions crates/re_memory/src/backtrace_native.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
use std::sync::Arc;

pub(crate) struct Backtrace(backtrace::Backtrace);

impl Backtrace {
pub fn new_unresolved() -> Self {
Self(backtrace::Backtrace::new_unresolved())
}

pub fn format(&mut self) -> Arc<str> {
self.0.resolve();
let stack = backtrace_to_string(&self.0);
trim_backtrace(&stack).into()
}
}

impl std::hash::Hash for Backtrace {
fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
for frame in self.0.frames() {
frame.ip().hash(state);
}
}
}

fn trim_backtrace(mut stack: &str) -> &str {
let start_pattern = "re_memory::accounting_allocator::note_alloc\n";
if let Some(start_offset) = stack.find(start_pattern) {
stack = &stack[start_offset + start_pattern.len()..];
}

let end_pattern = "std::sys_common::backtrace::__rust_begin_short_backtrace";
if let Some(end_offset) = stack.find(end_pattern) {
stack = &stack[..end_offset];
}

stack
}

fn backtrace_to_string(backtrace: &backtrace::Backtrace) -> String {
if backtrace.frames().is_empty() {
return "[empty backtrace]".to_owned();
}

// We need to get a `std::fmt::Formatter`, and there is no easy way to do that, so we do it the hard way:

struct AnonymizedBacktrace<'a>(&'a backtrace::Backtrace);

impl<'a> std::fmt::Display for AnonymizedBacktrace<'a> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
format_backtrace_with_fmt(self.0, f)
}
}

AnonymizedBacktrace(backtrace).to_string()
}

fn format_backtrace_with_fmt(
backtrace: &backtrace::Backtrace,
fmt: &mut std::fmt::Formatter<'_>,
) -> std::fmt::Result {
let mut print_path = |fmt: &mut std::fmt::Formatter<'_>,
path: backtrace::BytesOrWideString<'_>| {
let path = path.into_path_buf();
let shortened = shorten_source_file_path(&path);
std::fmt::Display::fmt(&shortened, fmt)
};

let style = if fmt.alternate() {
backtrace::PrintFmt::Full
} else {
backtrace::PrintFmt::Short
};
let mut f = backtrace::BacktraceFmt::new(fmt, style, &mut print_path);
f.add_context()?;
for frame in backtrace.frames() {
f.frame().backtrace_frame(frame)?;
}
f.finish()?;
Ok(())
}

/// Anonymize a path to a Rust source file from a callstack.
///
/// Example input:
/// * `/Users/emilk/.cargo/registry/src/jackfan.us.kg-1ecc6299db9ec823/tokio-1.24.1/src/runtime/runtime.rs`
/// * `crates/rerun/src/main.rs`
/// * `/rustc/d5a82bbd26e1ad8b7401f6a718a9c57c96905483/library/core/src/ops/function.rs`
fn shorten_source_file_path(path: &std::path::Path) -> String {
// We must make sure we strip everything sensitive (especially user name).
// The easiest way is to look for `src` and strip everything up to it.

use itertools::Itertools as _;
let components = path.iter().map(|path| path.to_string_lossy()).collect_vec();

// Look for the last `src`:
if let Some((src_rev_idx, _)) = components.iter().rev().find_position(|&c| c == "src") {
let src_idx = components.len() - src_rev_idx - 1;
// Before `src` comes the name of the crate - let's include that:
let first_index = src_idx.saturating_sub(1);
components.iter().skip(first_index).format("/").to_string()
} else {
// No `src` directory found - weird!
path.display().to_string()
}
}

#[test]
fn test_shorten_path() {
for (before, after) in [
("/Users/emilk/.cargo/registry/src/jackfan.us.kg-1ecc6299db9ec823/tokio-1.24.1/src/runtime/runtime.rs", "tokio-1.24.1/src/runtime/runtime.rs"),
("crates/rerun/src/main.rs", "rerun/src/main.rs"),
("/rustc/d5a82bbd26e1ad8b7401f6a718a9c57c96905483/library/core/src/ops/function.rs", "core/src/ops/function.rs"),
("/weird/path/file.rs", "/weird/path/file.rs"),
]
{
use std::str::FromStr as _;
let before = std::path::PathBuf::from_str(before).unwrap();
assert_eq!(shorten_source_file_path(&before), after);
}
}
63 changes: 63 additions & 0 deletions crates/re_memory/src/backtrace_web.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
use wasm_bindgen::prelude::*;

#[wasm_bindgen]
extern "C" {
#[wasm_bindgen(js_namespace = console)]
fn error(msg: String);

type Error;

#[wasm_bindgen(constructor)]
fn new() -> Error;

#[wasm_bindgen(structural, method, getter)]
fn stack(error: &Error) -> String;
}

#[derive(Hash)]
pub(crate) struct Backtrace(String);

impl Backtrace {
pub fn new_unresolved() -> Self {
Self(Error::new().stack())
}

pub fn format(&mut self) -> std::sync::Arc<str> {
trim_backtrace(&self.0).into()
}
}

fn trim_backtrace(mut stack: &str) -> String {
let start_pattern = "__rust_alloc_zeroed";
if let Some(start_offset) = stack.find(start_pattern) {
if let Some(next_newline) = stack[start_offset..].find('\n') {
stack = &stack[start_offset + next_newline + 1..];
}
}

let end_pattern = "paint_and_schedule"; // normal eframe entry-point
if let Some(end_offset) = stack.find(end_pattern) {
if let Some(next_newline) = stack[end_offset..].find('\n') {
stack = &stack[..end_offset + next_newline];
}
}

stack.split('\n').map(trim_line).collect::<String>()
}

/// Example inputs:
/// * `eframe::web::backend::AppRunner::paint::h584aff3234354fd5@http://127.0.0.1:9090/re_viewer.js line 366 > WebAssembly.instantiate:wasm-function[3352]:0x5d46b4`
/// * `getImports/imports.wbg.__wbg_new_83e4891414f9e5c1/<@http://127.0.0.1:9090/re_viewer.js:453:21`
/// * `__rg_realloc@http://127.0.0.1:9090/re_viewer.js line 366 > WebAssembly.instantiate:wasm-function[17996]:0x9b935f`
fn trim_line(mut line: &str) -> String {
if let Some(index) = line.rfind("::") {
line = &line[..index];
}
if let Some(index) = line.find("/imports.wbg") {
line = &line[..index];
}
if let Some(index) = line.find("@http:") {
line = &line[..index];
}
format!("{line}\n")
}
Loading

2 comments on commit 364f86d

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rust Benchmark

Benchmark suite Current: 364f86d Previous: 38f4fd6 Ratio
datastore/insert/batch/rects/insert 565626 ns/iter (± 2791) 559261 ns/iter (± 1861) 1.01
datastore/latest_at/batch/rects/query 1815 ns/iter (± 5) 1840 ns/iter (± 30) 0.99
datastore/latest_at/missing_components/primary 285 ns/iter (± 0) 286 ns/iter (± 0) 1.00
datastore/latest_at/missing_components/secondaries 438 ns/iter (± 2) 438 ns/iter (± 0) 1
datastore/range/batch/rects/query 152385 ns/iter (± 222) 151520 ns/iter (± 519) 1.01
mono_points_arrow/generate_message_bundles 53571375 ns/iter (± 1058657) 48318016 ns/iter (± 494422) 1.11
mono_points_arrow/generate_messages 141821941 ns/iter (± 1654501) 127474339 ns/iter (± 1508294) 1.11
mono_points_arrow/encode_log_msg 168643961 ns/iter (± 2034860) 159549179 ns/iter (± 1325016) 1.06
mono_points_arrow/encode_total 373382705 ns/iter (± 4015002) 337112136 ns/iter (± 1931763) 1.11
mono_points_arrow/decode_log_msg 191288531 ns/iter (± 7707687) 181004618 ns/iter (± 744958) 1.06
mono_points_arrow/decode_message_bundles 76612590 ns/iter (± 1439086) 65470475 ns/iter (± 794309) 1.17
mono_points_arrow/decode_total 265899147 ns/iter (± 2557182) 243039709 ns/iter (± 1379454) 1.09
batch_points_arrow/generate_message_bundles 326287 ns/iter (± 3260) 323386 ns/iter (± 480) 1.01
batch_points_arrow/generate_messages 6502 ns/iter (± 26) 6458 ns/iter (± 20) 1.01
batch_points_arrow/encode_log_msg 362765 ns/iter (± 2535) 356799 ns/iter (± 984) 1.02
batch_points_arrow/encode_total 712671 ns/iter (± 6012) 706689 ns/iter (± 2127) 1.01
batch_points_arrow/decode_log_msg 341372 ns/iter (± 2111) 347642 ns/iter (± 1114) 0.98
batch_points_arrow/decode_message_bundles 2090 ns/iter (± 20) 2072 ns/iter (± 12) 1.01
batch_points_arrow/decode_total 367811 ns/iter (± 21060) 356898 ns/iter (± 1454) 1.03
arrow_mono_points/insert 9256181423 ns/iter (± 120643625) 6126182762 ns/iter (± 35168800) 1.51
arrow_mono_points/query 1847569 ns/iter (± 39588) 1823165 ns/iter (± 16496) 1.01
arrow_batch_points/insert 2862042 ns/iter (± 314987) 2768566 ns/iter (± 73357) 1.03
arrow_batch_points/query 16966 ns/iter (± 70) 17005 ns/iter (± 39) 1.00
arrow_batch_vecs/insert 42618 ns/iter (± 150) 42357 ns/iter (± 101) 1.01
arrow_batch_vecs/query 506559 ns/iter (± 1221) 507025 ns/iter (± 1072) 1.00
tuid/Tuid::random 34 ns/iter (± 0) 34 ns/iter (± 0) 1

This comment was automatically generated by workflow using github-action-benchmark.

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'Rust Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.50.

Benchmark suite Current: 364f86d Previous: 38f4fd6 Ratio
arrow_mono_points/insert 9256181423 ns/iter (± 120643625) 6126182762 ns/iter (± 35168800) 1.51

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.