public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Milian Wolff <milian.wolff@kdab.com>
Cc: Jin Yao <yao.jin@linux.intel.com>,
	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: Fri, 24 Mar 2017 16:01:19 -0300	[thread overview]
Message-ID: <20170324190119.GE5148@kernel.org> (raw)
In-Reply-To: <1532086.vKWRnURGfG@agathebauer>

Em Sat, Mar 18, 2017 at 05:41:09PM +0100, Milian Wolff escreveu:
> 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.

Thanks, applied.
 
> 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

  reply	other threads:[~2017-03-24 19:01 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 ` [PATCH v5 0/5] perf report: Show inline stack Milian Wolff
2017-03-24 19:01   ` Arnaldo Carvalho de Melo [this message]
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=20170324190119.GE5148@kernel.org \
    --to=acme@kernel.org \
    --cc=Linux-kernel@vger.kernel.org \
    --cc=ak@linux.intel.com \
    --cc=jolsa@kernel.org \
    --cc=kan.liang@intel.com \
    --cc=milian.wolff@kdab.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