2017-03-13 08:39:17

by Du, Changbin

[permalink] [raw]
Subject: [PATCH] perf report: show sort_order in title

From: Changbin Du <[email protected]>

In the report, how does the data sort is a important info for
analyser. Moreover, perf sometimes insert or append sort fields
automatically. Thus user may confuse how it sorts w/o reading
perf internal. So here print the order info to repor title.

Signed-off-by: Changbin Du <[email protected]>
---
tools/perf/ui/browsers/hists.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index fc4fb66..6c6b615 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -2194,6 +2194,25 @@ static inline bool is_report_browser(void *timer)
return timer == NULL;
}

+static int get_sort_fields_str(struct hists *hists, char *buf, size_t size)
+{
+ struct perf_hpp_fmt *fmt;
+ bool first = true;
+ int ret = 0;
+
+ hists__for_each_sort_list(hists, fmt) {
+ if (first) {
+ first = false;
+ ret += scnprintf(buf + ret, size - ret, "%s", fmt->name);
+ } else {
+ ret += scnprintf(buf + ret, size - ret, ",%s", fmt->name);
+ }
+ if (size - ret <= 0)
+ break;
+ }
+ return ret;
+}
+
static int perf_evsel_browser_title(struct hist_browser *browser,
char *bf, size_t size)
{
@@ -2274,6 +2293,9 @@ static int perf_evsel_browser_title(struct hist_browser *browser,
printed += scnprintf(bf + printed, size - printed, " [z]");
}

+ get_sort_fields_str(hists, buf, sizeof(buf));
+ printed += scnprintf(bf + printed, size - printed, ", Sort by: %s", buf);
+
return printed;
}

--
2.7.4


2017-03-13 14:47:26

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH] perf report: show sort_order in title

Em Mon, Mar 13, 2017 at 04:36:35PM +0800, [email protected] escreveu:
> From: Changbin Du <[email protected]>
>
> In the report, how does the data sort is a important info for
> analyser. Moreover, perf sometimes insert or append sort fields
> automatically. Thus user may confuse how it sorts w/o reading
> perf internal. So here print the order info to repor title.
>
> Signed-off-by: Changbin Du <[email protected]>
> ---
> tools/perf/ui/browsers/hists.c | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
>
> diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> index fc4fb66..6c6b615 100644
> --- a/tools/perf/ui/browsers/hists.c
> +++ b/tools/perf/ui/browsers/hists.c
> @@ -2194,6 +2194,25 @@ static inline bool is_report_browser(void *timer)
> return timer == NULL;
> }
>
> +static int get_sort_fields_str(struct hists *hists, char *buf, size_t size)

I'll just rename this to use the tools/perf/ style for such functions,
making it:

static int hists__scnprintf_sort_fields(hists, buf, size)

> +{
> + struct perf_hpp_fmt *fmt;
> + bool first = true;
> + int ret = 0;
> +
> + hists__for_each_sort_list(hists, fmt) {
> + if (first) {
> + first = false;
> + ret += scnprintf(buf + ret, size - ret, "%s", fmt->name);
> + } else {
> + ret += scnprintf(buf + ret, size - ret, ",%s", fmt->name);
> + }
> + if (size - ret <= 0)
> + break;
> + }
> + return ret;
> +}
> +
> static int perf_evsel_browser_title(struct hist_browser *browser,
> char *bf, size_t size)
> {
> @@ -2274,6 +2293,9 @@ static int perf_evsel_browser_title(struct hist_browser *browser,
> printed += scnprintf(bf + printed, size - printed, " [z]");
> }
>
> + get_sort_fields_str(hists, buf, sizeof(buf));
> + printed += scnprintf(bf + printed, size - printed, ", Sort by: %s", buf);
> +
> return printed;
> }
>
> --
> 2.7.4

2017-03-13 14:57:40

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH] perf report: show sort_order in title

Em Mon, Mar 13, 2017 at 11:47:14AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Mon, Mar 13, 2017 at 04:36:35PM +0800, [email protected] escreveu:
> > From: Changbin Du <[email protected]>
> >
> > In the report, how does the data sort is a important info for
> > analyser. Moreover, perf sometimes insert or append sort fields
> > automatically. Thus user may confuse how it sorts w/o reading
> > perf internal. So here print the order info to repor title.
> >
> > Signed-off-by: Changbin Du <[email protected]>
> > ---
> > tools/perf/ui/browsers/hists.c | 22 ++++++++++++++++++++++
> > 1 file changed, 22 insertions(+)
> >
> > diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
> > index fc4fb66..6c6b615 100644
> > --- a/tools/perf/ui/browsers/hists.c
> > +++ b/tools/perf/ui/browsers/hists.c
> > @@ -2194,6 +2194,25 @@ static inline bool is_report_browser(void *timer)
> > return timer == NULL;
> > }
> >
> > +static int get_sort_fields_str(struct hists *hists, char *buf, size_t size)
>
> I'll just rename this to use the tools/perf/ style for such functions,
> making it:
>
> static int hists__scnprintf_sort_fields(hists, buf, size)

But then, while testing,

Before:

$ perf report
Samples: 405 of event 'cycles', Event count (approx.): 101733003
Overhead Command Shared Object Symbol
11.15% swapper [kernel.vmlinux] [k] intel_idle
3.00% firefox libxul.so [.] 0x0000000001298b8d
1.74% swapper [kernel.vmlinux] [k] update_blocked_averages
1.69% qemu-system-x86 [kernel.vmlinux] [k] __fget
1.18% swapper [kernel.vmlinux] [k] update_wall_time

Tip: Save output of perf stat using: perf stat record <target workload>

After:

$ perf report
Samples: 405 of event 'cycles', Event count (approx.): 101733003, Sort by: Children,Overhead,Command,Shared Object,Symbol
Overhead Command Shared Object Symbol
11.15% swapper [kernel.vmlinux] [k] intel_idle
3.00% firefox libxul.so [.] 0x0000000001298b8d
1.74% swapper [kernel.vmlinux] [k] update_blocked_averages
1.69% qemu-system-x86 [kernel.vmlinux] [k] __fget
1.18% swapper [kernel.vmlinux] [k] update_wall_time


I see now duplication of info, where is the value? Can you show the usecase in
a compelling way?

- Arnaldo

2017-03-14 02:19:14

by Du, Changbin

[permalink] [raw]
Subject: Re: [PATCH] perf report: show sort_order in title

On Mon, Mar 13, 2017 at 11:57:18AM -0300, Arnaldo Carvalho de Melo wrote:
> > I'll just rename this to use the tools/perf/ style for such functions,
> > making it:
> >
> > static int hists__scnprintf_sort_fields(hists, buf, size)
>
> But then, while testing,
>
> Before:
>
> $ perf report
> Samples: 405 of event 'cycles', Event count (approx.): 101733003
> Overhead Command Shared Object Symbol
> 11.15% swapper [kernel.vmlinux] [k] intel_idle
> 3.00% firefox libxul.so [.] 0x0000000001298b8d
> 1.74% swapper [kernel.vmlinux] [k] update_blocked_averages
> 1.69% qemu-system-x86 [kernel.vmlinux] [k] __fget
> 1.18% swapper [kernel.vmlinux] [k] update_wall_time
>
> Tip: Save output of perf stat using: perf stat record <target workload>
>
> After:
>
> $ perf report
> Samples: 405 of event 'cycles', Event count (approx.): 101733003, Sort by: Children,Overhead,Command,Shared Object,Symbol
> Overhead Command Shared Object Symbol
> 11.15% swapper [kernel.vmlinux] [k] intel_idle
> 3.00% firefox libxul.so [.] 0x0000000001298b8d
> 1.74% swapper [kernel.vmlinux] [k] update_blocked_averages
> 1.69% qemu-system-x86 [kernel.vmlinux] [k] __fget
> 1.18% swapper [kernel.vmlinux] [k] update_wall_time
>
>
> I see now duplication of info, where is the value? Can you show the usecase in
> a compelling way?
>
> - Arnaldo

Thanks for trying. The key idea is to show how does the data sort, especially
the first sort key. When I use some GUI based perf tool, I can see how
my data is sorted by checking the report header status. I think this is
a good for browser.

You are right, the info is duplicated. I got another idea that we show a '↓' at
the header string and only for the first sort key. What do you think?

$ perf report
Samples: 405 of event 'cycles', Event count (approx.): 101733003
↓Overhead Command Shared Object Symbol
11.15% swapper [kernel.vmlinux] [k] intel_idle
3.00% firefox libxul.so [.] 0x0000000001298b8d
1.74% swapper [kernel.vmlinux] [k] update_blocked_averages

Another idea I want to add is to support dynamic sorting. For me, I use perf to
analysing entire system performance, and the data is very large. Then sometimes
it take as long as ~10 minitues to read perf data. So I think if we can change
sort w/o reload data will be good.

--
Thanks,
Changbin Du


Attachments:
(No filename) (2.81 kB)
signature.asc (473.00 B)
Download all attachments

2017-03-14 13:04:24

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH] perf report: show sort_order in title

Em Tue, Mar 14, 2017 at 10:16:44AM +0800, Du, Changbin escreveu:
> On Mon, Mar 13, 2017 at 11:57:18AM -0300, Arnaldo Carvalho de Melo wrote:
> > But then, while testing,

> > Before:

> > $ perf report
> > Samples: 405 of event 'cycles', Event count (approx.): 101733003
> > Overhead Command Shared Object Symbol
> > 11.15% swapper [kernel.vmlinux] [k] intel_idle

> > Tip: Save output of perf stat using: perf stat record <target workload>

> > After:

> > $ perf report
> > Samples: 405 of event 'cycles', Event count (approx.): 101733003, Sort by: Children,Overhead,Command,Shared Object,Symbol
> > Overhead Command Shared Object Symbol
> > 11.15% swapper [kernel.vmlinux] [k] intel_idle

> > I see now duplication of info, where is the value? Can you show the usecase in
> > a compelling way?

> Thanks for trying. The key idea is to show how does the data sort, especially
> the first sort key. When I use some GUI based perf tool, I can see how
> my data is sorted by checking the report header status. I think this is
> a good for browser.

> You are right, the info is duplicated. I got another idea that we show a '↓' at
> the header string and only for the first sort key. What do you think?

> $ perf report
> Samples: 405 of event 'cycles', Event count (approx.): 101733003
> ↓Overhead Command Shared Object Symbol

this is much more compact, but you need to make it abundantly clear what
you are trying to achieve by showind counter examples were what we get
on that line starting with your suggested marker isn't the sort order.
Otherwise even a character is one too much :-)

> 11.15% swapper [kernel.vmlinux] [k] intel_idle
> 3.00% firefox libxul.so [.] 0x0000000001298b8d
> 1.74% swapper [kernel.vmlinux] [k] update_blocked_averages

> Another idea I want to add is to support dynamic sorting. For me, I use perf to
> analysing entire system performance, and the data is very large. Then sometimes
> it take as long as ~10 minitues to read perf data. So I think if we can change
> sort w/o reload data will be good.

And in some cases it is even possible! I.e. if you haven't collapsed too
much, you will not have to reprocess the file to get to the new order.

BTW, have you played with:

perf top --hierarchy

Try it with -g and --call-graph dwarf

Also try:

perf report --hierarchy

- Arnaldo

2017-03-15 02:14:08

by Du, Changbin

[permalink] [raw]
Subject: Re: [PATCH] perf report: show sort_order in title

On Tue, Mar 14, 2017 at 10:04:16AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Mar 14, 2017 at 10:16:44AM +0800, Du, Changbin escreveu:
> > On Mon, Mar 13, 2017 at 11:57:18AM -0300, Arnaldo Carvalho de Melo wrote:
> > > But then, while testing,
>
> > > Before:
>
> > > $ perf report
> > > Samples: 405 of event 'cycles', Event count (approx.): 101733003
> > > Overhead Command Shared Object Symbol
> > > 11.15% swapper [kernel.vmlinux] [k] intel_idle
>
> > > Tip: Save output of perf stat using: perf stat record <target workload>
>
> > > After:
>
> > > $ perf report
> > > Samples: 405 of event 'cycles', Event count (approx.): 101733003, Sort by: Children,Overhead,Command,Shared Object,Symbol
> > > Overhead Command Shared Object Symbol
> > > 11.15% swapper [kernel.vmlinux] [k] intel_idle
>
> > > I see now duplication of info, where is the value? Can you show the usecase in
> > > a compelling way?
>
> > Thanks for trying. The key idea is to show how does the data sort, especially
> > the first sort key. When I use some GUI based perf tool, I can see how
> > my data is sorted by checking the report header status. I think this is
> > a good for browser.
>
> > You are right, the info is duplicated. I got another idea that we show a '↓' at
> > the header string and only for the first sort key. What do you think?
>
> > $ perf report
> > Samples: 405 of event 'cycles', Event count (approx.): 101733003
> > ↓Overhead Command Shared Object Symbol
>
> this is much more compact, but you need to make it abundantly clear what
> you are trying to achieve by showind counter examples were what we get
> on that line starting with your suggested marker isn't the sort order.
> Otherwise even a character is one too much :-)
>
Yes, I just want get know how does perf data sort. Because sometimes the
real sort order doesn't match the '-s' option I given. In this case, I
was confused about the sorting before reading into the code.

> > 11.15% swapper [kernel.vmlinux] [k] intel_idle
> > 3.00% firefox libxul.so [.] 0x0000000001298b8d
> > 1.74% swapper [kernel.vmlinux] [k] update_blocked_averages
>
> > Another idea I want to add is to support dynamic sorting. For me, I use perf to
> > analysing entire system performance, and the data is very large. Then sometimes
> > it take as long as ~10 minitues to read perf data. So I think if we can change
> > sort w/o reload data will be good.
>
> And in some cases it is even possible! I.e. if you haven't collapsed too
> much, you will not have to reprocess the file to get to the new order.
>
> BTW, have you played with:
>
> perf top --hierarchy
>
> Try it with -g and --call-graph dwarf
>
> Also try:
>
> perf report --hierarchy
>
> - Arnaldo

Sounds great! I tried '--hierarchy' option, but still don't know how to
resort the report. Could you give a hint?

--
Thanks,
Changbin Du


Attachments:
(No filename) (3.10 kB)
signature.asc (473.00 B)
Download all attachments