From: Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>
To: Jin Yao <yao.jin@linux.intel.com>
Cc: jolsa@kernel.org, peterz@infradead.org, mingo@redhat.com,
alexander.shishkin@linux.intel.com, Linux-kernel@vger.kernel.org,
ak@linux.intel.com, kan.liang@intel.com, yao.jin@intel.com
Subject: Re: [PATCH v1 2/2] perf report: support hotkey to let user select any event in group for sorting
Date: Tue, 10 Dec 2019 13:00:28 -0300 [thread overview]
Message-ID: <20191210160028.GA28084@kernel.org> (raw)
In-Reply-To: <20191210083207.31569-2-yao.jin@linux.intel.com>
Em Tue, Dec 10, 2019 at 04:32:07PM +0800, Jin Yao escreveu:
> When performing "perf report --group", it shows the event group information
> together. In previous patch, we have supported a new option "--group-sort-idx"
> to sort the output by the event at the index n in event group.
>
> It would be nice if we can use a hotkey in browser to select a event
> for sorting.
>
> For example,
>
> # perf report --group
>
> Samples: 12K of events 'cpu/instructions,period=2000003/, cpu/cpu-cycles,period=200003/, ...
> Overhead Command Shared Object Symbol
> 92.19% 98.68% 0.00% 93.30% mgen mgen [.] LOOP1
> 3.12% 0.29% 0.00% 0.16% gsd-color libglib-2.0.so.0.5600.4 [.] 0x0000000000049515
> 1.56% 0.03% 0.00% 0.04% gsd-color libglib-2.0.so.0.5600.4 [.] 0x00000000000494b7
> 1.56% 0.01% 0.00% 0.00% gsd-color libglib-2.0.so.0.5600.4 [.] 0x00000000000494ce
> 1.56% 0.00% 0.00% 0.00% mgen [kernel.kallsyms] [k] task_tick_fair
> 0.00% 0.15% 0.00% 0.04% perf [kernel.kallsyms] [k] smp_call_function_single
> 0.00% 0.13% 0.00% 6.08% swapper [kernel.kallsyms] [k] intel_idle
> 0.00% 0.03% 0.00% 0.00% gsd-color libglib-2.0.so.0.5600.4 [.] g_main_context_check
> 0.00% 0.03% 0.00% 0.00% swapper [kernel.kallsyms] [k] apic_timer_interrupt
> 0.00% 0.03% 0.00% 0.00% swapper [kernel.kallsyms] [k] check_preempt_curr
>
> When user press hotkey '3' (event index, starting from 0), it indicates
> to sort output by the fourth event in group.
>
> Samples: 12K of events 'cpu/instructions,period=2000003/, cpu/cpu-cycles,period=200003/, ...
> Overhead Command Shared Object Symbol
> 92.19% 98.68% 0.00% 93.30% mgen mgen [.] LOOP1
> 0.00% 0.13% 0.00% 6.08% swapper [kernel.kallsyms] [k] intel_idle
> 3.12% 0.29% 0.00% 0.16% gsd-color libglib-2.0.so.0.5600.4 [.] 0x0000000000049515
> 0.00% 0.00% 0.00% 0.06% swapper [kernel.kallsyms] [k] hrtimer_start_range_ns
> 1.56% 0.03% 0.00% 0.04% gsd-color libglib-2.0.so.0.5600.4 [.] 0x00000000000494b7
> 0.00% 0.15% 0.00% 0.04% perf [kernel.kallsyms] [k] smp_call_function_single
> 0.00% 0.00% 0.00% 0.02% mgen [kernel.kallsyms] [k] update_curr
> 0.00% 0.00% 0.00% 0.02% mgen [kernel.kallsyms] [k] apic_timer_interrupt
> 0.00% 0.00% 0.00% 0.02% mgen [kernel.kallsyms] [k] native_apic_msr_eoi_write
> 0.00% 0.00% 0.00% 0.02% mgen [kernel.kallsyms] [k] __update_load_avg_se
>
> Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
> ---
> tools/perf/builtin-report.c | 4 ++--
> tools/perf/ui/browsers/hists.c | 21 ++++++++++++++++++++-
> tools/perf/ui/keysyms.h | 1 +
> 3 files changed, 23 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> index 729cf7611d8a..02178fc54d67 100644
> --- a/tools/perf/builtin-report.c
> +++ b/tools/perf/builtin-report.c
> @@ -635,7 +635,7 @@ static int report__browse_hists(struct report *rep)
> * Usually "ret" is the last pressed key, and we only
> * care if the key notifies us to switch data file.
> */
> - if (ret != K_SWITCH_INPUT_DATA)
> + if (ret != K_SWITCH_INPUT_DATA && ret != K_RELOAD)
> ret = 0;
> break;
> case 2:
> @@ -1538,7 +1538,7 @@ int cmd_report(int argc, const char **argv)
> sort__setup_elide(stdout);
>
> ret = __cmd_report(&report);
> - if (ret == K_SWITCH_INPUT_DATA) {
> + if (ret == K_SWITCH_INPUT_DATA || ret == K_RELOAD) {
> perf_session__delete(session);
> goto repeat;
> } else
> diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> index d4d3558fdef4..1de2456f27c3 100644
> --- a/tools/perf/ui/browsers/hists.c
> +++ b/tools/perf/ui/browsers/hists.c
> @@ -2876,7 +2876,8 @@ static int perf_evsel__hists_browse(struct evsel *evsel, int nr_events,
> "s Switch to another data file in PWD\n"
> "t Zoom into current Thread\n"
> "V Verbose (DSO names in callchains, etc)\n"
> - "/ Filter symbol by name";
> + "/ Filter symbol by name\n"
> + "0-9 Sort by event n in group";
> static const char top_help[] = HIST_BROWSER_HELP_COMMON
> "P Print histograms to perf.hist.N\n"
> "t Zoom into current Thread\n"
> @@ -2937,6 +2938,24 @@ static int perf_evsel__hists_browse(struct evsel *evsel, int nr_events,
> * go to the next or previous
> */
> goto out_free_stack;
> + case '0':
case '0' ... '9':
works as well, for instance, in the kernel sources we have:
[acme@quaco perf]$ grep 'case.*\.\..*:' */*.c
block/bio.c: case 2 ... 4:
block/bio.c: case 5 ... 16:
block/bio.c: case 17 ... 64:
block/bio.c: case 65 ... 128:
block/bio.c: case 129 ... BIO_MAX_PAGES:
block/sed-opal.c: case 0xbfff ... 0xffff:
fs/binfmt_elf.c: case PT_LOPROC ... PT_HIPROC:
fs/binfmt_elf.c: case PT_LOPROC ... PT_HIPROC:
kernel/audit.c: case AUDIT_FIRST_USER_MSG ... AUDIT_LAST_USER_MSG:
kernel/audit.c: case AUDIT_FIRST_USER_MSG2 ... AUDIT_LAST_USER_MSG2:
kernel/audit.c: case AUDIT_FIRST_USER_MSG ... AUDIT_LAST_USER_MSG:
kernel/audit.c: case AUDIT_FIRST_USER_MSG2 ... AUDIT_LAST_USER_MSG2:
[acme@quaco perf]$
> + case '1':
> + case '2':
> + case '3':
> + case '4':
> + case '5':
> + case '6':
> + case '7':
> + case '8':
> + case '9':
> + symbol_conf.group_sort_idx = key - '0';
> + if (!symbol_conf.event_group ||
> + symbol_conf.group_sort_idx >= evsel->core.nr_members) {
> + continue;
Better to put something on the helpline as:
The max event group index to sort is N!
And if symbol_conf.event_group isn't set, something like:
Sort by index only available with group events!
> + }
> +
> + key = K_RELOAD;
> + goto out_free_stack;
> case 'a':
> if (!hists__has(hists, sym)) {
> ui_browser__warning(&browser->b, delay_secs * 2,
> diff --git a/tools/perf/ui/keysyms.h b/tools/perf/ui/keysyms.h
> index fbfac29077f2..04cc4e5c031f 100644
> --- a/tools/perf/ui/keysyms.h
> +++ b/tools/perf/ui/keysyms.h
> @@ -25,5 +25,6 @@
> #define K_ERROR -2
> #define K_RESIZE -3
> #define K_SWITCH_INPUT_DATA -4
> +#define K_RELOAD -5
Can you please split the K_RELOAD logic from this patch?
> #endif /* _PERF_KEYSYMS_H_ */
> --
> 2.17.1
--
- Arnaldo
next prev parent reply other threads:[~2019-12-10 16:00 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-12-10 8:32 [PATCH v1 1/2] perf report: Change sort order by a specified event in group Jin Yao
2019-12-10 8:32 ` [PATCH v1 2/2] perf report: support hotkey to let user select any event in group for sorting Jin Yao
2019-12-10 16:00 ` Arnaldo Carvalho de Melo [this message]
2019-12-11 7:06 ` Jin, Yao
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=20191210160028.GA28084@kernel.org \
--to=arnaldo.melo@gmail.com \
--cc=Linux-kernel@vger.kernel.org \
--cc=ak@linux.intel.com \
--cc=alexander.shishkin@linux.intel.com \
--cc=jolsa@kernel.org \
--cc=kan.liang@intel.com \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=yao.jin@intel.com \
--cc=yao.jin@linux.intel.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.