Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1598939 > unrolled thread
| Started by | changbin.du@intel.com |
|---|---|
| First post | 2017-03-13 09:40 +0100 |
| Last post | 2017-03-14 03:20 +0100 |
| Articles | 4 — 3 participants |
Back to article view | Back to linux.kernel
[PATCH] perf report: show sort_order in title changbin.du@intel.com - 2017-03-13 09:40 +0100
Re: [PATCH] perf report: show sort_order in title Arnaldo Carvalho de Melo <acme@kernel.org> - 2017-03-13 15:50 +0100
Re: [PATCH] perf report: show sort_order in title Arnaldo Carvalho de Melo <acme@kernel.org> - 2017-03-13 16:00 +0100
Re: [PATCH] perf report: show sort_order in title "Du, Changbin" <changbin.du@intel.com> - 2017-03-14 03:20 +0100
| From | changbin.du@intel.com |
|---|---|
| Date | 2017-03-13 09:40 +0100 |
| Subject | [PATCH] perf report: show sort_order in title |
| Message-ID | <tktGh-6dB-5@gated-at.bofh.it> |
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
[toc] | [next] | [standalone]
| From | Arnaldo Carvalho de Melo <acme@kernel.org> |
|---|---|
| Date | 2017-03-13 15:50 +0100 |
| Message-ID | <tkzsm-1TD-21@gated-at.bofh.it> |
| In reply to | #1598939 |
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
[toc] | [prev] | [next] | [standalone]
| From | Arnaldo Carvalho de Melo <acme@kernel.org> |
|---|---|
| Date | 2017-03-13 16:00 +0100 |
| Message-ID | <tkzC2-1Yl-9@gated-at.bofh.it> |
| In reply to | #1599476 |
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
[toc] | [prev] | [next] | [standalone]
| From | "Du, Changbin" <changbin.du@intel.com> |
|---|---|
| Date | 2017-03-14 03:20 +0100 |
| Message-ID | <tkKe6-1Fo-7@gated-at.bofh.it> |
| In reply to | #1599486 |
[Multipart message — attachments visible in raw view] — view raw
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
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web