From 41de77c1b465b5267c643cdd57b14cab891ddc7a Mon Sep 17 00:00:00 2001 From: Ivan Babrou Date: Sun, 5 Jan 2025 00:27:38 -0800 Subject: [PATCH] Trim reversed stacks to prevent sort check from failing (#338) If the last line of the stack starts with a space, then the space is used in sorting. However, the space is removed when the line is then fed into the flamegraph, which makes the sorting invalid. This commit adds a trim to the reverse process to fix this. --- src/flamegraph/mod.rs | 4 + .../with-space-reversed.svg | 167 ++++++++++++++++++ .../fractional-samples/with-space.txt | 6 + tests/flamegraph.rs | 12 ++ 4 files changed, 189 insertions(+) create mode 100644 tests/data/flamegraph/fractional-samples/with-space-reversed.svg create mode 100644 tests/data/flamegraph/fractional-samples/with-space.txt diff --git a/src/flamegraph/mod.rs b/src/flamegraph/mod.rs index e43422e4..7b4991f2 100644 --- a/src/flamegraph/mod.rs +++ b/src/flamegraph/mod.rs @@ -429,6 +429,10 @@ where } stack.push(' '); stack.push_str(&line[samples_idx..]); + // Trim to handle the case where functions names internally contain `;`. + // This can happen, for example, with types like `[u8; 8]` in Rust. + // See https://github.com/jonhoo/inferno/pull/338. + let stack = stack.trim(); reversed.push(&stack); } let mut reversed: Vec<&str> = reversed.iter().collect(); diff --git a/tests/data/flamegraph/fractional-samples/with-space-reversed.svg b/tests/data/flamegraph/fractional-samples/with-space-reversed.svg new file mode 100644 index 00000000..a59076da --- /dev/null +++ b/tests/data/flamegraph/fractional-samples/with-space-reversed.svg @@ -0,0 +1,167 @@ + + + + + + + + + + + + + + + Flame Graph + + Reset Zoom + Search + + + + [unknown] (2 samples, 0.60%) + + + + + noploop (2 samples, 0.60%) + + + + + _]>::default::h67c9877e6f2a615c (19 samples, 5.71%) + + _]>::de.. + + + core::array::<impl core::default::Default for [T (19 samples, 5.71%) + + core::a.. + + + main (19 samples, 5.71%) + + main + + + cksum (19 samples, 5.71%) + + cksum + + + cksum (6 samples, 1.80%) + + c.. + + + cksum (37 samples, 11.11%) + + cksum + + + main (31 samples, 9.31%) + + main + + + __libc_start_main (31 samples, 9.31%) + + __libc_start_.. + + + _start (31 samples, 9.31%) + + _start + + + cksum (31 samples, 9.31%) + + cksum + + + ext4_file_read_iter (1 samples, 0.30%) + + + + + __vfs_read (1 samples, 0.30%) + + + + + vfs_read (1 samples, 0.30%) + + + + + sys_read (1 samples, 0.30%) + + + + + entry_SYSCALL_64_fastpath (1 samples, 0.30%) + + + + + _IO_file_read (1 samples, 0.30%) + + + + + _IO_file_xsgetn (1 samples, 0.30%) + + + + + __GI___fread_unlocked (1 samples, 0.30%) + + + + + cksum (1 samples, 0.30%) + + + + + cksum (1 samples, 0.30%) + + + + + all (333 samples, 100%) + + + + + main (274 samples, 82.28%) + + main + + + noploop (274 samples, 82.28%) + + noploop + + + \ No newline at end of file diff --git a/tests/data/flamegraph/fractional-samples/with-space.txt b/tests/data/flamegraph/fractional-samples/with-space.txt new file mode 100644 index 00000000..cfa3a89e --- /dev/null +++ b/tests/data/flamegraph/fractional-samples/with-space.txt @@ -0,0 +1,6 @@ +cksum;_start;__libc_start_main;main;cksum 31.23 +cksum;cksum 6.1 +cksum;cksum;__GI___fread_unlocked;_IO_file_xsgetn;_IO_file_read;entry_SYSCALL_64_fastpath_[k];sys_read_[k];vfs_read_[k];__vfs_read_[k];ext4_file_read_iter_[k] 1.4 +cksum;main;core::array::::default::h67c9877e6f2a615c 19.0 +noploop;[unknown] 2.567 +noploop;main 274.321 diff --git a/tests/flamegraph.rs b/tests/flamegraph.rs index 167da77c..ec1b3d27 100644 --- a/tests/flamegraph.rs +++ b/tests/flamegraph.rs @@ -806,6 +806,18 @@ fn flamegraph_reversed_stack_ordering_with_fractional_samples() { test_flamegraph(input_file, expected_result_file, options).unwrap(); } +#[test] +fn flamegraph_reversed_stack_ordering_with_space() { + let input_file = "./tests/data/flamegraph/fractional-samples/with-space.txt"; + let expected_result_file = "./tests/data/flamegraph/fractional-samples/with-space-reversed.svg"; + + let mut options = flamegraph::Options::default(); + options.hash = true; + options.reverse_stack_order = true; + + test_flamegraph(input_file, expected_result_file, options).unwrap(); +} + #[test] fn flamegraph_should_warn_about_no_sort_when_reversing_stack_ordering() { let mut options = flamegraph::Options::default();