Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

<> characters in C++ types get munged #46

Open
tavianator opened this issue Jan 18, 2015 · 12 comments
Open

<> characters in C++ types get munged #46

tavianator opened this issue Jan 18, 2015 · 12 comments

Comments

@tavianator
Copy link

C++ template types have lots of <s and >s in them. But they get filtered out by stackcollapse-perf.pl (and possibly others, didn't check), and the resulting SVG has things like

std::vectorint, std::allocatorint ::push_back

instead of

std::vector<int, std::allocator<int> >::push_back

The following patch fixes this, but I assume has unintended consequences:

diff --git a/flamegraph.pl b/flamegraph.pl
index def5ed0..ede4514 100755
--- a/flamegraph.pl
+++ b/flamegraph.pl
@@ -517,8 +517,6 @@ foreach (sort @Data) {
                $maxdelta = abs($delta) if abs($delta) > $maxdelta;
        }

-       $stack =~ tr/<>/()/;
-
        # merge frames and populate %Node:
        $last = flow($last, [ '', split ";", $stack ], $time, $delta);

diff --git a/stackcollapse-perf.pl b/stackcollapse-perf.pl
index 16b47f0..fdca6cc 100755
--- a/stackcollapse-perf.pl
+++ b/stackcollapse-perf.pl
@@ -135,7 +135,6 @@ foreach (<STDIN>) {
                next if $func =~ /^\(/;         # skip process names
                if ($tidy_generic) {
                        $func =~ s/;/:/g;
-                       $func =~ tr/<>//d;
                        $func =~ s/\(.*//;
                        # now tidy this horrible thing:
                        # 13a80b608e0a RegExp:[&<>\"\'] (/tmp/perf-7539.map)
@tavianator
Copy link
Author

Also, demangled C++ types can have (anonymous namespace):: in them, which makes the $func =~ s/\(.*//; line suspect as well.

@borellim
Copy link

borellim commented Feb 9, 2016

Fixing this issue would be very useful. It's quite hard to read templated functions at the moment.

Thank you @tavianator for the patch: it appears to turn angle brackets into round brackets, which is a nice temporary solution.

Even better, in my opinion, would be to add a switch to completely strip template parameters: this would shorten the names, and the loss of information would be negligible in many cases. 'gprof2dot' can do it with the '-s' option.

@brendangregg , any input on this?

@brendangregg
Copy link
Owner

This might have been fixed since the issue was filed, as I see flamegraph.pl currently has:

   572      # clean up SVG breaking characters:
   573      $stack =~ tr/<>/()/;

@borellim
Copy link

So, I was wrong: the angle brackets are turned into parentheses exactly by the line that you mentioned. Unfortunately (at least with perf) they don't actually show up unless you also remove line 216 in stackcollapse-perf.pl:

$func =~ tr/<>//d;

In my tests, removing this line shows them as parentheses, while also removing line 573 of flamegraph.pl correctly shows them as angle brackets. I didn't try with input from other profilers, though.

My suggestion to add a switch to strip the template parameters still stands; I'm using a sed script now, and I find the output much clearer.

Thanks a lot.

@Yamakaky
Copy link
Contributor

I have a similar issue with Rust, with has the same syntax that C++ for the generics. <...> gets turned into (). If I remove the $stack =~ tr/<>/()/; line mentioned above, the <> are kept as expected. From the comments it seems like it's to protect SVG, but I don't have any issues with this fix.

@Yamakaky
Copy link
Contributor

The <> characters are xml encoded, so I don't think it's a problem https://github.com/brendangregg/FlameGraph/blob/master/flamegraph.pl#L1022-L1023

@brendangregg
Copy link
Owner

I just pushed the patch earlier (thanks), and this is hopefully fixed.

I also added a new test file, test/perf-js-stacks-01.txt, and the corresponding test/results files, which are checked by the test.sh program. This new test file has many <> chars in.

If you have some rust or C++ profile output in particular that should behave, it would be good to add sample output (you may need to redact it) in a similar way under test/. It can be just a few stacks (like the one I just added).

Eg, to fix the "(anonymous namespace)::" issue it's going to be helpful to have a variety of output in test/ so we can check what regexp is going to work...

@Yamakaky
Copy link
Contributor

I have a rust example. What do you want exactly? perf.data + output svg?

@brendangregg
Copy link
Owner

Just the "perf script" output, and you can edit/truncate it down as much as you like. See under test/ -- some are the full output (including header), some are just a couple of stacks. You can also just create a new ticket with "rust example" and paste the "perf script" output in and I'll add it to tests. thanks!

@Yamakaky
Copy link
Contributor

in a PR?

@brendangregg
Copy link
Owner

either a PR or just pasted in a ticket is fine

@Yamakaky
Copy link
Contributor

Yamakaky commented Oct 2, 2016

Just did #109

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants