All of lore.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.