Commit-ID: d5e3d747007fdb541e57ed72e020ff0b94db3470
Gitweb: http://git.kernel.org/tip/d5e3d747007fdb541e57ed72e020ff0b94db3470
Author: Arnaldo Carvalho de Melo <[email protected]>
AuthorDate: Tue, 8 Feb 2011 15:29:25 -0200
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 8 Feb 2011 15:29:25 -0200
perf annotate: Fix annotate context lines regression
The live annotation done in 'perf top' needs to limit the context before
lines that aren't filtered out by the min percent filter, if we don't do
that, the screen in a tty often is not enough for showing what is
interesting: lines with hits and a few source code lines before it.
Reported-by: Mike Galbraith <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stephane Eranian <[email protected]>
Cc: Tom Zanussi <[email protected]>
LKML-Reference: <new-submission>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/builtin-top.c | 2 +-
tools/perf/util/annotate.c | 47 +++++++++++++++++++++++++++++++++++++------
tools/perf/util/annotate.h | 3 +-
3 files changed, 43 insertions(+), 9 deletions(-)
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 7dbf22d..210c736 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -231,7 +231,7 @@ static void show_details(struct sym_entry *syme)
printf(" Events Pcnt (>=%d%%)\n", sym_pcnt_filter);
more = symbol__annotate_printf(symbol, syme->map, top.sym_evsel->idx,
- 0, sym_pcnt_filter, top.print_entries);
+ 0, sym_pcnt_filter, top.print_entries, 4);
if (top.zero)
symbol__annotate_zero_histogram(symbol, top.sym_evsel->idx);
else
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index c777bda..02976b8 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -111,7 +111,8 @@ struct objdump_line *objdump__get_next_ip_line(struct list_head *head,
static int objdump_line__print(struct objdump_line *oline, struct symbol *sym,
int evidx, u64 len, int min_pcnt,
- int printed, int max_lines)
+ int printed, int max_lines,
+ struct objdump_line *queue)
{
static const char *prev_line;
static const char *prev_color;
@@ -150,6 +151,15 @@ static int objdump_line__print(struct objdump_line *oline, struct symbol *sym,
if (max_lines && printed >= max_lines)
return 1;
+ if (queue != NULL) {
+ list_for_each_entry_from(queue, ¬es->src->source, node) {
+ if (queue == oline)
+ break;
+ objdump_line__print(queue, sym, evidx, len,
+ 0, 0, 1, NULL);
+ }
+ }
+
color = get_percent_color(percent);
/*
@@ -172,6 +182,9 @@ static int objdump_line__print(struct objdump_line *oline, struct symbol *sym,
} else if (max_lines && printed >= max_lines)
return 1;
else {
+ if (queue)
+ return -1;
+
if (!*oline->line)
printf(" :\n");
else
@@ -452,13 +465,14 @@ static void symbol__annotate_hits(struct symbol *sym, int evidx)
}
int symbol__annotate_printf(struct symbol *sym, struct map *map, int evidx,
- bool full_paths, int min_pcnt, int max_lines)
+ bool full_paths, int min_pcnt, int max_lines,
+ int context)
{
struct dso *dso = map->dso;
const char *filename = dso->long_name, *d_filename;
struct annotation *notes = symbol__annotation(sym);
- struct objdump_line *pos;
- int printed = 2;
+ struct objdump_line *pos, *queue = NULL;
+ int printed = 2, queue_len = 0;
int more = 0;
u64 len;
@@ -476,10 +490,20 @@ int symbol__annotate_printf(struct symbol *sym, struct map *map, int evidx,
symbol__annotate_hits(sym, evidx);
list_for_each_entry(pos, ¬es->src->source, node) {
+ if (context && queue == NULL) {
+ queue = pos;
+ queue_len = 0;
+ }
+
switch (objdump_line__print(pos, sym, evidx, len, min_pcnt,
- printed, max_lines)) {
+ printed, max_lines, queue)) {
case 0:
++printed;
+ if (context) {
+ printed += queue_len;
+ queue = NULL;
+ queue_len = 0;
+ }
break;
case 1:
/* filtered by max_lines */
@@ -487,7 +511,16 @@ int symbol__annotate_printf(struct symbol *sym, struct map *map, int evidx,
break;
case -1:
default:
- /* filtered by min_pcnt */
+ /*
+ * Filtered by min_pcnt or non IP lines when
+ * context != 0
+ */
+ if (!context)
+ break;
+ if (queue_len == context)
+ queue = list_entry(queue->node.next, typeof(*queue), node);
+ else
+ ++queue_len;
break;
}
}
@@ -550,7 +583,7 @@ int symbol__tty_annotate(struct symbol *sym, struct map *map, int evidx,
}
symbol__annotate_printf(sym, map, evidx, full_paths,
- min_pcnt, max_lines);
+ min_pcnt, max_lines, 0);
if (print_lines)
symbol__free_source_line(sym, len);
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index b237c86..e848803 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -78,7 +78,8 @@ void symbol__annotate_zero_histograms(struct symbol *sym);
int symbol__annotate(struct symbol *sym, struct map *map, size_t privsize);
int symbol__annotate_init(struct map *map __used, struct symbol *sym);
int symbol__annotate_printf(struct symbol *sym, struct map *map, int evidx,
- bool full_paths, int min_pcnt, int max_lines);
+ bool full_paths, int min_pcnt, int max_lines,
+ int context);
void symbol__annotate_zero_histogram(struct symbol *sym, int evidx);
void symbol__annotate_decay_histogram(struct symbol *sym, int evidx);
void objdump_line_list__purge(struct list_head *head);
On Wed, 2011-02-09 at 00:43 +0000, tip-bot for Arnaldo Carvalho de Melo
wrote:
> Commit-ID: d5e3d747007fdb541e57ed72e020ff0b94db3470
> Gitweb: http://git.kernel.org/tip/d5e3d747007fdb541e57ed72e020ff0b94db3470
> Author: Arnaldo Carvalho de Melo <[email protected]>
> AuthorDate: Tue, 8 Feb 2011 15:29:25 -0200
> Committer: Arnaldo Carvalho de Melo <[email protected]>
> CommitDate: Tue, 8 Feb 2011 15:29:25 -0200
>
> perf annotate: Fix annotate context lines regression
>
> The live annotation done in 'perf top' needs to limit the context before
> lines that aren't filtered out by the min percent filter, if we don't do
> that, the screen in a tty often is not enough for showing what is
> interesting: lines with hits and a few source code lines before it.
Looks to me like this went from regress a bit while waiting for tui
thing to progress a bit immediately :)
-Mike
Em Wed, Feb 09, 2011 at 05:06:18AM +0100, Mike Galbraith escreveu:
> On Wed, 2011-02-09 at 00:43 +0000, tip-bot for Arnaldo Carvalho de Melo wrote
> > Gitweb: http://git.kernel.org/tip/d5e3d747007fdb541e57ed72e020ff0b94db3470
> > Author: Arnaldo Carvalho de Melo <[email protected]>
> >
> > perf annotate: Fix annotate context lines regression
> >
> > The live annotation done in 'perf top' needs to limit the context before
> > lines that aren't filtered out by the min percent filter, if we don't do
> > that, the screen in a tty often is not enough for showing what is
> > interesting: lines with hits and a few source code lines before it.
>
> Looks to me like this went from regress a bit while waiting for tui
> thing to progress a bit immediately :)
:-) Can you please apply the two patches and give 'perf top --tui' live
annotation a shot? Just press enter or -> (right key) on a symbol you
want to annotate.
I'm still polishing it a bit, want to avoid trying the notes->lock on
each time record_precise_ip is called, for that I need to share the
current symbol being annotated (sym_filter_entry) with the annotate
browser, the timer should not always run, etc, but just so that I can
get some feedback on how it is shaping up, please try it and report your
impressions :-)
Also when enter is pressed and one is monitoring multiple events, a
popup menu with the event names should be presented, etc.
- Arnaldo
On Wed, 2011-02-09 at 14:41 -0200, Arnaldo Carvalho de Melo wrote:
> Em Wed, Feb 09, 2011 at 05:06:18AM +0100, Mike Galbraith escreveu:
> > On Wed, 2011-02-09 at 00:43 +0000, tip-bot for Arnaldo Carvalho de Melo wrote
> > > Gitweb: http://git.kernel.org/tip/d5e3d747007fdb541e57ed72e020ff0b94db3470
> > > Author: Arnaldo Carvalho de Melo <[email protected]>
> > >
> > > perf annotate: Fix annotate context lines regression
> > >
> > > The live annotation done in 'perf top' needs to limit the context before
> > > lines that aren't filtered out by the min percent filter, if we don't do
> > > that, the screen in a tty often is not enough for showing what is
> > > interesting: lines with hits and a few source code lines before it.
> >
> > Looks to me like this went from regress a bit while waiting for tui
> > thing to progress a bit immediately :)
>
> :-) Can you please apply the two patches and give 'perf top --tui' live
> annotation a shot? Just press enter or -> (right key) on a symbol you
> want to annotate.
>
> I'm still polishing it a bit, want to avoid trying the notes->lock on
> each time record_precise_ip is called, for that I need to share the
> current symbol being annotated (sym_filter_entry) with the annotate
> browser, the timer should not always run, etc, but just so that I can
> get some feedback on how it is shaping up, please try it and report your
> impressions :-)
Initial impression: can I have a pony too? :)
I like this a lot, great for live rummaging. One thing that would help
with large files maybe - a jump to next spot hotter than N%, but as is
is quite nice.
I managed to lock it up once, but shiny new gizmos usually do hiccup.
-Mike
On Thu, 2011-02-10 at 04:41 +0100, Mike Galbraith wrote:
> I like this a lot, great for live rummaging. One thing that would help
> with large files maybe - a jump to next spot hotter than N%, but as is
(and callgraphs, and a snapshot logger with notes insertion and and...
boy, tui thingy adds a _lot_ of potential to top:)
* Mike Galbraith <[email protected]> wrote:
> Initial impression: can I have a pony too? :)
>
> I like this a lot, great for live rummaging. One thing that would help with large
> files maybe - a jump to next spot hotter than N%, but as is is quite nice.
>
> I managed to lock it up once, but shiny new gizmos usually do hiccup.
The TUI locked up or the kernel? If it's the TUI then a gdb dump of the lockup would
be nice.
Thanks,
Ingo
On Thu, 2011-02-10 at 13:15 +0100, Ingo Molnar wrote:
> * Mike Galbraith <[email protected]> wrote:
>
> > Initial impression: can I have a pony too? :)
> >
> > I like this a lot, great for live rummaging. One thing that would help with large
> > files maybe - a jump to next spot hotter than N%, but as is is quite nice.
> >
> > I managed to lock it up once, but shiny new gizmos usually do hiccup.
>
> The TUI locked up or the kernel? If it's the TUI then a gdb dump of the lockup would
> be nice.
TUI stopped responding to my key poking. I'll see if it'll repeat
(weekend).
-Mike
Em Thu, Feb 10, 2011 at 01:43:31PM +0100, Mike Galbraith escreveu:
> On Thu, 2011-02-10 at 13:15 +0100, Ingo Molnar wrote:
> > * Mike Galbraith <[email protected]> wrote:
> >
> > > Initial impression: can I have a pony too? :)
> > >
> > > I like this a lot, great for live rummaging. One thing that would help with large
> > > files maybe - a jump to next spot hotter than N%, but as is is quite nice.
It already does that if you use TAB/shift-TAB, first one goes to the
hottest line, next one to the 2nd place, etc, shift-TAB goes the other
direction.
There are some problems with that as on the first snapshot, just after
parsing the sources, all lines have zero hits, I plan to fix that doing
something like 'top' does, which is to wait a bit, say 2 seconds
collecting samples, then show the annotation centered on the hottest
line so far.
> > > I managed to lock it up once, but shiny new gizmos usually do hiccup.
> >
> > The TUI locked up or the kernel? If it's the TUI then a gdb dump of the lockup would
> > be nice.
>
> TUI stopped responding to my key poking. I'll see if it'll repeat
> (weekend).
Probably is something I already noticed and fixed, some mutex lock
inbalance, I'll repost it soon.
- Arnaldo
Em Thu, Feb 10, 2011 at 04:51:31AM +0100, Mike Galbraith escreveu:
> On Thu, 2011-02-10 at 04:41 +0100, Mike Galbraith wrote:
>
> > I like this a lot, great for live rummaging. One thing that would help
> > with large files maybe - a jump to next spot hotter than N%, but as is
> (and callgraphs, and a snapshot logger with notes insertion and and...
Right, when testing the top TUI I keep trying to use the same menu we
have on the report, to filter by DSO or thread (the Zoom/Unzoom
operations in report TUI).
So eventually I want to get that struct sym_entry used by top merged
with hist_entry, used by report, in an efficient fashion, so that we can
merge all these interfaces.
Live callgraphs if one presses enter on a annotate line seems like a
nice thing to have, indeed, exactly as we have statically in the perf
report TUI :)
> boy, tui thingy adds a _lot_ of potential to top:)
Yeah, and all the work done to have it working on a TUI paves the way
for a GUI, but I want to have the TUI streamlined first, so that the GUI
becomes just a boilerplatish matter :-)
- Arnaldo
* Arnaldo Carvalho de Melo <[email protected]> wrote:
> Em Thu, Feb 10, 2011 at 01:43:31PM +0100, Mike Galbraith escreveu:
> > On Thu, 2011-02-10 at 13:15 +0100, Ingo Molnar wrote:
> > > * Mike Galbraith <[email protected]> wrote:
> > >
> > > > Initial impression: can I have a pony too? :)
> > > >
> > > > I like this a lot, great for live rummaging. One thing that would help with large
> > > > files maybe - a jump to next spot hotter than N%, but as is is quite nice.
>
> It already does that if you use TAB/shift-TAB, first one goes to the
> hottest line, next one to the 2nd place, etc, shift-TAB goes the other
> direction.
A minor request: could we also make it an intuitive shift+arrow-keys thing as well
perhaps? I.e. Shift will 'modify' the meaning of regular navigation of the lines
from 'next' to 'next highest', etc.
Thanks,
Ingo
On Thu, 2011-02-10 at 14:15 -0200, Arnaldo Carvalho de Melo wrote:
> Em Thu, Feb 10, 2011 at 01:43:31PM +0100, Mike Galbraith escreveu:
> > On Thu, 2011-02-10 at 13:15 +0100, Ingo Molnar wrote:
> > > * Mike Galbraith <[email protected]> wrote:
> > >
> > > > Initial impression: can I have a pony too? :)
> > > >
> > > > I like this a lot, great for live rummaging. One thing that would help with large
> > > > files maybe - a jump to next spot hotter than N%, but as is is quite nice.
>
> It already does that if you use TAB/shift-TAB, first one goes to the
> hottest line, next one to the 2nd place, etc, shift-TAB goes the other
> direction.
Cool, exactly what I was thinking. Intuitive already built in, I just
didn't know it.
> There are some problems with that as on the first snapshot, just after
> parsing the sources, all lines have zero hits, I plan to fix that doing
> something like 'top' does, which is to wait a bit, say 2 seconds
> collecting samples, then show the annotation centered on the hottest
> line so far.
Rock on :)
-Mike