Skip to content

Commit

Permalink
Allow parenthesis in template arguments. (#174)
Browse files Browse the repository at this point in the history
Previously, we were too eager to truncate argument lists, and ended up
truncating expressions in parentheses even when they were _not_ argument
lists. For example, in C++ and in Rust generics, you can easily have a
type like `std::function<void (int, int)>::operator` or `foo::Bar<(A,
B)>`, neither of which should have the parts in `()` truncated.

This patch fixes that behavior by only pruning `()` expressions outside
of template parameters (`<>`).

Fixes #173.
  • Loading branch information
SoftwareApe authored May 29, 2020
1 parent 23df1d4 commit 2230ac7
Show file tree
Hide file tree
Showing 10 changed files with 653 additions and 81 deletions.
72 changes: 57 additions & 15 deletions src/collapse/perf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -609,25 +609,44 @@ fn tidy_generic(mut func: String) -> String {
// see https://github.com/brendangregg/FlameGraph/pull/72
// - C++ anonymous namespace annotations.
// see https://github.com/brendangregg/FlameGraph/pull/93
if let Some(first_paren) = func.find('(') {
if func[first_paren..].starts_with("anonymous namespace)") {
// C++ anonymous namespace
} else {
let mut is_go = false;
if let Some(c) = func.get((first_paren - 1)..first_paren) {
// if .get(-1) is None, can't be a dot
if c == "." {
// assume it's a Go method name, so do nothing
is_go = true;
let mut angle_bracket_depth = 0;
let mut parentheses_depth = 0;
let mut last_dot_index = Option::<usize>::None;
let mut length_without_parameters = func.len();
for (idx, c) in func.char_indices() {
match c {
'<' => {
angle_bracket_depth += 1;
}
'>' => {
angle_bracket_depth -= 1;
}
'(' => {
// ignore parentheses inside Rust/C++ templates
if angle_bracket_depth == 0 {
// Don't remove go functions starting with .(
let is_go_function = parentheses_depth == 0 && last_dot_index == Some(idx);
if !is_go_function {
// found start of parameter list
length_without_parameters = idx;
break;
}
parentheses_depth += 1;
}
}

if !is_go {
// kill it with fire!
func.truncate(first_paren);
')' => {
if angle_bracket_depth == 0 {
parentheses_depth -= 1;
}
}
}
'.' => {
// insert index + 1 so we can associate it with the opening parentheses for Golang
last_dot_index = Some(idx + 1);
}
_ => (),
};
}
func.truncate(length_without_parameters);

// The perl version here strips ' and "; we don't do that.
// see https://github.com/brendangregg/FlameGraph/commit/817c6ea3b92417349605e5715fe6a7cb8cbc9776
Expand Down Expand Up @@ -664,6 +683,28 @@ mod tests {
use crate::collapse::common;
use crate::collapse::Collapse;

// Test some interesting edge cased for tidy_generic
#[test]
fn test_tidy_generic() {
let test_expectations = [
(
"go/build.(*importReader).readByte",
"go/build.(*importReader).readByte",
),
("foo<Vec::<usize>>(Vec<usize>)", "foo<Vec::<usize>>"),
(".run()V", ".run"),
("base(BasicType) const", "base"),
(
"std::function<void (int, int)>::operator(int, int)",
"std::function<void (int, int)>::operator",
),
];

for (input, expected) in test_expectations.iter() {
assert_eq!(&tidy_generic(input.to_string()), expected);
}
}

lazy_static! {
static ref INPUT: Vec<PathBuf> = {
[
Expand All @@ -685,6 +726,7 @@ mod tests {
"./tests/data/collapse-perf/go-stacks.txt",
"./tests/data/collapse-perf/java-inline.txt",
"./tests/data/collapse-perf/weird-stack-line.txt",
"./tests/data/collapse-perf/cpp-stacks-std-function.txt",
]
.iter()
.map(PathBuf::from)
Expand Down
7 changes: 7 additions & 0 deletions tests/collapse-guess.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,13 @@ fn collapse_guess_perf_go_stacks() {
test_collapse_guess(test_file, result_file, true).unwrap()
}

#[test]
fn collapse_guess_perf_cpp_stacks() {
let test_file = "./tests/data/collapse-perf/cpp-stacks-std-function.txt";
let result_file = "./tests/data/collapse-perf/results/cpp-stacks-std-function-collapsed.txt";
test_collapse_guess(test_file, result_file, true).unwrap()
}

#[test]
fn collapse_guess_perf_java_inline() {
let test_file = "./tests/data/collapse-perf/java-inline.txt";
Expand Down
515 changes: 515 additions & 0 deletions tests/data/collapse-perf/cpp-stacks-std-function.txt

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
perf_stacks;[unknown];std::ostream::sentry::sentry 1
perf_stacks;_dl_start;[[kernel.kallsyms]] 1
perf_stacks;_start;[[kernel.kallsyms]] 6
perf_stacks;_start;__libc_start_main;__libc_csu_init;_GLOBAL__sub_I_main;__static_initialization_and_destruction_0;std::ios_base::Init::Init;std::locale::locale;[libstdc++.so.6.0.25];std::locale::_Impl::_Impl;std::__timepunct<wchar_t>::__timepunct;std::__timepunct<wchar_t>::_M_initialize_timepunct 1
perf_stacks;_start;__libc_start_main;main;std::endl<char, std::char_traits<char> >;std::ostream::put;_IO_new_file_overflow;_IO_new_do_write;new_do_write;_IO_new_file_write;__GI___libc_write;[[kernel.kallsyms]] 34
perf_stacks;_start;__libc_start_main;main;std::function<int (int, int)>::operator;std::_Function_handler<int (int, int), main::{lambda(int, int)#1}>::_M_invoke 1
perf_stacks;_start;__libc_start_main;main;std::ostream::flush 1
perf_stacks;_start;_dl_start;_dl_start_final;_dl_sysdep_start;dl_main;_dl_map_object_deps;_dl_catch_exception;openaux;_dl_map_object;_dl_map_object_from_fd;_dl_map_segments;__mmap64;[[kernel.kallsyms]] 1
Loading

0 comments on commit 2230ac7

Please sign in to comment.