* [PATCH] perf report: show sort_order in title
@ 2017-03-13 8:36 changbin.du
2017-03-13 14:47 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 6+ messages in thread
From: changbin.du @ 2017-03-13 8:36 UTC (permalink / raw)
To: peterz, mingo, acme; +Cc: linux-kernel, Changbin Du
From: Changbin Du <changbin.du@intel.com>
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 <changbin.du@intel.com>
---
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] perf report: show sort_order in title
2017-03-13 8:36 [PATCH] perf report: show sort_order in title changbin.du
@ 2017-03-13 14:47 ` Arnaldo Carvalho de Melo
2017-03-13 14:57 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 6+ messages in thread
From: Arnaldo Carvalho de Melo @ 2017-03-13 14:47 UTC (permalink / raw)
To: changbin.du; +Cc: peterz, mingo, linux-kernel
Em Mon, Mar 13, 2017 at 04:36:35PM +0800, changbin.du@intel.com escreveu:
> From: Changbin Du <changbin.du@intel.com>
>
> 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 <changbin.du@intel.com>
> ---
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] perf report: show sort_order in title
2017-03-13 14:47 ` Arnaldo Carvalho de Melo
@ 2017-03-13 14:57 ` Arnaldo Carvalho de Melo
2017-03-14 2:16 ` Du, Changbin
0 siblings, 1 reply; 6+ messages in thread
From: Arnaldo Carvalho de Melo @ 2017-03-13 14:57 UTC (permalink / raw)
To: changbin.du; +Cc: peterz, mingo, linux-kernel
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, changbin.du@intel.com escreveu:
> > From: Changbin Du <changbin.du@intel.com>
> >
> > 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 <changbin.du@intel.com>
> > ---
> > 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] perf report: show sort_order in title
2017-03-13 14:57 ` Arnaldo Carvalho de Melo
@ 2017-03-14 2:16 ` Du, Changbin
2017-03-14 13:04 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 6+ messages in thread
From: Du, Changbin @ 2017-03-14 2:16 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo; +Cc: changbin.du, peterz, mingo, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 2938 bytes --]
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] perf report: show sort_order in title
2017-03-14 2:16 ` Du, Changbin
@ 2017-03-14 13:04 ` Arnaldo Carvalho de Melo
2017-03-15 2:11 ` Du, Changbin
0 siblings, 1 reply; 6+ messages in thread
From: Arnaldo Carvalho de Melo @ 2017-03-14 13:04 UTC (permalink / raw)
To: Du, Changbin
Cc: peterz, Namhyung Kim, Jiri Olsa, Wang Nan, David Ahern,
Ingo Molnar, linux-kernel
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] perf report: show sort_order in title
2017-03-14 13:04 ` Arnaldo Carvalho de Melo
@ 2017-03-15 2:11 ` Du, Changbin
0 siblings, 0 replies; 6+ messages in thread
From: Du, Changbin @ 2017-03-15 2:11 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Du, Changbin, peterz, Namhyung Kim, Jiri Olsa, Wang Nan,
David Ahern, Ingo Molnar, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 3251 bytes --]
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-03-15 2:14 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-13 8:36 [PATCH] perf report: show sort_order in title changbin.du
2017-03-13 14:47 ` Arnaldo Carvalho de Melo
2017-03-13 14:57 ` Arnaldo Carvalho de Melo
2017-03-14 2:16 ` Du, Changbin
2017-03-14 13:04 ` Arnaldo Carvalho de Melo
2017-03-15 2:11 ` Du, Changbin
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).