Messages in this thread Patch in this message | | | From | Arnaldo Carvalho de Melo <> | Date | Tue, 10 Dec 2019 13:00:28 -0300 | Subject | Re: [PATCH v1 2/2] perf report: support hotkey to let user select any event in group for sorting |
| |
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
| |