public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Milian Wolff <milian.wolff@kdab.com>
To: Jin Yao <yao.jin@linux.intel.com>
Cc: acme@kernel.org, jolsa@kernel.org, Linux-kernel@vger.kernel.org,
	ak@linux.intel.com, kan.liang@intel.com, yao.jin@intel.com
Subject: Re: [PATCH v5 0/5] perf report: Show inline stack
Date: Sat, 18 Mar 2017 17:41:09 +0100	[thread overview]
Message-ID: <1532086.vKWRnURGfG@agathebauer> (raw)
In-Reply-To: <1489700547-7260-1-git-send-email-yao.jin@linux.intel.com>

[-- Attachment #1: Type: text/plain, Size: 3220 bytes --]

On Donnerstag, 16. März 2017 22:42:22 CET Jin Yao wrote:
> v5: Update according to Milian Wolff's comments. It groups by address
>     (then display file/ line), or by function (then display function name).

Thank you Jin, that is really good. I tested it and it works really well for 
me.

Arnaldo, could you please consider merging this? It's an extremely useful 
feature and direly missing from perf so far.

That said, Jin, here are some observations that could be improved in the 
future (I don't think any of these should hold back merging this feature now):

For the following example code build with "-O2 -g" and recorded with "--call-
graph dwarf" I observe some output combinations that could potentially be 
improved in the future:

~~~~~~~~~~~~~~~~~~~~
#include <complex>
#include <cmath>
#include <random>
#include <iostream>

using namespace std;

int main()
{
    uniform_real_distribution<double> uniform(-1E5, 1E5);
    default_random_engine engine;
    double s = 0;
    for (int i = 0; i < 10000000; ++i) {
        s += norm(complex<double>(uniform(engine), uniform(engine)));
    }
    cout << s << '\n';
    return 0;
}
~~~~~~~~~~~~~~~~

#1 duplicated entries when grouping by function:

~~~~~~~~~~~~~~~~
perf report --inline --stdio
...
             --35.34%--_start
                       __libc_start_main
                       main
                       main (inline)
                       std::uniform_real_distribution<double>::operator()<std::linear_congruential_engine<unsigned 
long, 16807ul, 0ul, 2147483647ul> > (inline)
                       std::uniform_real_distribution<double>::operator()<std::linear_congruential_engine<unsigned 
long, 16807ul, 0ul, 2147483647ul> > (inline)
                       std::__detail::_Adaptor<std::linear_congruential_engine<unsigned 
long, 16807ul, 0ul, 2147483647ul>, double>::operator() (inline)
~~~~~~~~~~~~~~~~

Here, we see main twice, once for the "real" frame, and once for an inlined 
one? Then we see the same function twice as inlined frame, which is also odd.

~~~~~~~~~~~~~~~~
perf report --inline --stdio --no-children
...
    59.81%  cpp-inlining  libm-2.25.so      [.] __hypot_finite
            |
            ---__hypot_finite
               hypot
               main
               std::norm<double> (inline)
               main (inline)
               __libc_start_main
               _start
~~~~~~~~~~~~~~~~

Here we see a confusing output. The first "main" frame below "hypot" is 
actually code form cpp's complex header which got inlined into main. That 
associates the wrong function name to this frame, i.e. "main" instead of 
std::norm". When the inline stack is shown below we actually see what happens, 
i.e. we eventually end up in main again, but of course this output is not the 
best as-is.

But, again: I think these are minor issues, and the feature itself is already 
extremely useful and I hope to see it finally merged.

Thanks again Jin for your good work!

Cheers

-- 
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

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5903 bytes --]

  parent reply	other threads:[~2017-03-18 17:34 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-16 21:42 [PATCH v5 0/5] perf report: Show inline stack Jin Yao
2017-03-16 21:42 ` [PATCH v5 1/5] perf report: Refactor common code in srcline.c Jin Yao
2017-03-16 21:42 ` [PATCH v5 2/5] perf report: Find the inline stack for a given address Jin Yao
2017-03-24 18:37   ` Arnaldo Carvalho de Melo
2017-03-25  7:18   ` Ravi Bangoria
2017-03-25 12:45     ` Jin, Yao
2017-03-16 21:42 ` [PATCH v5 3/5] perf report: Create new inline option Jin Yao
2017-03-16 21:42 ` [PATCH v5 4/5] perf report: Show inline stack for stdio mode Jin Yao
2017-03-16 21:42 ` [PATCH v5 5/5] perf report: Show inline stack for browser mode Jin Yao
2017-03-18 16:41 ` Milian Wolff [this message]
2017-03-24 19:01   ` [PATCH v5 0/5] perf report: Show inline stack Arnaldo Carvalho de Melo
2017-03-24 19:24     ` Arnaldo Carvalho de Melo
2017-03-25  0:20       ` Jin, Yao

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1532086.vKWRnURGfG@agathebauer \
    --to=milian.wolff@kdab.com \
    --cc=Linux-kernel@vger.kernel.org \
    --cc=acme@kernel.org \
    --cc=ak@linux.intel.com \
    --cc=jolsa@kernel.org \
    --cc=kan.liang@intel.com \
    --cc=yao.jin@intel.com \
    --cc=yao.jin@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox