Messages in this thread | | | Subject | Re: [PATCH v3 0/5] perf report: Show inline stack | From | "Jin, Yao" <> | Date | Fri, 3 Mar 2017 10:38:40 +0800 |
| |
Hi Wolff,
Thanks so much for your testing. I also wish this feature could be upstreamed.
I will send a v4 series soon. In v4, It removes the options "--inline-line" and "--inline-name".
It just uses a new option "--inline" to print the inline function information. The policy is if the inline function name can be resolved then print the function name otherwise it prints the source line number.
For example: perf report --stdio --inline
It prints:
0.69% 0.00% inline ld-2.23.so [.] dl_main | ---dl_main | --0.56%--_dl_relocate_object | ---_dl_relocate_object (inline) elf_dynamic_do_Rela (inline)
Thanks
Jin Yao
On 3/3/2017 5:42 AM, Milian Wolff wrote: > On Dienstag, 21. Februar 2017 01:28:17 CET Jin, Yao wrote: >> Hi, >> >> Any comments for this patch series? > Sorry for the delay. I just tested it again. > > Overall, this is a clear improvement, so I'm all for getting this > functionality in. > > But from a usability point of view, I still have the some of the issues that I > have raised in the past: > > a) --inline should be a boolean setting that enables inline resolution on > demand > > b) the other callgraph settings and formatting should be used for inlined > frames, i.e. > > - instead of `perf report --inline-name` > it should be: `perf report --inline -g function` > and since `-g function` is the default, it would be the same as: > `perf report --inline` > > - instead of `perf report --inline-line -g address` > it should be: `perf report --inline -g address` > > Again: As a user of `perf report`, I do not care whether a frame is an inlined > one or a non-inlined one - both should be grouped and displayed the same way. > > I.e. this is unusable (imo): > > ~~~~~~~~~~~~~~~~~~~~~~~~~ > perf report --inline-line --stdio > > 99.81% 35.99% cpp-inlining cpp-inlining [.] main > | > |--63.82%--main > | > ---/home/milian/projects/kdab/rnd/hotspot/tests/test- > clients/cpp-inlining/main.cpp:39 (inline) > /usr/include/c++/6.3.1/complex:664 (inline) > | | > | |--63.19%--hypot > | | | > | | --58.04%--__hypot_finite > | | > | --0.62%--cabs > ~~~~~~~~~~~~~~~~~~~~~~~~~ > > Dito for this: > > ~~~~~~~~~~~~~~~~~~~~~~~~~ > perf report --stdio --inline-name -g address --stdio > > 99.81% 35.99% cpp-inlining cpp-inlining [.] main > | > |--63.82%--main complex:655 > | > ---main (inline) > std::norm<double> (inline) > ~~~~~~~~~~~~~~~~~~~~~~~~~ > > But, again, even with these gripes, I think it's a very useful feature and I > would like to see it integrated upstream. From my POV, you can add > > Tested-by: Milian Wolff <milian.wolff@kdab.com> > > to all patches in this series. > > Many thanks! >
| |