* [PATCH v3 1/3] perf report: Change sort order by a specified event in group
@ 2019-12-12 12:33 Jin Yao
2019-12-12 12:33 ` [PATCH v3 2/3] perf report: Support a new key to reload the browser Jin Yao
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Jin Yao @ 2019-12-12 12:33 UTC (permalink / raw)
To: acme, jolsa, peterz, mingo, alexander.shishkin
Cc: Linux-kernel, ak, kan.liang, yao.jin, Jin Yao
When performing "perf report --group", it shows the event group information
together. By default, the output is sorted by the first event in group.
It would be nice for user to select any event for sorting. This patch
introduces a new option "--group-sort-idx" to sort the output by the
event at the index n in event group.
For example,
Before:
# perf report --group --stdio
# To display the perf.data header info, please use --header/--header-only options.
#
#
# Total Lost Samples: 0
#
# Samples: 12K of events 'cpu/instructions,period=2000003/, cpu/cpu-cycles,period=200003/, BR_MISP_RETIRED.ALL_BRANCHES:pp, cpu/event=0xc0,umask=1,cmask=1,
# Event count (approx.): 6451235635
#
# 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
...
After:
# perf report --group --stdio --group-sort-idx 3
# To display the perf.data header info, please use --header/--header-only options.
#
#
# Total Lost Samples: 0
#
# Samples: 12K of events 'cpu/instructions,period=2000003/, cpu/cpu-cycles,period=200003/, BR_MISP_RETIRED.ALL_BRANCHES:pp, cpu/event=0xc0,umask=1,cmask=1,
# Event count (approx.): 6451235635
#
# 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
0.00% 0.00% 0.00% 0.02% mgen [kernel.kallsyms] [k] scheduler_tick
Now the output is sorted by the fourth event in group.
v3:
---
Refine the code in __hpp__group_sort_idx().
Before:
for (i = 1; i < nr_members; i++) {
if (i == idx) {
ret = field_cmp(fields_a[i], fields_b[i]);
if (ret)
goto out;
}
}
After:
if (idx >= 1 && idx < nr_members) {
ret = field_cmp(fields_a[idx], fields_b[idx]);
if (ret)
goto out;
}
v2:
---
No change
Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
---
tools/perf/Documentation/perf-report.txt | 4 +
tools/perf/builtin-report.c | 10 +++
tools/perf/ui/hist.c | 108 +++++++++++++++++++----
tools/perf/util/symbol_conf.h | 1 +
4 files changed, 108 insertions(+), 15 deletions(-)
diff --git a/tools/perf/Documentation/perf-report.txt b/tools/perf/Documentation/perf-report.txt
index 8dbe2119686a..9ade613ef020 100644
--- a/tools/perf/Documentation/perf-report.txt
+++ b/tools/perf/Documentation/perf-report.txt
@@ -371,6 +371,10 @@ OPTIONS
Show event group information together. It forces group output also
if there are no groups defined in data file.
+--group-sort-idx::
+ Sort the output by the event at the index n in group. If n is invalid,
+ sort by the first event. WARNING: This should be used with --group.
+
--demangle::
Demangle symbol names to human readable form. It's enabled by default,
disable with --no-demangle.
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 387311c67264..729cf7611d8a 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -1210,6 +1210,10 @@ int cmd_report(int argc, const char **argv)
"Show a column with the sum of periods"),
OPT_BOOLEAN_SET(0, "group", &symbol_conf.event_group, &report.group_set,
"Show event group information together"),
+ OPT_INTEGER(0, "group-sort-idx", &symbol_conf.group_sort_idx,
+ "Sort the output by the event at the index n in group. "
+ "If n is invalid, sort by the first event. "
+ "WARNING: use only with --group."),
OPT_CALLBACK_NOOPT('b', "branch-stack", &branch_mode, "",
"use branch records for per branch histogram filling",
parse_branch_mode),
@@ -1302,6 +1306,12 @@ int cmd_report(int argc, const char **argv)
return -EINVAL;
}
+ if (symbol_conf.group_sort_idx &&
+ (!symbol_conf.event_group || !report.group_set)) {
+ parse_options_usage(NULL, options, "group-sort-idx", 0);
+ return -EINVAL;
+ }
+
if (report.inverted_callchain)
callchain_param.order = ORDER_CALLER;
if (symbol_conf.cumulate_callchain && !callchain_param.order_set)
diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index f73675500061..d921b32f9d2a 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -151,15 +151,104 @@ static int field_cmp(u64 field_a, u64 field_b)
return 0;
}
+static int pair_fields_alloc(struct hist_entry *a, struct hist_entry *b,
+ hpp_field_fn get_field, int nr_members,
+ u64 **fields_a, u64 **fields_b)
+{
+ struct evsel *evsel;
+ struct hist_entry *pair;
+ u64 *fa, *fb;
+ int ret = -1;
+
+ fa = calloc(nr_members, sizeof(*fa));
+ fb = calloc(nr_members, sizeof(*fb));
+
+ if (!fa || !fb)
+ goto out;
+
+ list_for_each_entry(pair, &a->pairs.head, pairs.node) {
+ evsel = hists_to_evsel(pair->hists);
+ fa[perf_evsel__group_idx(evsel)] = get_field(pair);
+ }
+
+ list_for_each_entry(pair, &b->pairs.head, pairs.node) {
+ evsel = hists_to_evsel(pair->hists);
+ fb[perf_evsel__group_idx(evsel)] = get_field(pair);
+ }
+
+ *fields_a = fa;
+ *fields_b = fb;
+ ret = 0;
+
+out:
+ if (ret != 0) {
+ free(fa);
+ free(fb);
+ *fields_a = NULL;
+ *fields_b = NULL;
+ }
+
+ return ret;
+}
+
+static int __hpp__group_sort_idx(struct hist_entry *a, struct hist_entry *b,
+ hpp_field_fn get_field, int idx)
+{
+ struct evsel *evsel = hists_to_evsel(a->hists);
+ u64 *fields_a, *fields_b;
+ int cmp, nr_members, ret, i;
+
+ cmp = field_cmp(get_field(a), get_field(b));
+ if (!perf_evsel__is_group_event(evsel))
+ return cmp;
+
+ nr_members = evsel->core.nr_members;
+ ret = pair_fields_alloc(a, b, get_field, nr_members,
+ &fields_a, &fields_b);
+ if (ret) {
+ ret = cmp;
+ goto out;
+ }
+
+ if (idx >= 1 && idx < nr_members) {
+ ret = field_cmp(fields_a[idx], fields_b[idx]);
+ if (ret)
+ goto out;
+ }
+
+ if (cmp) {
+ ret = cmp;
+ goto out;
+ }
+
+ for (i = 1; i < nr_members; i++) {
+ if (i != idx) {
+ ret = field_cmp(fields_a[i], fields_b[i]);
+ if (ret)
+ goto out;
+ }
+ }
+
+out:
+ free(fields_a);
+ free(fields_b);
+
+ return ret;
+}
+
static int __hpp__sort(struct hist_entry *a, struct hist_entry *b,
hpp_field_fn get_field)
{
s64 ret;
int i, nr_members;
struct evsel *evsel;
- struct hist_entry *pair;
u64 *fields_a, *fields_b;
+ if (symbol_conf.group_sort_idx && symbol_conf.event_group) {
+ return __hpp__group_sort_idx(a, b, get_field,
+ symbol_conf.group_sort_idx);
+ }
+
ret = field_cmp(get_field(a), get_field(b));
if (ret || !symbol_conf.event_group)
return ret;
@@ -169,22 +258,11 @@ static int __hpp__sort(struct hist_entry *a, struct hist_entry *b,
return ret;
nr_members = evsel->core.nr_members;
- fields_a = calloc(nr_members, sizeof(*fields_a));
- fields_b = calloc(nr_members, sizeof(*fields_b));
-
- if (!fields_a || !fields_b)
+ i = pair_fields_alloc(a, b, get_field, nr_members,
+ &fields_a, &fields_b);
+ if (i)
goto out;
- list_for_each_entry(pair, &a->pairs.head, pairs.node) {
- evsel = hists_to_evsel(pair->hists);
- fields_a[perf_evsel__group_idx(evsel)] = get_field(pair);
- }
-
- list_for_each_entry(pair, &b->pairs.head, pairs.node) {
- evsel = hists_to_evsel(pair->hists);
- fields_b[perf_evsel__group_idx(evsel)] = get_field(pair);
- }
-
for (i = 1; i < nr_members; i++) {
ret = field_cmp(fields_a[i], fields_b[i]);
if (ret)
diff --git a/tools/perf/util/symbol_conf.h b/tools/perf/util/symbol_conf.h
index 10f1ec3e0349..b916afb95ec5 100644
--- a/tools/perf/util/symbol_conf.h
+++ b/tools/perf/util/symbol_conf.h
@@ -73,6 +73,7 @@ struct symbol_conf {
const char *symfs;
int res_sample;
int pad_output_len_dso;
+ int group_sort_idx;
};
extern struct symbol_conf symbol_conf;
--
2.17.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v3 2/3] perf report: Support a new key to reload the browser
2019-12-12 12:33 [PATCH v3 1/3] perf report: Change sort order by a specified event in group Jin Yao
@ 2019-12-12 12:33 ` Jin Yao
2019-12-12 12:33 ` [PATCH v3 3/3] perf report: support hotkey to let user select any event for sorting Jin Yao
2019-12-16 7:31 ` [PATCH v3 1/3] perf report: Change sort order by a specified event in group Jiri Olsa
2 siblings, 0 replies; 7+ messages in thread
From: Jin Yao @ 2019-12-12 12:33 UTC (permalink / raw)
To: acme, jolsa, peterz, mingo, alexander.shishkin
Cc: Linux-kernel, ak, kan.liang, yao.jin, Jin Yao
Sometimes we may need to reload the browser to update the output since
some options are changed.
This patch creates a new key K_RELOAD. Once the __cmd_report() returns
K_RELOAD, it would repeat the whole process, such as, read samples from
data file, sort the data and display in the browser.
v3:
---
No change.
v2:
---
This is a new patch created in v2.
Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
---
tools/perf/builtin-report.c | 4 ++--
tools/perf/ui/keysyms.h | 1 +
2 files changed, 3 insertions(+), 2 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/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
#endif /* _PERF_KEYSYMS_H_ */
--
2.17.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v3 3/3] perf report: support hotkey to let user select any event for sorting
2019-12-12 12:33 [PATCH v3 1/3] perf report: Change sort order by a specified event in group Jin Yao
2019-12-12 12:33 ` [PATCH v3 2/3] perf report: Support a new key to reload the browser Jin Yao
@ 2019-12-12 12:33 ` Jin Yao
2019-12-16 7:31 ` [PATCH v3 1/3] perf report: Change sort order by a specified event in group Jiri Olsa
2 siblings, 0 replies; 7+ messages in thread
From: Jin Yao @ 2019-12-12 12:33 UTC (permalink / raw)
To: acme, jolsa, peterz, mingo, alexander.shishkin
Cc: Linux-kernel, ak, kan.liang, yao.jin, Jin Yao
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
to sort.
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 forth 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
v3:
---
No change
v2:
---
1. Report warning at helpline when index is invalid.
2. Report warning at helpline when it's not group event.
3. Use "case '0' ... '9'" to refine the code
4. Split K_RELOAD implementation to another patch.
Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
---
tools/perf/ui/browsers/hists.c | 25 ++++++++++++++++++++++++-
1 file changed, 24 insertions(+), 1 deletion(-)
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index d4d3558fdef4..b9288a1385dd 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,28 @@ static int perf_evsel__hists_browse(struct evsel *evsel, int nr_events,
* go to the next or previous
*/
goto out_free_stack;
+ case '0' ... '9':
+ if (!symbol_conf.event_group ||
+ evsel->core.nr_members < 2) {
+ snprintf(buf, sizeof(buf),
+ "Sort by index only available with group events!");
+ helpline = buf;
+ continue;
+ }
+
+ symbol_conf.group_sort_idx = key - '0';
+
+ if (symbol_conf.group_sort_idx >= evsel->core.nr_members) {
+ snprintf(buf, sizeof(buf),
+ "Max event group index to sort is %d (index from 0 to %d)",
+ evsel->core.nr_members - 1,
+ evsel->core.nr_members - 1);
+ helpline = buf;
+ continue;
+ }
+
+ key = K_RELOAD;
+ goto out_free_stack;
case 'a':
if (!hists__has(hists, sym)) {
ui_browser__warning(&browser->b, delay_secs * 2,
--
2.17.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v3 1/3] perf report: Change sort order by a specified event in group
2019-12-12 12:33 [PATCH v3 1/3] perf report: Change sort order by a specified event in group Jin Yao
2019-12-12 12:33 ` [PATCH v3 2/3] perf report: Support a new key to reload the browser Jin Yao
2019-12-12 12:33 ` [PATCH v3 3/3] perf report: support hotkey to let user select any event for sorting Jin Yao
@ 2019-12-16 7:31 ` Jiri Olsa
2019-12-17 1:47 ` Jin, Yao
2 siblings, 1 reply; 7+ messages in thread
From: Jiri Olsa @ 2019-12-16 7:31 UTC (permalink / raw)
To: Jin Yao
Cc: acme, jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak,
kan.liang, yao.jin
On Thu, Dec 12, 2019 at 08:33:35PM +0800, Jin Yao wrote:
SNIP
>
> Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
> ---
> tools/perf/Documentation/perf-report.txt | 4 +
> tools/perf/builtin-report.c | 10 +++
> tools/perf/ui/hist.c | 108 +++++++++++++++++++----
> tools/perf/util/symbol_conf.h | 1 +
> 4 files changed, 108 insertions(+), 15 deletions(-)
>
> diff --git a/tools/perf/Documentation/perf-report.txt b/tools/perf/Documentation/perf-report.txt
> index 8dbe2119686a..9ade613ef020 100644
> --- a/tools/perf/Documentation/perf-report.txt
> +++ b/tools/perf/Documentation/perf-report.txt
> @@ -371,6 +371,10 @@ OPTIONS
> Show event group information together. It forces group output also
> if there are no groups defined in data file.
>
> +--group-sort-idx::
> + Sort the output by the event at the index n in group. If n is invalid,
> + sort by the first event. WARNING: This should be used with --group.
--group in record or report?
you can also create groups with -e '{}', not just --group option
I wonder you could check early on 'evlist->nr_groups' and fail
if there's no group defined if the option is enabled
also what happens when we have more groups?
I think the option is easy to use as it is, just needs to be explained
consequences for more groups with different amount of events
SNIP
> +
> +static int __hpp__group_sort_idx(struct hist_entry *a, struct hist_entry *b,
> + hpp_field_fn get_field, int idx)
> +{
> + struct evsel *evsel = hists_to_evsel(a->hists);
> + u64 *fields_a, *fields_b;
> + int cmp, nr_members, ret, i;
> +
> + cmp = field_cmp(get_field(a), get_field(b));
> + if (!perf_evsel__is_group_event(evsel))
> + return cmp;
> +
> + nr_members = evsel->core.nr_members;
> + ret = pair_fields_alloc(a, b, get_field, nr_members,
> + &fields_a, &fields_b);
> + if (ret) {
> + ret = cmp;
> + goto out;
> + }
> +
> + if (idx >= 1 && idx < nr_members) {
do we need to continue here if idx is out of the limit?
I'm not sure but mybe in that case the comparison above
should be enough?
thanks,
jirka
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 1/3] perf report: Change sort order by a specified event in group
2019-12-16 7:31 ` [PATCH v3 1/3] perf report: Change sort order by a specified event in group Jiri Olsa
@ 2019-12-17 1:47 ` Jin, Yao
2019-12-17 9:06 ` Jiri Olsa
0 siblings, 1 reply; 7+ messages in thread
From: Jin, Yao @ 2019-12-17 1:47 UTC (permalink / raw)
To: Jiri Olsa
Cc: acme, jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak,
kan.liang, yao.jin
On 12/16/2019 3:31 PM, Jiri Olsa wrote:
> On Thu, Dec 12, 2019 at 08:33:35PM +0800, Jin Yao wrote:
>
> SNIP
>
>>
>> Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
>> ---
>> tools/perf/Documentation/perf-report.txt | 4 +
>> tools/perf/builtin-report.c | 10 +++
>> tools/perf/ui/hist.c | 108 +++++++++++++++++++----
>> tools/perf/util/symbol_conf.h | 1 +
>> 4 files changed, 108 insertions(+), 15 deletions(-)
>>
>> diff --git a/tools/perf/Documentation/perf-report.txt b/tools/perf/Documentation/perf-report.txt
>> index 8dbe2119686a..9ade613ef020 100644
>> --- a/tools/perf/Documentation/perf-report.txt
>> +++ b/tools/perf/Documentation/perf-report.txt
>> @@ -371,6 +371,10 @@ OPTIONS
>> Show event group information together. It forces group output also
>> if there are no groups defined in data file.
>>
>> +--group-sort-idx::
>> + Sort the output by the event at the index n in group. If n is invalid,
>> + sort by the first event. WARNING: This should be used with --group.
>
> --group in record or report?
>
This --group is in perf-report. So even if it's not created with -e '{}'
in perf-record, it still supports to show event information together.
> you can also create groups with -e '{}', not just --group option
>
> I wonder you could check early on 'evlist->nr_groups' and fail
> if there's no group defined if the option is enabled
>
Maybe we don't need to check that because it supports the case of no
group defined.
For example,
perf record -e cycles,instructions
perf report --group --group-sort-idx 1 --stdio
# Overhead Command Shared Object Symbol
# ................ ....... .................
.............................
#
0.00% 39.68% swapper [kernel.kallsyms] [k] file_free_rcu
0.00% 31.85% swapper [kernel.kallsyms] [k] ___perf_sw_event
3.96% 4.68% swapper [kernel.kallsyms] [k]
tick_nohz_idle_stop_tick
0.00% 4.49% perf [kernel.kallsyms] [k] update_load_avg
0.00% 4.49% swapper [kernel.kallsyms] [k] tcp_ack
4.06% 3.75% swapper [kernel.kallsyms] [k] update_rt_rq_load_avg
3.54% 3.71% swapper [kernel.kallsyms] [k] update_blocked_averages
1.88% 2.06% swapper [kernel.kallsyms] [k] load_balance
The output is sorted by the second event.
> also what happens when we have more groups?
>
Let me take an example.
perf record -e '{cycles,instructions}' -e '{branches,cache-misses}' -a
perf report --group --group-sort-idx 1 --stdio
# Samples: 116 of events 'anon group { cycles, instructions }'
# Event count (approx.): 17552917
#
# Overhead Command Shared Object Symbol
# ................ ............... .................
...............................
#
0.00% 13.81% perf [kernel.kallsyms] [k] number
0.00% 12.50% swapper [kernel.kallsyms] [k] cpuidle_select
0.00% 12.02% sleep [kernel.kallsyms] [k]
filemap_map_pages
0.00% 11.40% perf [kernel.kallsyms] [k] rcu_all_qs
0.00% 11.30% perf [kernel.kallsyms] [k] alloc_set_pte
0.00% 10.54% swapper [kernel.kallsyms] [k]
timekeeping_advance
0.00% 9.89% kworker/5:2-mm_ [kernel.kallsyms] [k]
update_rt_rq_load_avg
0.00% 7.86% rcu_sched [kernel.kallsyms] [k]
finish_task_switch
......
# Samples: 100 of events 'anon group { branches, cache-misses }'
# Event count (approx.): 1214391
#
# Overhead Command Shared Object Symbol
# ................ ............... .......................
...............................
#
0.00% 23.13% perf [kernel.kallsyms] [k]
ext4_dirty_inode
0.00% 18.66% sleep [kernel.kallsyms] [k]
free_pipe_info
0.00% 10.74% perf [kernel.kallsyms] [k]
unmap_page_range
0.00% 9.35% sleep [kernel.kallsyms] [k]
__wake_up_common_lock
0.00% 7.71% wpa_supplicant libc-2.27.so [.]
cfree@GLIBC_2.2.5
0.00% 6.55% thermald libglib-2.0.so.0.5600.4 [.]
g_mutex_lock
0.00% 3.43% perf [kernel.kallsyms] [k]
_raw_spin_lock
0.00% 3.12% migration/0 [kernel.kallsyms] [k]
kthread_should_stop
0.00% 2.87% migration/1 [kernel.kallsyms] [k]
__bitmap_and
0.00% 2.37% migration/3 [kernel.kallsyms] [k]
update_rt_rq_load_avg
0.00% 2.01% perf [kernel.kallsyms] [k]
security_inode_permission
0.00% 1.91% migration/5 [kernel.kallsyms] [k]
update_sd_lb_stats
0.00% 1.86% perf [kernel.kallsyms] [k]
__alloc_file
0.00% 1.83% swapper [kernel.kallsyms] [k]
intel_idle
0.00% 1.49% migration/2 [kernel.kallsyms] [k]
__switch_to_asm
0.00% 1.41% migration/6 [kernel.kallsyms] [k]
__bitmap_and
......
The outputs are sorted by the second event.
Let me take one more example of multiple groups with different amount of
events.
perf record -e '{cycles,instructions}' -e
'{branches,cache-misses,cache-references}' -a
perf report --group --group-sort-idx 2 --stdio
# Samples: 135 of events 'anon group { cycles, instructions }'
# Event count (approx.): 20558030
#
# Overhead Command Shared Object Symbol
# ................ ............... ........................
...................................
#
21.52% 0.00% perf [kernel.kallsyms] [k]
anon_vma_interval_tree_remove
10.63% 0.00% perf [kernel.kallsyms] [k] strncpy
9.61% 0.00% migration/5 [kernel.kallsyms] [k]
put_prev_task_stop
8.64% 0.00% migration/3 [kernel.kallsyms] [k]
update_blocked_averages
6.53% 0.00% migration/2 [kernel.kallsyms] [k]
update_sd_lb_stats
5.41% 0.00% migration/0 [kernel.kallsyms] [k] rb_erase
5.32% 0.00% migration/4 [kernel.kallsyms] [k]
reweight_entity
4.90% 0.00% swapper [kernel.kallsyms] [k]
e1000_clean_rx_irq
4.35% 0.00% migration/1 [kernel.kallsyms] [k] schedule
3.94% 0.00% migration/6 [kernel.kallsyms] [k]
propagate_entity_cfs_rq.isra.97
......
The output for group '{cycles, instructions}' is sorted by default
(first event) since the group-sort-idx (2) is invalid.
# Samples: 187 of events 'anon group { branches, cache-misses,
cache-references }'
# Event count (approx.): 2428023
#
# Overhead Command Shared Object
Symbol
# ........................ ............... ........................
.........................................
#
0.00% 0.00% 21.54% perf [kernel.kallsyms]
[k] up_write
0.00% 0.00% 10.01% swapper [kernel.kallsyms]
[k] note_gp_changes
0.00% 0.00% 9.51% kworker/u16:1-e [kernel.kallsyms]
[k] finish_task_switch
0.00% 12.48% 8.06% swapper [kernel.kallsyms]
[k] intel_idle
0.00% 0.00% 4.08% perf [kernel.kallsyms]
[k] kmem_cache_alloc
0.00% 0.00% 3.83% migration/3 [kernel.kallsyms]
[k] update_blocked_averages
0.00% 0.00% 3.58% migration/5 [kernel.kallsyms]
[k] pick_next_task_rt
0.00% 0.00% 3.46% swapper [kernel.kallsyms]
[k] rcu_dynticks_eqs_exit
0.00% 0.00% 3.22% swapper [kernel.kallsyms]
[k] calc_global_load
0.00% 0.00% 3.13% swapper [kernel.kallsyms]
[k] cpuidle_not_available
......
The output for group '{branches, cache-misses, cache-references}' is
sorted by the third event. It's expected.
> I think the option is easy to use as it is, just needs to be explained
> consequences for more groups with different amount of events
>
Thanks. Can we say something as following?
--group-sort-idx::
Sort the output by the event at the index n in group. If n is invalid,
sort by the first event. It can support multiple groups with different
amount of events. WARNING: This should be used with perf report --group.
> SNIP
>
>> +
>> +static int __hpp__group_sort_idx(struct hist_entry *a, struct hist_entry *b,
>> + hpp_field_fn get_field, int idx)
>> +{
>> + struct evsel *evsel = hists_to_evsel(a->hists);
>> + u64 *fields_a, *fields_b;
>> + int cmp, nr_members, ret, i;
>> +
>> + cmp = field_cmp(get_field(a), get_field(b));
>> + if (!perf_evsel__is_group_event(evsel))
>> + return cmp;
>> +
>> + nr_members = evsel->core.nr_members;
>> + ret = pair_fields_alloc(a, b, get_field, nr_members,
>> + &fields_a, &fields_b);
>> + if (ret) {
>> + ret = cmp;
>> + goto out;
>> + }
>> +
>> + if (idx >= 1 && idx < nr_members) {
>
> do we need to continue here if idx is out of the limit?
> I'm not sure but mybe in that case the comparison above
> should be enough?
>
Yes, we can use simpler code. Something like,
+ cmp = field_cmp(get_field(a), get_field(b));
+ if (!perf_evsel__is_group_event(evsel))
+ return cmp;
+
+ nr_members = evsel->core.nr_members;
+ if (idx < 1 || idx >= nr_members)
+ return cmp;
......
Thanks
Jin Yao
> thanks,
> jirka
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 1/3] perf report: Change sort order by a specified event in group
2019-12-17 1:47 ` Jin, Yao
@ 2019-12-17 9:06 ` Jiri Olsa
2019-12-18 2:29 ` Jin, Yao
0 siblings, 1 reply; 7+ messages in thread
From: Jiri Olsa @ 2019-12-17 9:06 UTC (permalink / raw)
To: Jin, Yao
Cc: acme, jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak,
kan.liang, yao.jin
On Tue, Dec 17, 2019 at 09:47:01AM +0800, Jin, Yao wrote:
>
>
> On 12/16/2019 3:31 PM, Jiri Olsa wrote:
> > On Thu, Dec 12, 2019 at 08:33:35PM +0800, Jin Yao wrote:
> >
> > SNIP
> >
> > >
> > > Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
> > > ---
> > > tools/perf/Documentation/perf-report.txt | 4 +
> > > tools/perf/builtin-report.c | 10 +++
> > > tools/perf/ui/hist.c | 108 +++++++++++++++++++----
> > > tools/perf/util/symbol_conf.h | 1 +
> > > 4 files changed, 108 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/tools/perf/Documentation/perf-report.txt b/tools/perf/Documentation/perf-report.txt
> > > index 8dbe2119686a..9ade613ef020 100644
> > > --- a/tools/perf/Documentation/perf-report.txt
> > > +++ b/tools/perf/Documentation/perf-report.txt
> > > @@ -371,6 +371,10 @@ OPTIONS
> > > Show event group information together. It forces group output also
> > > if there are no groups defined in data file.
> > > +--group-sort-idx::
> > > + Sort the output by the event at the index n in group. If n is invalid,
> > > + sort by the first event. WARNING: This should be used with --group.
> >
> > --group in record or report?
> >
>
> This --group is in perf-report. So even if it's not created with -e '{}' in
> perf-record, it still supports to show event information together.
>
> > you can also create groups with -e '{}', not just --group option
> >
> > I wonder you could check early on 'evlist->nr_groups' and fail
> > if there's no group defined if the option is enabled
> >
>
> Maybe we don't need to check that because it supports the case of no group
> defined.
>
> For example,
> perf record -e cycles,instructions
> perf report --group --group-sort-idx 1 --stdio
hum, --group will force evlist->nr_groups == 1, right?
so we could warn/fail on (group-sort-idx && !evlist->nr_groups)
SNIP
>
> Thanks. Can we say something as following?
>
> --group-sort-idx::
> Sort the output by the event at the index n in group. If n is invalid, sort
> by the first event. It can support multiple groups with different amount of
> events. WARNING: This should be used with perf report --group.
if the events are already grouped you dont need --group ;-) how about:
This should be used on grouped events.
thanks,
jirka
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 1/3] perf report: Change sort order by a specified event in group
2019-12-17 9:06 ` Jiri Olsa
@ 2019-12-18 2:29 ` Jin, Yao
0 siblings, 0 replies; 7+ messages in thread
From: Jin, Yao @ 2019-12-18 2:29 UTC (permalink / raw)
To: Jiri Olsa
Cc: acme, jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak,
kan.liang, yao.jin
On 12/17/2019 5:06 PM, Jiri Olsa wrote:
> On Tue, Dec 17, 2019 at 09:47:01AM +0800, Jin, Yao wrote:
>>
>>
>> On 12/16/2019 3:31 PM, Jiri Olsa wrote:
>>> On Thu, Dec 12, 2019 at 08:33:35PM +0800, Jin Yao wrote:
>>>
>>> SNIP
>>>
>>>>
>>>> Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
>>>> ---
>>>> tools/perf/Documentation/perf-report.txt | 4 +
>>>> tools/perf/builtin-report.c | 10 +++
>>>> tools/perf/ui/hist.c | 108 +++++++++++++++++++----
>>>> tools/perf/util/symbol_conf.h | 1 +
>>>> 4 files changed, 108 insertions(+), 15 deletions(-)
>>>>
>>>> diff --git a/tools/perf/Documentation/perf-report.txt b/tools/perf/Documentation/perf-report.txt
>>>> index 8dbe2119686a..9ade613ef020 100644
>>>> --- a/tools/perf/Documentation/perf-report.txt
>>>> +++ b/tools/perf/Documentation/perf-report.txt
>>>> @@ -371,6 +371,10 @@ OPTIONS
>>>> Show event group information together. It forces group output also
>>>> if there are no groups defined in data file.
>>>> +--group-sort-idx::
>>>> + Sort the output by the event at the index n in group. If n is invalid,
>>>> + sort by the first event. WARNING: This should be used with --group.
>>>
>>> --group in record or report?
>>>
>>
>> This --group is in perf-report. So even if it's not created with -e '{}' in
>> perf-record, it still supports to show event information together.
>>
>>> you can also create groups with -e '{}', not just --group option
>>>
>>> I wonder you could check early on 'evlist->nr_groups' and fail
>>> if there's no group defined if the option is enabled
>>>
>>
>> Maybe we don't need to check that because it supports the case of no group
>> defined.
>>
>> For example,
>> perf record -e cycles,instructions
>> perf report --group --group-sort-idx 1 --stdio
>
> hum, --group will force evlist->nr_groups == 1, right?
>
> so we could warn/fail on (group-sort-idx && !evlist->nr_groups)
>
>
Yes, we can. I have added this checking in v4.
> SNIP
>
>>
>> Thanks. Can we say something as following?
>>
>> --group-sort-idx::
>> Sort the output by the event at the index n in group. If n is invalid, sort
>> by the first event. It can support multiple groups with different amount of
>> events. WARNING: This should be used with perf report --group.
>
> if the events are already grouped you dont need --group ;-) how about:
>
> This should be used on grouped events.
>
OK, yes, that's better. I just post v4 which includes this update.
Thanks
Jin Yao
> thanks,
> jirka
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-12-18 7:18 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-12 12:33 [PATCH v3 1/3] perf report: Change sort order by a specified event in group Jin Yao
2019-12-12 12:33 ` [PATCH v3 2/3] perf report: Support a new key to reload the browser Jin Yao
2019-12-12 12:33 ` [PATCH v3 3/3] perf report: support hotkey to let user select any event for sorting Jin Yao
2019-12-16 7:31 ` [PATCH v3 1/3] perf report: Change sort order by a specified event in group Jiri Olsa
2019-12-17 1:47 ` Jin, Yao
2019-12-17 9:06 ` Jiri Olsa
2019-12-18 2:29 ` Jin, Yao
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).