mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@elte.hu>
To: Arnaldo Carvalho de Melo <acme@infradead.org>
Cc: linux-kernel@vger.kernel.org, David Ahern <dsahern@gmail.com>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	Mike Galbraith <efault@gmx.de>, Paul Mackerras <paulus@samba.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Stephane Eranian <eranian@google.com>
Subject: Re: [GIT PULL 0/7] perf/core fixes and improvements
Date: Wed, 19 Oct 2011 09:14:57 +0200	[thread overview]
Message-ID: <20111019071457.GB2075@elte.hu> (raw)
In-Reply-To: <1318974247-6683-1-git-send-email-acme@infradead.org>


* Arnaldo Carvalho de Melo <acme@infradead.org> wrote:

>         Please consider pulling from:
> 
> git://github.com/acmel/linux.git perf/core
> 
> 	The TUI now should be much closer to the old 'perf top' stdio
> visual/experience, more on that vein in the next pull req.
> 
> - Arnaldo
> 
> Arnaldo Carvalho de Melo (7):
>   perf hists browser: Add missing hotkeys to the help window
>   perf tui: Catch signals to exit gracefully
>   perf ui browser: Allow initial use without navigation UI elements
>   perf hists: Don't format the percentage on hist_entry__snprintf
>   perf tui: Remove unneeded call to newtCls on startup
>   perf ui browser: Make the colors configurable and change the defaults
>   perf top tui: Give color hints just on the percentage, like on --stdio
> 
>  tools/perf/Documentation/perfconfig.example |   20 ++++
>  tools/perf/util/hist.c                      |   29 ++++--
>  tools/perf/util/hist.h                      |    4 +-
>  tools/perf/util/ui/browser.c                |  127 ++++++++++++++++++++++-----
>  tools/perf/util/ui/browser.h                |    2 +
>  tools/perf/util/ui/browsers/annotate.c      |    6 ++
>  tools/perf/util/ui/browsers/hists.c         |   51 ++++++-----
>  tools/perf/util/ui/setup.c                  |   25 +++++-
>  8 files changed, 203 insertions(+), 61 deletions(-)
>  create mode 100644 tools/perf/Documentation/perfconfig.example

Pulled, thanks Arnaldo.

The way the TUI now follows terminal colors is really nice. There's a 
few regressions that sneaked in, and there's also still a few rough 
edges as well:

 - in callq following if i exit a secondary annotation screen via 'q' 
   or left-arrow, it does not jump back to the callq line as it did 
   earlier.

 - it's still hard to find all the callq's in the stream of assembly,
   i think it should be highlighted in a minimal fashion.

 - the mixed assembly and source code output for annotation now 
   became *harder* to follow, that the instruction opcodes are not 
   embedded. The reason is that there's now fewer visual patterns 
   that set apart the two types of lines.

   Not sure what to do about it, but it's not really usable this way, 
   to me at least. Color differentiation would certainly help, if 
   it's not too intrusive - could assembly be drawn grey? That would 
   put it into the 'visual background' on most terminal color 
   schemes.

   I tried a few mockup screens of splitting the screen 
   intelligently, and found one variant that works pretty well for 
   me. The main UI design complication is that the assembly opcodes 
   look so C source code-ish when put next to each other.

   So this is the original output:

         :        static u8 kallsyms2elf_type(char type)                                            ▒
         :        {                                                                                 ▒
         :                if (type == 'W')                                                          ▒
    0.00 :          43fd18:       mov    %rdx,%rdi                                                  ▒
         :                struct rb_node **p = &symbols->rb_node;                                   ▒
         :                struct rb_node *parent = NULL;                                            ▒
         :                const u64 ip = sym->start;                                                ▒
         :                struct symbol *s;                                                         ▒
         :                                                                                          ▒
         :                while (*p != NULL) {                                                      ▒
    0.00 :          43fd1b:       mov    (%rcx),%rdx                                                ▒
    0.00 :          43fd1e:       test   %rdx,%rdx                                                  ▒
    0.00 :          43fd21:       jne    43fd08 <map__process_kallsym_symbol+0xc8>                  ▒


   and here's the mockup:

        .                              | static u8 kallsyms2elf_type(char type)                     ▒
        .                              | {                                                          ▒
        .                              |         if (type == 'W')                                   ▒
   0.00 #  43fd18: mov    %rdx,%rdi                                                                 ▒
        .                              |         struct rb_node **p = &symbols->rb_node;            ▒
        .                              |         struct rb_node *parent = NULL;                     ▒
        .                              |         const u64 ip = sym->start;                         ▒
        .                              |         struct symbol *s;                                  ▒
        .                              |                                                            ▒
        .                              |         while (*p != NULL) {                               ▒
   0.00 #  43fd1b: mov    (%rcx),%rdx                                                               ▒
   0.00 #  43fd1e: test   %rdx,%rdx                                                                 ▒
   0.00 #  43fd21: jne    43fd08 <map__process_kallsym_symbol+0xc8>                                 ▒

There's several UI tricks:

 - typical short opcodes (80% of assembly) will fit on the left side 
   of the screen.

 - lines can still be arbitrarily long and overlap, so it's not a 
   true split screen - but the vertical helper line prefixing source 
   code lines keeps the eye focused on whichever side one intends to 
   concentrate on.

 - the first column separator uses two types of characters, '.' and 
   '#', to help the eye find the blocks of assembly.

 - we could, in addition, print assembly in grey.

 - i cut one character from the percentage column - the maximum value 
   is 100.00 so we don't need the original 7.2 format, 6.2 is enough.

We could eventually further compress the assembly display later on, 
but auto-labeling function-local labels (which are 99% of the jump 
targets). This would compress such jumps:

   0.00 #  43fd21: jne    43fd08 <map__process_kallsym_symbol+0xc8>

into:

   0.00 #  43fd21: jne    43fd08 <L3>

Thanks,

	Ingo

  parent reply	other threads:[~2011-10-19  7:16 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-10-18 21:44 Arnaldo Carvalho de Melo
2011-10-18 21:44 ` [PATCH 1/7] perf hists browser: Add missing hotkeys to the help window Arnaldo Carvalho de Melo
2011-10-18 21:44 ` [PATCH 2/7] perf tui: Catch signals to exit gracefully Arnaldo Carvalho de Melo
2011-10-18 21:44 ` [PATCH 3/7] perf ui browser: Allow initial use without navigation UI elements Arnaldo Carvalho de Melo
2011-10-18 21:44 ` [PATCH 4/7] perf hists: Don't format the percentage on hist_entry__snprintf Arnaldo Carvalho de Melo
2011-10-18 21:44 ` [PATCH 5/7] perf tui: Remove unneeded call to newtCls on startup Arnaldo Carvalho de Melo
2011-10-18 21:44 ` [PATCH 6/7] perf ui browser: Make the colors configurable and change the defaults Arnaldo Carvalho de Melo
2011-10-18 23:13   ` David Ahern
2011-10-18 23:51     ` Arnaldo Carvalho de Melo
2011-10-19  1:12       ` Arnaldo Carvalho de Melo
2011-10-19  2:41         ` [PATCH] perf ui browser: Honour the xterm colors Arnaldo Carvalho de Melo
2011-10-19  3:03           ` David Ahern
2011-10-19 13:53             ` Arnaldo Carvalho de Melo
2011-10-18 21:44 ` [PATCH 7/7] perf top tui: Give color hints just on the percentage, like on --stdio Arnaldo Carvalho de Melo
2011-10-19  7:14 ` Ingo Molnar [this message]
2011-10-19 14:14   ` [GIT PULL 0/7] perf/core fixes and improvements Arnaldo Carvalho de Melo
2011-10-19 18:23 Arnaldo Carvalho de Melo

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=20111019071457.GB2075@elte.hu \
    --to=mingo@elte.hu \
    --cc=acme@infradead.org \
    --cc=dsahern@gmail.com \
    --cc=efault@gmx.de \
    --cc=eranian@google.com \
    --cc=fweisbec@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=paulus@samba.org \
    --cc=peterz@infradead.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

all inboxes | Powered by JetHome®