* [PATCH 2/4] perf ui browser: Introduce ui_browser__input_window
2012-03-13 6:14 [PATCH 1/4] perf hists: Add hists__filter_by_symbol Namhyung Kim
@ 2012-03-13 6:14 ` Namhyung Kim
2012-03-13 6:14 ` [PATCH 3/4] perf ui browser: Add 's' key to filter by symbol name Namhyung Kim
` (2 subsequent siblings)
3 siblings, 0 replies; 9+ messages in thread
From: Namhyung Kim @ 2012-03-13 6:14 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Namhyung Kim, LKML
The ui_browser__input_window() function is to get user's key input.
Current implementation can handle maximum 49 characters.
Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
---
tools/perf/util/ui/browser.h | 2 +
tools/perf/util/ui/keysyms.h | 2 +
tools/perf/util/ui/util.c | 78 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 82 insertions(+), 0 deletions(-)
diff --git a/tools/perf/util/ui/browser.h b/tools/perf/util/ui/browser.h
index 84d761b730c1..6ee82f60feaf 100644
--- a/tools/perf/util/ui/browser.h
+++ b/tools/perf/util/ui/browser.h
@@ -49,6 +49,8 @@ int ui_browser__warning(struct ui_browser *browser, int timeout,
const char *format, ...);
int ui_browser__help_window(struct ui_browser *browser, const char *text);
bool ui_browser__dialog_yesno(struct ui_browser *browser, const char *text);
+int ui_browser__input_window(const char *title, const char *text, char *input,
+ const char *exit_msg, int delay_sec);
void ui_browser__argv_seek(struct ui_browser *browser, off_t offset, int whence);
unsigned int ui_browser__argv_refresh(struct ui_browser *browser);
diff --git a/tools/perf/util/ui/keysyms.h b/tools/perf/util/ui/keysyms.h
index 3458b1985761..809eca5707fa 100644
--- a/tools/perf/util/ui/keysyms.h
+++ b/tools/perf/util/ui/keysyms.h
@@ -16,6 +16,8 @@
#define K_TAB '\t'
#define K_UNTAB SL_KEY_UNTAB
#define K_UP SL_KEY_UP
+#define K_BKSPC 0x7f
+#define K_DEL SL_KEY_DELETE
/* Not really keys */
#define K_TIMER -1
diff --git a/tools/perf/util/ui/util.c b/tools/perf/util/ui/util.c
index 45daa7c41dad..360f43fd5400 100644
--- a/tools/perf/util/ui/util.c
+++ b/tools/perf/util/ui/util.c
@@ -69,6 +69,84 @@ int ui__popup_menu(int argc, char * const argv[])
return popup_menu__run(&menu);
}
+int ui_browser__input_window(const char *title, const char *text, char *input,
+ const char *exit_msg, int delay_secs)
+{
+ int x, y, len, key;
+ int max_len = 60, nr_lines = 0;
+ static char buf[50];
+ const char *t;
+
+ t = text;
+ while (1) {
+ const char *sep = strchr(t, '\n');
+
+ if (sep == NULL)
+ sep = strchr(t, '\0');
+ len = sep - t;
+ if (max_len < len)
+ max_len = len;
+ ++nr_lines;
+ if (*sep == '\0')
+ break;
+ t = sep + 1;
+ }
+
+ max_len += 2;
+ nr_lines += 8;
+ y = SLtt_Screen_Rows / 2 - nr_lines / 2;
+ x = SLtt_Screen_Cols / 2 - max_len / 2;
+
+ SLsmg_set_color(0);
+ SLsmg_draw_box(y, x++, nr_lines, max_len);
+ if (title) {
+ SLsmg_gotorc(y, x + 1);
+ SLsmg_write_string((char *)title);
+ }
+ SLsmg_gotorc(++y, x);
+ nr_lines -= 7;
+ max_len -= 2;
+ SLsmg_write_wrapped_string((unsigned char *)text, y, x,
+ nr_lines, max_len, 1);
+ y += nr_lines + 1;
+ SLsmg_set_color(0);
+ SLsmg_draw_box(y - 1, x + 1, 3, max_len - 2);
+
+ SLsmg_gotorc(y + 3, x);
+ SLsmg_write_nstring((char *)exit_msg, max_len);
+ SLsmg_refresh();
+
+ x += 2;
+ len = 0;
+ key = ui__getch(delay_secs);
+ while (key != K_TIMER && key != K_ENTER && key != K_ESC) {
+ if (key == K_BKSPC) {
+ if (len == 0)
+ goto next_key;
+ SLsmg_gotorc(y, x + --len);
+ SLsmg_write_char(' ');
+ } else {
+ buf[len] = key;
+ SLsmg_gotorc(y, x + len++);
+ SLsmg_write_char(key);
+ }
+ SLsmg_refresh();
+
+ /* XXX more graceful overflow handling needed */
+ if (len == sizeof(buf) - 1) {
+ ui_helpline__push("maximum size of symbol name reached!");
+ key = K_ENTER;
+ break;
+ }
+next_key:
+ key = ui__getch(delay_secs);
+ }
+
+ buf[len] = '\0';
+ strncpy(input, buf, len+1);
+ return key;
+}
+
int ui__question_window(const char *title, const char *text,
const char *exit_msg, int delay_secs)
{
--
1.7.9
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 3/4] perf ui browser: Add 's' key to filter by symbol name
2012-03-13 6:14 [PATCH 1/4] perf hists: Add hists__filter_by_symbol Namhyung Kim
2012-03-13 6:14 ` [PATCH 2/4] perf ui browser: Introduce ui_browser__input_window Namhyung Kim
@ 2012-03-13 6:14 ` Namhyung Kim
2012-03-13 6:14 ` [PATCH 4/4] perf report: Treat an argument as a symbol filter Namhyung Kim
2012-03-15 17:34 ` [PATCH 1/4] perf hists: Add hists__filter_by_symbol Arnaldo Carvalho de Melo
3 siblings, 0 replies; 9+ messages in thread
From: Namhyung Kim @ 2012-03-13 6:14 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Namhyung Kim, LKML
Now user can enter symbol name interested via ui_browser__input_window,
and perf can process it using hists__filter_by_symbol(). Giving empty
symbol (by pressing 's' followed by ENTER) will disable the filtering.
Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
---
tools/perf/util/ui/browsers/hists.c | 14 +++++++++++++-
1 files changed, 13 insertions(+), 1 deletions(-)
diff --git a/tools/perf/util/ui/browsers/hists.c b/tools/perf/util/ui/browsers/hists.c
index bfba0490c098..8b6abcbb71c5 100644
--- a/tools/perf/util/ui/browsers/hists.c
+++ b/tools/perf/util/ui/browsers/hists.c
@@ -863,6 +863,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
struct hist_browser *browser = hist_browser__new(self);
struct pstack *fstack;
int key = -1;
+ char buf[64];
if (browser == NULL)
return -1;
@@ -915,6 +916,16 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
goto zoom_dso;
case 't':
goto zoom_thread;
+ case 's':
+ if (ui_browser__input_window("Symbol to show",
+ "Please enter the name of symbol you want to see",
+ buf, "ENTER: OK, ESC: Cancel",
+ delay_secs * 2) == K_ENTER) {
+ self->symbol_filter_str = *buf ? buf : NULL;
+ hists__filter_by_symbol(self);
+ hist_browser__reset(browser);
+ }
+ continue;
case K_F1:
case 'h':
case '?':
@@ -932,7 +943,8 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
"C Collapse all callchains\n"
"E Expand all callchains\n"
"d Zoom into current DSO\n"
- "t Zoom into current Thread");
+ "t Zoom into current Thread\n"
+ "s Filter symbol by name");
continue;
case K_ENTER:
case K_RIGHT:
--
1.7.9
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 4/4] perf report: Treat an argument as a symbol filter
2012-03-13 6:14 [PATCH 1/4] perf hists: Add hists__filter_by_symbol Namhyung Kim
2012-03-13 6:14 ` [PATCH 2/4] perf ui browser: Introduce ui_browser__input_window Namhyung Kim
2012-03-13 6:14 ` [PATCH 3/4] perf ui browser: Add 's' key to filter by symbol name Namhyung Kim
@ 2012-03-13 6:14 ` Namhyung Kim
2012-03-13 6:21 ` Ingo Molnar
2012-03-15 17:34 ` [PATCH 1/4] perf hists: Add hists__filter_by_symbol Arnaldo Carvalho de Melo
3 siblings, 1 reply; 9+ messages in thread
From: Namhyung Kim @ 2012-03-13 6:14 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Namhyung Kim, LKML
As Ingo requested, it'd be better off treating first (and the only)
argument as a symbol filter, so that user doesn't need to input the
symbol on the dialog window.
Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
---
tools/perf/builtin-report.c | 19 ++++++++++++++-----
1 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 25d34d483e49..62c4a11bcf3e 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -50,6 +50,7 @@ struct perf_report {
const char *pretty_printing_style;
symbol_filter_t annotate_init;
const char *cpu_list;
+ const char *symbol_filter_str;
DECLARE_BITMAP(cpu_bitmap, MAX_NR_CPUS);
};
@@ -315,6 +316,9 @@ static int __cmd_report(struct perf_report *rep)
list_for_each_entry(pos, &session->evlist->entries, node) {
struct hists *hists = &pos->hists;
+ if (pos->idx == 0)
+ hists->symbol_filter_str = rep->symbol_filter_str;
+
hists__collapse_resort(hists);
hists__output_resort(hists);
nr_samples += hists->stats.nr_events[PERF_RECORD_SAMPLE];
@@ -586,11 +590,16 @@ int cmd_report(int argc, const char **argv, const char *prefix __used)
} else
symbol_conf.exclude_other = false;
- /*
- * Any (unrecognized) arguments left?
- */
- if (argc)
- usage_with_options(report_usage, options);
+ if (argc) {
+ /*
+ * Special case: if there's an argument left then assume tha
+ * it's a symbol filter:
+ */
+ if (argc > 1)
+ usage_with_options(report_usage, options);
+
+ report.symbol_filter_str = argv[0];
+ }
sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list, "dso", stdout);
sort_entry__setup_elide(&sort_comm, symbol_conf.comm_list, "comm", stdout);
--
1.7.9
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 4/4] perf report: Treat an argument as a symbol filter
2012-03-13 6:14 ` [PATCH 4/4] perf report: Treat an argument as a symbol filter Namhyung Kim
@ 2012-03-13 6:21 ` Ingo Molnar
2012-03-13 13:40 ` Peter Zijlstra
0 siblings, 1 reply; 9+ messages in thread
From: Ingo Molnar @ 2012-03-13 6:21 UTC (permalink / raw)
To: Namhyung Kim
Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Paul Mackerras,
Namhyung Kim, LKML
* Namhyung Kim <namhyung.kim@lge.com> wrote:
> As Ingo requested, it'd be better off treating first (and the only)
> argument as a symbol filter, so that user doesn't need to input the
> symbol on the dialog window.
>
> Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
> ---
> tools/perf/builtin-report.c | 19 ++++++++++++++-----
> 1 files changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> index 25d34d483e49..62c4a11bcf3e 100644
> --- a/tools/perf/builtin-report.c
> +++ b/tools/perf/builtin-report.c
> @@ -50,6 +50,7 @@ struct perf_report {
> const char *pretty_printing_style;
> symbol_filter_t annotate_init;
> const char *cpu_list;
> + const char *symbol_filter_str;
> DECLARE_BITMAP(cpu_bitmap, MAX_NR_CPUS);
> };
>
> @@ -315,6 +316,9 @@ static int __cmd_report(struct perf_report *rep)
> list_for_each_entry(pos, &session->evlist->entries, node) {
> struct hists *hists = &pos->hists;
>
> + if (pos->idx == 0)
> + hists->symbol_filter_str = rep->symbol_filter_str;
> +
> hists__collapse_resort(hists);
> hists__output_resort(hists);
> nr_samples += hists->stats.nr_events[PERF_RECORD_SAMPLE];
> @@ -586,11 +590,16 @@ int cmd_report(int argc, const char **argv, const char *prefix __used)
> } else
> symbol_conf.exclude_other = false;
>
> - /*
> - * Any (unrecognized) arguments left?
> - */
> - if (argc)
> - usage_with_options(report_usage, options);
> + if (argc) {
> + /*
> + * Special case: if there's an argument left then assume tha
s/tha/that
> + * it's a symbol filter:
> + */
> + if (argc > 1)
> + usage_with_options(report_usage, options);
> +
> + report.symbol_filter_str = argv[0];
> + }
Are people fine with this special case?
To play the devil's advocate it uses up valuable perf report
command line real estate and we probably cannot change this
workflow in the future, once people get used to it.
For example doing this would preclude us from adding subsystem
subcommands to perf report in the future, such as:
$ perf report sched
$ perf report kvm
...
OTOH have 'perf sched' and 'perf kvm' and maybe those are better
provided as:
$ perf kvm report
$ perf sched report
:-)
Thanks,
Ingo
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 4/4] perf report: Treat an argument as a symbol filter
2012-03-13 6:21 ` Ingo Molnar
@ 2012-03-13 13:40 ` Peter Zijlstra
2012-03-13 15:24 ` Ingo Molnar
0 siblings, 1 reply; 9+ messages in thread
From: Peter Zijlstra @ 2012-03-13 13:40 UTC (permalink / raw)
To: Ingo Molnar
Cc: Namhyung Kim, Arnaldo Carvalho de Melo, Paul Mackerras,
Namhyung Kim, LKML
On Tue, 2012-03-13 at 07:21 +0100, Ingo Molnar wrote:
> To play the devil's advocate it uses up valuable perf report
> command line real estate and we probably cannot change this
> workflow in the future, once people get used to it.
I would indeed suggest to make it a proper argument and add argument
parsing to report like we have for all the other commands.
Say something like: perf report --symbol-filter="foobar"
and maybe an appropriate short version.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 4/4] perf report: Treat an argument as a symbol filter
2012-03-13 13:40 ` Peter Zijlstra
@ 2012-03-13 15:24 ` Ingo Molnar
2012-03-15 0:39 ` [PATCH] perf report: Add --symbol-filter option Namhyung Kim
0 siblings, 1 reply; 9+ messages in thread
From: Ingo Molnar @ 2012-03-13 15:24 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Namhyung Kim, Arnaldo Carvalho de Melo, Paul Mackerras,
Namhyung Kim, LKML
* Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
> On Tue, 2012-03-13 at 07:21 +0100, Ingo Molnar wrote:
> > To play the devil's advocate it uses up valuable perf report
> > command line real estate and we probably cannot change this
> > workflow in the future, once people get used to it.
>
> I would indeed suggest to make it a proper argument and add
> argument parsing to report like we have for all the other
> commands.
>
> Say something like: perf report --symbol-filter="foobar" and
> maybe an appropriate short version.
No objection against also having a command line option, but I
really like the Git way of doing things like:
git remote update tip
where it's several layers of commands plus arguments, with no
silly command switch names to remember.
Thanks,
Ingo
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] perf report: Add --symbol-filter option
2012-03-13 15:24 ` Ingo Molnar
@ 2012-03-15 0:39 ` Namhyung Kim
0 siblings, 0 replies; 9+ messages in thread
From: Namhyung Kim @ 2012-03-15 0:39 UTC (permalink / raw)
To: Ingo Molnar
Cc: Peter Zijlstra, Arnaldo Carvalho de Melo, Paul Mackerras, LKML
Add new --symbol-filter command line option to set appropriate
filter string. Its short version is missing as I couldn't find
an ideal one and --filter option of perf record also has no
short version.
Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
---
tools/perf/builtin-report.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 62c4a11bcf3e..8784d5c77110 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -499,6 +499,8 @@ int cmd_report(int argc, const char **argv, const char *prefix __used)
"only consider symbols in these comms"),
OPT_STRING('S', "symbols", &symbol_conf.sym_list_str, "symbol[,symbol...]",
"only consider these symbols"),
+ OPT_STRING(0, "symbol-filter", &report.symbol_filter_str, "filter",
+ "only show symbols that (partially) match with this filter"),
OPT_STRING('w', "column-widths", &symbol_conf.col_width_list_str,
"width[,width...]",
"don't try to adjust column width, use these fixed values"),
--
1.7.9
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/4] perf hists: Add hists__filter_by_symbol
2012-03-13 6:14 [PATCH 1/4] perf hists: Add hists__filter_by_symbol Namhyung Kim
` (2 preceding siblings ...)
2012-03-13 6:14 ` [PATCH 4/4] perf report: Treat an argument as a symbol filter Namhyung Kim
@ 2012-03-15 17:34 ` Arnaldo Carvalho de Melo
3 siblings, 0 replies; 9+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-03-15 17:34 UTC (permalink / raw)
To: Namhyung Kim
Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar, Namhyung Kim, LKML
Em Tue, Mar 13, 2012 at 03:14:51PM +0900, Namhyung Kim escreveu:
> This function will be used for simple (sub-)string matching
> filter based on user input.
Can you check if these apply as well? I fear they wont.
- Arnaldo
> Signed-off-by: Namhyung Kim <namhyung.kim@lge.com>
> ---
> tools/perf/util/hist.c | 35 +++++++++++++++++++++++++++++++++++
> tools/perf/util/hist.h | 2 ++
> 2 files changed, 37 insertions(+), 0 deletions(-)
>
> diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
> index 6f505d1abac7..021b49c498d5 100644
> --- a/tools/perf/util/hist.c
> +++ b/tools/perf/util/hist.c
> @@ -10,11 +10,14 @@ static bool hists__filter_entry_by_dso(struct hists *hists,
> struct hist_entry *he);
> static bool hists__filter_entry_by_thread(struct hists *hists,
> struct hist_entry *he);
> +static bool hists__filter_entry_by_symbol(struct hists *hists,
> + struct hist_entry *he);
>
> enum hist_filter {
> HIST_FILTER__DSO,
> HIST_FILTER__THREAD,
> HIST_FILTER__PARENT,
> + HIST_FILTER__SYMBOL,
> };
>
> struct callchain_param callchain_param = {
> @@ -352,6 +355,7 @@ static void hists__apply_filters(struct hists *hists, struct hist_entry *he)
> {
> hists__filter_entry_by_dso(hists, he);
> hists__filter_entry_by_thread(hists, he);
> + hists__filter_entry_by_symbol(hists, he);
> }
>
> static void __hists__collapse_resort(struct hists *hists, bool threaded)
> @@ -1179,6 +1183,37 @@ void hists__filter_by_thread(struct hists *hists)
> }
> }
>
> +static bool hists__filter_entry_by_symbol(struct hists *hists,
> + struct hist_entry *he)
> +{
> + if (hists->symbol_filter_str != NULL &&
> + (!he->ms.sym || strstr(he->ms.sym->name,
> + hists->symbol_filter_str) == NULL)) {
> + he->filtered |= (1 << HIST_FILTER__SYMBOL);
> + return true;
> + }
> +
> + return false;
> +}
> +
> +void hists__filter_by_symbol(struct hists *hists)
> +{
> + struct rb_node *nd;
> +
> + hists->nr_entries = hists->stats.total_period = 0;
> + hists->stats.nr_events[PERF_RECORD_SAMPLE] = 0;
> + hists__reset_col_len(hists);
> +
> + for (nd = rb_first(&hists->entries); nd; nd = rb_next(nd)) {
> + struct hist_entry *h = rb_entry(nd, struct hist_entry, rb_node);
> +
> + if (hists__filter_entry_by_symbol(hists, h))
> + continue;
> +
> + hists__remove_entry_filter(hists, h, HIST_FILTER__SYMBOL);
> + }
> +}
> +
> int hist_entry__inc_addr_samples(struct hist_entry *he, int evidx, u64 ip)
> {
> return symbol__inc_addr_samples(he->ms.sym, he->ms.map, evidx, ip);
> diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
> index 48e5acd1e862..020d3477cabc 100644
> --- a/tools/perf/util/hist.h
> +++ b/tools/perf/util/hist.h
> @@ -57,6 +57,7 @@ struct hists {
> const struct thread *thread_filter;
> const struct dso *dso_filter;
> const char *uid_filter_str;
> + const char *symbol_filter_str;
> pthread_mutex_t lock;
> struct events_stats stats;
> u64 event_stream;
> @@ -96,6 +97,7 @@ int hist_entry__annotate(struct hist_entry *self, size_t privsize);
>
> void hists__filter_by_dso(struct hists *hists);
> void hists__filter_by_thread(struct hists *hists);
> +void hists__filter_by_symbol(struct hists *hists);
>
> u16 hists__col_len(struct hists *self, enum hist_column col);
> void hists__set_col_len(struct hists *self, enum hist_column col, u16 len);
> --
> 1.7.9
^ permalink raw reply [flat|nested] 9+ messages in thread