Hi Ingo,
Please consider pulling from the "perf" branch in:
git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux-2.6
Best Regards,
Arnaldo Carvalho de Melo (2):
perf newt: Add a "Zoom into foo.so DSO" and reverse operations
perf TUI: Add a "Zoom into COMM(PID) thread" and reverse operations
tools/perf/util/newt.c | 185 ++++++++++++++++++++++++++++++++++++++++--------
tools/perf/util/sort.h | 10 ++-
2 files changed, 162 insertions(+), 33 deletions(-)
From: Arnaldo Carvalho de Melo <[email protected]>
Now one can press the right arrow key and in addition to being able to
filter by DSO, filter out by thread too, or a combination of both
filters.
With this one can start collecting events for the whole system, then
focus on a subset of the collected data quickly.
Cc: Avi Kivity <[email protected]>
Cc: Frédéric Weisbecker <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Paul Mackerras <[email protected]>
LKML-Reference: <new-submission>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/newt.c | 82 +++++++++++++++++++++++++++++++++++++++++++-----
tools/perf/util/sort.h | 9 ++++-
2 files changed, 81 insertions(+), 10 deletions(-)
diff --git a/tools/perf/util/newt.c b/tools/perf/util/newt.c
index bbf725d..6d6e022 100644
--- a/tools/perf/util/newt.c
+++ b/tools/perf/util/newt.c
@@ -490,6 +490,11 @@ static int hist_browser__populate(struct hist_browser *self, struct rb_root *his
return 0;
}
+enum hist_filter {
+ HIST_FILTER__DSO,
+ HIST_FILTER__THREAD,
+};
+
static u64 hists__filter_by_dso(struct rb_root *hists, struct dso *dso,
u64 *session_total)
{
@@ -502,10 +507,10 @@ static u64 hists__filter_by_dso(struct rb_root *hists, struct dso *dso,
struct hist_entry *h = rb_entry(nd, struct hist_entry, rb_node);
if (dso != NULL && (h->ms.map == NULL || h->ms.map->dso != dso)) {
- h->filtered = true;
+ h->filtered |= (1 << HIST_FILTER__DSO);
continue;
}
- h->filtered = false;
+ h->filtered &= ~(1 << HIST_FILTER__DSO);
++nr_hists;
*session_total += h->count;
}
@@ -513,12 +518,54 @@ static u64 hists__filter_by_dso(struct rb_root *hists, struct dso *dso,
return nr_hists;
}
+static u64 hists__filter_by_thread(struct rb_root *hists, const struct thread *thread,
+ u64 *session_total)
+{
+ struct rb_node *nd;
+ u64 nr_hists = 0;
+
+ *session_total = 0;
+
+ for (nd = rb_first(hists); nd; nd = rb_next(nd)) {
+ struct hist_entry *h = rb_entry(nd, struct hist_entry, rb_node);
+
+ if (thread != NULL && h->thread != thread) {
+ h->filtered |= (1 << HIST_FILTER__THREAD);
+ continue;
+ }
+ h->filtered &= ~(1 << HIST_FILTER__THREAD);
+ ++nr_hists;
+ *session_total += h->count;
+ }
+
+ return nr_hists;
+}
+
+static struct thread *hist_browser__selected_thread(struct hist_browser *self)
+{
+ int *indexes;
+
+ if (!symbol_conf.use_callchain)
+ goto out;
+
+ indexes = newtCheckboxTreeFindItem(self->tree, (void *)self->selection);
+ if (indexes) {
+ bool is_hist_entry = indexes[1] == NEWT_ARG_LAST;
+ free(indexes);
+ if (is_hist_entry)
+ goto out;
+ }
+ return NULL;
+out:
+ return *(struct thread **)(self->selection + 1);
+}
+
int perf_session__browse_hists(struct rb_root *hists, u64 nr_hists,
u64 session_total, const char *helpline,
const char *input_name)
{
struct newtExitStruct es;
- bool dso_filtered = false;
+ bool dso_filtered = false, thread_filtered = false;
int err = -1;
struct hist_browser *browser = hist_browser__new();
@@ -531,9 +578,10 @@ int perf_session__browse_hists(struct rb_root *hists, u64 nr_hists,
goto out;
while (1) {
+ const struct thread *thread;
char *options[16];
int nr_options = 0, choice = 0, i,
- annotate = -2, zoom_dso = -2;
+ annotate = -2, zoom_dso = -2, zoom_thread = -2;
newtFormRun(browser->form, &es);
if (es.reason == NEWT_EXIT_HOTKEY) {
@@ -561,6 +609,13 @@ int perf_session__browse_hists(struct rb_root *hists, u64 nr_hists,
browser->selection->map->dso->short_name)) > 0)
zoom_dso = nr_options++;
+ thread = hist_browser__selected_thread(browser);
+ if (thread != NULL &&
+ asprintf(&options[nr_options], "Zoom %s %s(%d) thread",
+ (thread_filtered ? "out of" : "into"),
+ (thread->comm_set ? thread->comm : ""), thread->pid) > 0)
+ zoom_thread = nr_options++;
+
options[nr_options++] = (char *)"Exit";
choice = popup_menu(nr_options, options);
@@ -570,6 +625,9 @@ int perf_session__browse_hists(struct rb_root *hists, u64 nr_hists,
if (choice == nr_options - 1)
break;
+
+ if (choice == -1)
+ continue;
do_annotate:
if (choice == annotate) {
if (browser->selection->map->dso->origin == DSO__ORIG_KERNEL) {
@@ -581,13 +639,21 @@ do_annotate:
}
map_symbol__annotate_browser(browser->selection,
input_name);
- } if (choice == zoom_dso) {
- hists__filter_by_dso(hists,
- dso_filtered ? NULL : browser->selection->map->dso,
- &session_total);
+ } else if (choice == zoom_dso) {
+ nr_hists = hists__filter_by_dso(hists,
+ (dso_filtered ? NULL :
+ browser->selection->map->dso),
+ &session_total);
dso_filtered = !dso_filtered;
if (hist_browser__populate(browser, hists, nr_hists, session_total) < 0)
goto out;
+ } else if (choice == zoom_thread) {
+ nr_hists = hists__filter_by_thread(hists,
+ (thread_filtered ? NULL : thread),
+ &session_total);
+ thread_filtered = !thread_filtered;
+ if (hist_browser__populate(browser, hists, nr_hists, session_total) < 0)
+ goto out;
}
}
err = 0;
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index dce79d3..2046ab7 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -44,11 +44,16 @@ extern enum sort_type sort__first_dimension;
struct hist_entry {
struct rb_node rb_node;
u64 count;
- struct thread *thread;
+ /*
+ * XXX WARNING!
+ * thread _has_ to come after ms, see
+ * map_symbol__thread in util/newt.c
+ */
struct map_symbol ms;
+ struct thread *thread;
u64 ip;
char level;
- bool filtered;
+ u8 filtered;
struct symbol *parent;
union {
unsigned long position;
--
1.6.2.5
From: Arnaldo Carvalho de Melo <[email protected]>
Clicking on -> will bring as one of the popup menu options a "Zoom into
CURRENT DSO", i.e. CURRENT will be replaced by the name of the DSO in
the current line.
Choosing this option will filter out all samples that didn't took place
in a symbol in this DSO.
After that the option reverts to "Zoom out of CURRENT DSO", to allow
going back to the more compreensive view, not filtered by DSO.
Future similar operations will include zooming into a particular thread,
COMM, CPU, "last minute", "last N usecs", etc.
Cc: Frédéric Weisbecker <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Paul Mackerras <[email protected]>
LKML-Reference: <new-submission>
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/util/newt.c | 119 +++++++++++++++++++++++++++++++++++------------
tools/perf/util/sort.h | 3 +-
2 files changed, 90 insertions(+), 32 deletions(-)
diff --git a/tools/perf/util/newt.c b/tools/perf/util/newt.c
index c93bc2a..bbf725d 100644
--- a/tools/perf/util/newt.c
+++ b/tools/perf/util/newt.c
@@ -94,7 +94,7 @@ static newtComponent newt_form__new(void)
return self;
}
-static int popup_menu(int argc, const char *argv[])
+static int popup_menu(int argc, char * const argv[])
{
struct newtExitStruct es;
int i, rc = -1, max_len = 5;
@@ -397,22 +397,8 @@ static struct hist_browser *hist_browser__new(void)
{
struct hist_browser *self = malloc(sizeof(*self));
- if (self != NULL) {
- char seq[] = ".";
- int rows;
-
- newtGetScreenSize(NULL, &rows);
-
- if (symbol_conf.use_callchain)
- self->tree = newtCheckboxTreeMulti(0, 0, rows - 5, seq,
- NEWT_FLAG_SCROLL);
- else
- self->tree = newtListbox(0, 0, rows - 5,
- (NEWT_FLAG_SCROLL |
- NEWT_FLAG_RETURNEXIT));
- newtComponentAddCallback(self->tree, hist_browser__selection,
- &self->selection);
- }
+ if (self != NULL)
+ self->form = NULL;
return self;
}
@@ -431,6 +417,30 @@ static int hist_browser__populate(struct hist_browser *self, struct rb_root *his
struct ui_progress *progress;
struct rb_node *nd;
u64 curr_hist = 0;
+ char seq[] = ".";
+ char str[256];
+
+ if (self->form) {
+ newtFormDestroy(self->form);
+ newtPopWindow();
+ }
+
+ snprintf(str, sizeof(str), "Samples: %Ld ",
+ session_total);
+ newtDrawRootText(0, 0, str);
+
+ newtGetScreenSize(NULL, &rows);
+
+ if (symbol_conf.use_callchain)
+ self->tree = newtCheckboxTreeMulti(0, 0, rows - 5, seq,
+ NEWT_FLAG_SCROLL);
+ else
+ self->tree = newtListbox(0, 0, rows - 5,
+ (NEWT_FLAG_SCROLL |
+ NEWT_FLAG_RETURNEXIT));
+
+ newtComponentAddCallback(self->tree, hist_browser__selection,
+ &self->selection);
progress = ui_progress__new("Adding entries to the browser...", nr_hists);
if (progress == NULL)
@@ -439,7 +449,12 @@ static int hist_browser__populate(struct hist_browser *self, struct rb_root *his
idx = 0;
for (nd = rb_first(hists); nd; nd = rb_next(nd)) {
struct hist_entry *h = rb_entry(nd, struct hist_entry, rb_node);
- int len = hist_entry__append_browser(h, self->tree, session_total);
+ int len;
+
+ if (h->filtered)
+ continue;
+
+ len = hist_entry__append_browser(h, self->tree, session_total);
if (len > max_len)
max_len = len;
if (symbol_conf.use_callchain)
@@ -463,6 +478,9 @@ static int hist_browser__populate(struct hist_browser *self, struct rb_root *his
newtCenteredWindow(max_len + (symbol_conf.use_callchain ? 5 : 0),
rows - 5, "Report");
self->form = newt_form__new();
+ if (self->form == NULL)
+ return -1;
+
newtFormAddHotKey(self->form, 'A');
newtFormAddHotKey(self->form, 'a');
newtFormAddHotKey(self->form, NEWT_KEY_RIGHT);
@@ -472,29 +490,50 @@ static int hist_browser__populate(struct hist_browser *self, struct rb_root *his
return 0;
}
+static u64 hists__filter_by_dso(struct rb_root *hists, struct dso *dso,
+ u64 *session_total)
+{
+ struct rb_node *nd;
+ u64 nr_hists = 0;
+
+ *session_total = 0;
+
+ for (nd = rb_first(hists); nd; nd = rb_next(nd)) {
+ struct hist_entry *h = rb_entry(nd, struct hist_entry, rb_node);
+
+ if (dso != NULL && (h->ms.map == NULL || h->ms.map->dso != dso)) {
+ h->filtered = true;
+ continue;
+ }
+ h->filtered = false;
+ ++nr_hists;
+ *session_total += h->count;
+ }
+
+ return nr_hists;
+}
+
int perf_session__browse_hists(struct rb_root *hists, u64 nr_hists,
u64 session_total, const char *helpline,
const char *input_name)
{
struct newtExitStruct es;
- char str[1024];
+ bool dso_filtered = false;
int err = -1;
struct hist_browser *browser = hist_browser__new();
if (browser == NULL)
return -1;
- snprintf(str, sizeof(str), "Samples: %Ld", session_total);
- newtDrawRootText(0, 0, str);
newtPushHelpLine(helpline);
if (hist_browser__populate(browser, hists, nr_hists, session_total) < 0)
goto out;
while (1) {
- char annotate[512];
- const char *options[2];
- int nr_options = 0, choice = 0;
+ char *options[16];
+ int nr_options = 0, choice = 0, i,
+ annotate = -2, zoom_dso = -2;
newtFormRun(browser->form, &es);
if (es.reason == NEWT_EXIT_HOTKEY) {
@@ -510,18 +549,29 @@ int perf_session__browse_hists(struct rb_root *hists, u64 nr_hists,
}
}
- if (browser->selection->sym != NULL) {
- snprintf(annotate, sizeof(annotate),
- "Annotate %s", browser->selection->sym->name);
- options[nr_options++] = annotate;
- }
+ if (browser->selection->sym != NULL &&
+ asprintf(&options[nr_options], "Annotate %s",
+ browser->selection->sym->name) > 0)
+ annotate = nr_options++;
+
+ if (browser->selection->map != NULL &&
+ asprintf(&options[nr_options], "Zoom %s %s DSO",
+ dso_filtered ? "out of" : "into",
+ (browser->selection->map->dso->kernel ? "the Kernel" :
+ browser->selection->map->dso->short_name)) > 0)
+ zoom_dso = nr_options++;
+
+ options[nr_options++] = (char *)"Exit";
- options[nr_options++] = "Exit";
choice = popup_menu(nr_options, options);
+
+ for (i = 0; i < nr_options - 1; ++i)
+ free(options[i]);
+
if (choice == nr_options - 1)
break;
do_annotate:
- if (browser->selection->sym != NULL && choice >= 0) {
+ if (choice == annotate) {
if (browser->selection->map->dso->origin == DSO__ORIG_KERNEL) {
newtPopHelpLine();
newtPushHelpLine("No vmlinux file found, can't "
@@ -531,6 +581,13 @@ do_annotate:
}
map_symbol__annotate_browser(browser->selection,
input_name);
+ } if (choice == zoom_dso) {
+ hists__filter_by_dso(hists,
+ dso_filtered ? NULL : browser->selection->map->dso,
+ &session_total);
+ dso_filtered = !dso_filtered;
+ if (hist_browser__populate(browser, hists, nr_hists, session_total) < 0)
+ goto out;
}
}
err = 0;
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index 5bf2b74..dce79d3 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -48,7 +48,8 @@ struct hist_entry {
struct map_symbol ms;
u64 ip;
char level;
- struct symbol *parent;
+ bool filtered;
+ struct symbol *parent;
union {
unsigned long position;
struct hist_entry *pair;
--
1.6.2.5
* Arnaldo Carvalho de Melo <[email protected]> wrote:
> Hi Ingo,
>
> Please consider pulling from the "perf" branch in:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux-2.6
>
> Best Regards,
>
> Arnaldo Carvalho de Melo (2):
> perf newt: Add a "Zoom into foo.so DSO" and reverse operations
> perf TUI: Add a "Zoom into COMM(PID) thread" and reverse operations
>
> tools/perf/util/newt.c | 185 ++++++++++++++++++++++++++++++++++++++++--------
> tools/perf/util/sort.h | 10 ++-
> 2 files changed, 162 insertions(+), 33 deletions(-)
Pulled, thanks Arnaldo!
There's still a few basic usability glitches that i've noticed:
- it's not clear how to zoom out, once zoomed in
- there's no help screen and no help line for people to see what can be
done on a given screen. While it's good that the intuitive things work
by default (arrow keys, escape, enter, etc.), there should also be additional
visual information about what can be done.
- i think double escape should exit, right now it stays at the 'are you sure'
screen
Ingo
Em Sun, Apr 04, 2010 at 11:58:27AM +0200, Ingo Molnar escreveu:
> * Arnaldo Carvalho de Melo <[email protected]> wrote:
> > Arnaldo Carvalho de Melo (2):
> > perf newt: Add a "Zoom into foo.so DSO" and reverse operations
> > perf TUI: Add a "Zoom into COMM(PID) thread" and reverse operations
>
> Pulled, thanks Arnaldo!
>
> There's still a few basic usability glitches that i've noticed:
>
> - it's not clear how to zoom out, once zoomed in
I'll use the title line to store the name of the DSO and Thread and the
help line to state that to zoom out, press -> + zoom out, and I think
that it is a good idea to use 'd' and 'D' to zoom in/out of DSOs and 't'
and 'T' to zoom in/out of DSOs.
> - there's no help screen and no help line for people to see what can be
> done on a given screen. While it's good that the intuitive things work
> by default (arrow keys, escape, enter, etc.), there should also be additional
> visual information about what can be done.
F1 and textboxes with a short description of what can be done given for
the first time or till the user presses "Suppress these info boxes" will
be added.
> - i think double escape should exit, right now it stays at the 'are you sure'
> screen
Here I thought that something had to be done to avoid that accidentally
pressing ESC two times or having "Yes" as default option, trowing away
possibly a long processed session, what do others think about it?
I think that ESC + "Yes" as a default may be ok, as two keys need to be
pressed (ESC + Enter), enough warning/confirmation, I gues, right?
> Ingo
* Arnaldo Carvalho de Melo <[email protected]> wrote:
> Em Sun, Apr 04, 2010 at 11:58:27AM +0200, Ingo Molnar escreveu:
> > * Arnaldo Carvalho de Melo <[email protected]> wrote:
> > > Arnaldo Carvalho de Melo (2):
> > > perf newt: Add a "Zoom into foo.so DSO" and reverse operations
> > > perf TUI: Add a "Zoom into COMM(PID) thread" and reverse operations
> >
> > Pulled, thanks Arnaldo!
> >
> > There's still a few basic usability glitches that i've noticed:
> >
> > - it's not clear how to zoom out, once zoomed in
>
> I'll use the title line to store the name of the DSO and Thread and the help
> line to state that to zoom out, press -> + zoom out, and I think that it is
> a good idea to use 'd' and 'D' to zoom in/out of DSOs and 't' and 'T' to
> zoom in/out of DSOs.
Cool, a help line will do the trick i'm sure. The only problem situation is
when there is zero information about how to proceed - some people exit and
dont ever come back in that case :-)
> > - there's no help screen and no help line for people to see what can be
> > done on a given screen. While it's good that the intuitive things work
> > by default (arrow keys, escape, enter, etc.), there should also be additional
> > visual information about what can be done.
>
> F1 and textboxes with a short description of what can be done given for the
> first time or till the user presses "Suppress these info boxes" will be
> added.
Maybe we need 'h' and '?' in addition of F1, as F1 is often already mapped and
caught by the terminal (it is so with gnome-terminal).
>
> > - i think double escape should exit, right now it stays at the 'are you sure'
> > screen
>
> Here I thought that something had to be done to avoid that accidentally
> pressing ESC two times or having "Yes" as default option, trowing away
> possibly a long processed session, what do others think about it?
Yeah, i think you are right.
> I think that ESC + "Yes" as a default may be ok, as two keys need to be
> pressed (ESC + Enter), enough warning/confirmation, I gues, right?
Ok, that would work!
Thanks,
Ingo
Em Sun, Apr 04, 2010 at 09:05:12PM +0200, Ingo Molnar escreveu:
>
> * Arnaldo Carvalho de Melo <[email protected]> wrote:
>
> > Em Sun, Apr 04, 2010 at 11:58:27AM +0200, Ingo Molnar escreveu:
> > > * Arnaldo Carvalho de Melo <[email protected]> wrote:
> > > > Arnaldo Carvalho de Melo (2):
> > > > perf newt: Add a "Zoom into foo.so DSO" and reverse operations
> > > > perf TUI: Add a "Zoom into COMM(PID) thread" and reverse operations
> > >
> > > Pulled, thanks Arnaldo!
> > >
> > > There's still a few basic usability glitches that i've noticed:
> > >
> > > - it's not clear how to zoom out, once zoomed in
> >
> > I'll use the title line to store the name of the DSO and Thread and the help
> > line to state that to zoom out, press -> + zoom out, and I think that it is
> > a good idea to use 'd' and 'D' to zoom in/out of DSOs and 't' and 'T' to
> > zoom in/out of DSOs.
>
> Cool, a help line will do the trick i'm sure. The only problem situation is
> when there is zero information about how to proceed - some people exit and
> dont ever come back in that case :-)
Yeah, first impression is always important to take into account, but
difficult to get it right, what I'm trying, as much as possible, is to
keep key assignments as intuitive as possible, but early adopters are
key in this regard, so please do speak up!
> > > - there's no help screen and no help line for people to see what can be
> > > done on a given screen. While it's good that the intuitive things work
> > > by default (arrow keys, escape, enter, etc.), there should also be additional
> > > visual information about what can be done.
> >
> > F1 and textboxes with a short description of what can be done given for the
> > first time or till the user presses "Suppress these info boxes" will be
> > added.
>
> Maybe we need 'h' and '?' in addition of F1, as F1 is often already mapped and
> caught by the terminal (it is so with gnome-terminal).
Right, h and ? will be mapped too, even I'm not being aware that those
are associated with asking for help, but the
ask-for-help-via-F1-taken-by-your-xterm-app problem is well known and we
have to provide a workaround for that :-)
> >
> > > - i think double escape should exit, right now it stays at the 'are you sure'
> > > screen
> >
> > Here I thought that something had to be done to avoid that accidentally
> > pressing ESC two times or having "Yes" as default option, trowing away
> > possibly a long processed session, what do others think about it?
>
> Yeah, i think you are right.
>
> > I think that ESC + "Yes" as a default may be ok, as two keys need to be
> > pressed (ESC + Enter), enough warning/confirmation, I gues, right?
>
> Ok, that would work!
Excellent, I almost got the title line for in place filters bullet done
before being dragged by family to go out fishing, will fishing, oops,
finish that tomorrow :-)
- Arnaldo