Feedback on: [RESEND PATCH v2 0/5] perf report: Show inline stack

[Milian Wolff <milian.wolff@kdab.com>]

Hey Jin,

I have finally had the time to test your patch series. Sadly, I hit a 
fundamental issue with it early on in my testing:

In my test case, I get with `perf report --no-children` e.g. the following:

    42.50%  lab_mandelbrot   libm-2.24.so                [.] __hypot_finite
            |
            ---__hypot_finite
               hypot
               main
               __libc_start_main
               _start

Enabling --inline does not change the output, even though hypot is inlined 
into main.

Looking at your code, it looks to me as if you are only trying to find 
inliners for root entries, cf. hist_entry__fprintf (stdio/hist.c), and only 
when unwinding failed (?, cf. (callchain_ret == 0)).

Is this understanding correct, or should your patch also add inliners for 
nodes in the middle of the callstack?

This approach makes the whole feature unusable for my use case. In my (really 
hacky) approach to adding inliners to the output of perf (https://github.com/
milianw/linux/commit/71d031c9d679bfb4a4044226e8903dd80ea601b3) I decided to 
lookup inliners for _every_ callchain entry. The result then there is with my 
branch and a `perf report -g srcline --stdio`:

    42.50%  lab_mandelbrot   libm-2.24.so                [.] __hypot_finite
            |
            ---__hypot_finite +387
               hypot +20
               std::__complex_abs complex:589
               std::abs<double> complex:597
               drawMandelbrot mandelbrot.h:39
               main main.cpp:55
               __libc_start_main +241
               _start +42

Note the three inliner entries in the middle of the callstack. The way I did 
it, one will also see "proper" entries for inlined functions in a top-down 
`perf report --children` output.

In general, your patch series looks much more polished than mine, so I would 
love to get it integrated. But with your approach, we will need lots of 
changes to the various browsers to get all features in. My hacky approach has 
the nice property that it simply adds "normal" callchain entries for inlined 
nodes - maybe you should do the same?

Thanks a lot for your work, much appreciated.

-- 
Milian Wolff | milian.wolff@kdab.com | Software Engineer
KDAB (Deutschland) GmbH&Co KG, a KDAB Group company
Tel: +49-30-521325470
KDAB - The Qt Experts