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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®