mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Namhyung Kim <namhyung@kernel.org>
To: Ingo Molnar <mingo@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Paul Mackerras <paulus@samba.org>,
	Namhyung Kim <namhyung.kim@lge.com>,
	LKML <linux-kernel@vger.kernel.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	Jiri Olsa <jolsa@redhat.com>
Subject: Re: [PATCHSET 0/8] perf tools: Fix scalability problem on callchain merging (v4)
Date: Fri, 27 Sep 2013 11:08:06 +0900	[thread overview]
Message-ID: <87ioxnow3d.fsf@sejong.aot.lge.com> (raw)
In-Reply-To: <20130926093426.GA24596@gmail.com> (Ingo Molnar's message of "Thu, 26 Sep 2013 11:34:26 +0200")

Hi Ingo,

On Thu, 26 Sep 2013 11:34:26 +0200, Ingo Molnar wrote:
> * Namhyung Kim <namhyung@kernel.org> wrote:
>
>> Hello,
>> 
>> This is a new version of callchain improvement patchset.  I found and
>> fixed bugs in the previous version.  I verified that it produced
>> exactly same output before and after applying rbtree conversion patch
>> (#1).  However after Frederic's new comm infrastructure patches are
>> applied it'd be little different.
>> 
>> The patches are on 'perf/callchain-v4' branch in my tree
>> 
>>   git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
>
> I pulled this into a test-branch and resolved the builtin-trace.c conflict 
> with tip:master.
>
> I tried your new code out with Linus's testcase of a parallel kernel 
> build:
>
>    perf record -g -- make -j8 bzImage
>
> The 'perf record' session went mostly fine except for lost events:
>
>   Kernel: arch/x86/boot/bzImage is ready  (#25)
>   [ perf record: Woken up 553 times to write data ]
>   [ perf record: Captured and wrote 196.811 MB perf.data (~8598789 samples) ]
>   Warning:
>   Processed 2631982 events and lost 54 chunks!
>
>   Check IO/CPU overload!
>
> So, for completeness I'll mention that perf fails in two ways here:
>
>   - UI bug: it prints some scary warnings about 'IO/CPU overload' but 
>     does not give any idea what the user could do about it. At minimum it 
>     should print something about increasing --mmap-pages. It should also 
>     print the current value of --mmap-pages so that the user knows to what 
>     value to increase it ...
>
>   - defaults bug: this isn't an extreme workload on an extreme box - it's
>     a relatively bog standard system with 8 cores and 16 CPUs. The kernel
>     build was mild as well, with -j8. So losing events in perf record is
>     absolutely unacceptable. A solution might be to automatically increase 
>     the ring-buffer if -g is used, in expectation of a higher data rate.

Both look sensible.  I'll try to cook patches for them.

>
> Once perf record was done I had the ~200 MB perf.data - and the 'perf 
> report' session went much smoother than ever before: the analysis went 
> very quickly, it finished within 10 seconds and displayed a nice progress 
> bar. So this is entirely usable IMO.
>
> One small 'perf report' annoyance is that during the analysis passes 
> missing symbol printouts flash in the TUI - without the user having a 
> chance to read them. So those messages should either linger, or be 
> displayed at a later stage, for example when the user is confronted with 
> non-resolved symbols like:
>
> +  28.63%             cc1  cc1                                [.] 0x00000000006a92cb                                  ◆
>
> that is the point where the message would be useful - but nothing 
> indicates at all that this is an undesirable symbol entry and nothing 
> helps the user what to do about it!
>
> A solution might be to display non-intrusive messages about non-resolved 
> symbols when such a symbol is manipulated (its children are openened, or 
> annotation is attempted).

Hmm.. I'll think about it more how to do it.

>
> Here there is a second annoyance: on the detailed screen the 'annotate' 
> entry is simply missing when the symbol has not been resolved. If I hit 
> 'a' on the symbol entry itself in the graph view, then sometimes 
> annotation works - sometimes it does not and there's no UI feedback 
> whatsoever why it's not working.
>
> I'm not suggesting to change the keyboard flow - that is very smooth - but 
> display information about failed annotations in the status line at the 
> bottom of the screen would be very useful. Remember: it's _always_ an UI 
> bug if the user hits a key and absolutely nothing happens. At minimum a 
> low-key, non-intrusive 'key X not bound' message should appear in the 
> status line bottom. Deterministic action/reaction sequences are utterly 
> important when interacting with computers.
>
> Anyway, very nice progress with the tree here!

Hehe, thanks!
Namhyung

  reply	other threads:[~2013-09-27  2:08 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-09-26  8:58 Namhyung Kim
2013-09-26  8:58 ` [PATCH 1/8] perf callchain: Convert children list to rbtree Namhyung Kim
2013-10-02 10:18   ` Frederic Weisbecker
2013-10-08  2:03     ` Namhyung Kim
2013-10-08 19:22       ` Frederic Weisbecker
2013-10-10  1:06         ` Namhyung Kim
2013-09-26  8:58 ` [PATCH 2/8] perf ui/progress: Add new helper functions for progress bar Namhyung Kim
2013-09-26  8:58 ` [PATCH 3/8] perf tools: Show progress on histogram collapsing Namhyung Kim
2013-09-26  8:58 ` [PATCH 4/8] perf tools: Use an accessor to read thread comm Namhyung Kim
2013-09-26  8:58 ` [PATCH 5/8] perf tools: Add time argument on comm setting Namhyung Kim
2013-09-26  8:58 ` [PATCH 6/8] perf tools: Add new comm infrastructure Namhyung Kim
2013-09-26  8:58 ` [PATCH 7/8] perf tools: Compare hists comm by addresses Namhyung Kim
2013-09-26  8:58 ` [PATCH 8/8] perf tools: Get current comm instead of last one Namhyung Kim
2013-10-02 10:01   ` Frederic Weisbecker
2013-10-08  1:56     ` Namhyung Kim
2013-09-26  9:34 ` [PATCHSET 0/8] perf tools: Fix scalability problem on callchain merging (v4) Ingo Molnar
2013-09-27  2:08   ` Namhyung Kim [this message]
2013-09-26 13:46 ` David Ahern
2013-09-26 14:07   ` Arnaldo Carvalho de Melo
2013-09-27  2:10     ` Namhyung Kim

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=87ioxnow3d.fsf@sejong.aot.lge.com \
    --to=namhyung@kernel.org \
    --cc=a.p.zijlstra@chello.nl \
    --cc=acme@ghostprotocols.net \
    --cc=fweisbec@gmail.com \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=namhyung.kim@lge.com \
    --cc=paulus@samba.org \
    --cc=torvalds@linux-foundation.org \
    /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

Powered by JetHome