perf report: Initial TUI using newt
in [Kernel]
|
Prev: [PATCH] usb/gadget: fix compile error on r8a66597-udc.c
Next: [PATCH 1/6] perf symbols: Bump plt synthesizing warning debug level
From: Avi Kivity on 12 Mar 2010 03:30 On 03/12/2010 01:29 AM, Arnaldo Carvalho de Melo wrote: > Em Thu, Mar 11, 2010 at 08:12:44PM -0300, Arnaldo Carvalho de Melo escreveu: > >> From: Arnaldo Carvalho de Melo<acme(a)redhat.com> >> >> Newt has widespread availability and provides a rather simple API as can be >> seen by the size of this patch. >> >> The work needed to support it will benefit other frontends too. >> >> In this initial patch it just checks if the output is a tty, if not it falls >> back to the previous behaviour, also if newt-devel/libnewt-dev is not installed >> the previous behaviour is maintaned. >> >> Pressing enter on a symbol will annotate it, ESC in the annotation window will >> return to the report symbol list. >> > For those not so curious as to apply and try it out, here is a > screenshot of it in action: > > http://tglx.de/~acme/perf-newt.png > > Looks really useful! -- Do not meddle in the internals of kernels, for they are subtle and quick to panic. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Ingo Molnar on 12 Mar 2010 04:50 * Arnaldo Carvalho de Melo <acme(a)infradead.org> wrote: > From: Arnaldo Carvalho de Melo <acme(a)redhat.com> > > Newt has widespread availability and provides a rather simple API as can be > seen by the size of this patch. > > The work needed to support it will benefit other frontends too. > > In this initial patch it just checks if the output is a tty, if not it falls > back to the previous behaviour, also if newt-devel/libnewt-dev is not > installed the previous behaviour is maintaned. > > Pressing enter on a symbol will annotate it, ESC in the annotation window will > return to the report symbol list. Very nice! a few observations. Firstly, could we perhaps make most of the interface functions GUI/TUI invariant? I.e. things like: > + if (use_browser) > + r = vfprintf(fp, fmt, args); > + else > + r = color_vfprintf(fp, color, fmt, args); should be abstracted away into a single method: r = color_vprintf(fp, color, fmt, args); where color_vprintf() knows about which current GUI front-end to use. (The old color_printf() should be renamed to ascii_color_printf() or so, and put into a front-end driver structure perhaps - instead of explicit flags.) There's a similar situation here too: > - ret = vfprintf(stderr, fmt, args); > + if (use_browser) > + ret = browser__show_help(fmt, args); > + else > + ret = vfprintf(stderr, fmt, args); Plus a few other observations about the newt TUI itself: - The most important first-impression thing in a TUI is to make it obvious to exit it. I eventually found that Escape would exit - but it would be nice to map 'Q' and 'Ctrl-C' to it as well. Nothing is more annoying than a TUI you cannot exit from. - There's still a 'perf annotate' bug that has been introduced recently, and it shows up in the TUI too. The bug is due to us passing this to objdump and grep: 18573 execve("/bin/sh", ["sh", "-c", "objdump --start-address=0xffffffff81387b36 --stop-address=0xffffffff81387b4f -dS [kernel.kallsyms]|grep -v [kernel.kallsyms]"] Look at how [kernel.kallsyms] goes unquoted to the shell, so globbing will match it on random file names in the current directory - which will then be showed by objdump, much to the surprise of the user! - I suspect we should finally make use of the .perfconfig parser and enable people to use a different front-end from Newt? Just in case they prefer ASCII. - When i hit enter on a symbol to annotate it, but the annotation fails, the TUI just does nothing currently. Instead it should print something informative (and eye-catching) into a status line at the top or the bottom of the screen, possibly printed in red characters or so. Not a separate window as that needs extra key-hits to get rid of - just a sufficiently visible status line would be perfect. There can be a few reasons why some functions can be annotated while others cannot be. - [ call-graph data is not represented yet :-) ] Anyway, very nice stuff! Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Avi Kivity on 12 Mar 2010 05:00 On 03/12/2010 11:45 AM, Ingo Molnar wrote: > > - When i hit enter on a symbol to annotate it, but the annotation fails, the TUI > just does nothing currently. Instead it should print something informative > (and eye-catching) into a status line at the top or the bottom of the > screen, possibly printed in red characters or so. Not a separate window as that > needs extra key-hits to get rid of - just a sufficiently visible status > line would be perfect. There can be a few reasons why some functions can be > annotated while others cannot be. > Alternatively, mark it as unannotatable in the first place (e.g. an annotatable symbol is a hyperlink, an unannotatable one is not). But perhaps that's too computationally expensive. -- Do not meddle in the internals of kernels, for they are subtle and quick to panic. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Arnaldo Carvalho de Melo on 12 Mar 2010 08:30 Em Fri, Mar 12, 2010 at 10:45:33AM +0100, Ingo Molnar escreveu: > > * Arnaldo Carvalho de Melo <acme(a)infradead.org> wrote: > > > From: Arnaldo Carvalho de Melo <acme(a)redhat.com> > > > > Newt has widespread availability and provides a rather simple API as can be > > seen by the size of this patch. > > > > The work needed to support it will benefit other frontends too. > > > > In this initial patch it just checks if the output is a tty, if not it falls > > back to the previous behaviour, also if newt-devel/libnewt-dev is not > > installed the previous behaviour is maintaned. > > > > Pressing enter on a symbol will annotate it, ESC in the annotation window will > > return to the report symbol list. > > Very nice! > > a few observations. Firstly, could we perhaps make most of the interface > functions GUI/TUI invariant? I.e. things like: > > > + if (use_browser) > > + r = vfprintf(fp, fmt, args); > > + else > > + r = color_vfprintf(fp, color, fmt, args); > > should be abstracted away into a single method: > > r = color_vprintf(fp, color, fmt, args); > > where color_vprintf() knows about which current GUI front-end to use. Yeah, I'll get to that, its just that I wanted to have something out as small as possible and that pointed to the places that need refactoring to properly support multiple UI frontends. > (The old color_printf() should be renamed to ascii_color_printf() or so, and > put into a front-end driver structure perhaps - instead of explicit flags.) Right > There's a similar situation here too: > > > - ret = vfprintf(stderr, fmt, args); > > + if (use_browser) > > + ret = browser__show_help(fmt, args); > > + else > > + ret = vfprintf(stderr, fmt, args); > > Plus a few other observations about the newt TUI itself: > > - The most important first-impression thing in a TUI is to make it obvious to > exit it. I eventually found that Escape would exit - but it would be nice > to map 'Q' and 'Ctrl-C' to it as well. Nothing is more annoying than a TUI > you cannot exit from. Newt's default is F12, I had to killall perf while developing and getting to know newtFormAddHotKey to exit, will add the other usual keys to exit. > - There's still a 'perf annotate' bug that has been introduced recently, and > it shows up in the TUI too. The bug is due to us passing this to objdump > and grep: > > 18573 execve("/bin/sh", ["sh", "-c", "objdump --start-address=0xffffffff81387b36 > --stop-address=0xffffffff81387b4f -dS [kernel.kallsyms]|grep -v [kernel.kallsyms]"] > > Look at how [kernel.kallsyms] goes unquoted to the shell, so globbing will > match it on random file names in the current directory - which will then be > showed by objdump, much to the surprise of the user! Will fix that, if we don't find a vmlinux file, the kernel symbols will be marked non annotable in some way. > - I suspect we should finally make use of the .perfconfig parser and enable people > to use a different front-end from Newt? Just in case they prefer ASCII. > > - When i hit enter on a symbol to annotate it, but the annotation fails, the TUI > just does nothing currently. Instead it should print something informative > (and eye-catching) into a status line at the top or the bottom of the > screen, possibly printed in red characters or so. Not a separate window as that > needs extra key-hits to get rid of - just a sufficiently visible status > line would be perfect. There can be a few reasons why some functions can be > annotated while others cannot be. Right. > - [ call-graph data is not represented yet :-) ] It will > Anyway, very nice stuff! Thanks! - Arnaldo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Arnaldo Carvalho de Melo on 12 Mar 2010 08:30
Em Fri, Mar 12, 2010 at 11:55:04AM +0200, Avi Kivity escreveu: > On 03/12/2010 11:45 AM, Ingo Molnar wrote: >> >> - When i hit enter on a symbol to annotate it, but the annotation fails, the TUI >> just does nothing currently. Instead it should print something informative >> (and eye-catching) into a status line at the top or the bottom of the >> screen, possibly printed in red characters or so. Not a separate window as that >> needs extra key-hits to get rid of - just a sufficiently visible status >> line would be perfect. There can be a few reasons why some functions can be >> annotated while others cannot be. > > Alternatively, mark it as unannotatable in the first place (e.g. an > annotatable symbol is a hyperlink, an unannotatable one is not). But > perhaps that's too computationally expensive. I'll come up with some way to do that, changing the color or adding markings to highlight the top functions, like the stdio one does with red for te top functions, etc is needed. - Arnaldo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |