Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1591513 > unrolled thread
| Started by | Milian Wolff <milian.wolff@kdab.com> |
|---|---|
| First post | 2017-03-02 23:20 +0100 |
| Last post | 2017-03-03 03:40 +0100 |
| Articles | 2 — 2 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: [PATCH v3 0/5] perf report: Show inline stack Milian Wolff <milian.wolff@kdab.com> - 2017-03-02 23:20 +0100
Re: [PATCH v3 0/5] perf report: Show inline stack "Jin, Yao" <yao.jin@linux.intel.com> - 2017-03-03 03:40 +0100
| From | Milian Wolff <milian.wolff@kdab.com> |
|---|---|
| Date | 2017-03-02 23:20 +0100 |
| Subject | Re: [PATCH v3 0/5] perf report: Show inline stack |
| Message-ID | <tgHeO-eg-25@gated-at.bofh.it> |
[Multipart message — attachments visible in raw view] — view raw
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!
--
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
[toc] | [next] | [standalone]
| From | "Jin, Yao" <yao.jin@linux.intel.com> |
|---|---|
| Date | 2017-03-03 03:40 +0100 |
| Message-ID | <tgLiq-2RF-9@gated-at.bofh.it> |
| In reply to | #1591513 |
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!
>
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web