Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1635443 > unrolled thread
| Started by | Jin Yao <yao.jin@linux.intel.com> |
|---|---|
| First post | 2017-05-04 09:10 +0200 |
| Last post | 2017-05-04 17:00 +0200 |
| Articles | 5 — 3 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
[PATCH v1 2/2] perf report: Display titles in left frame of annotate browser Jin Yao <yao.jin@linux.intel.com> - 2017-05-04 09:10 +0200
Re: [PATCH v1 2/2] perf report: Display titles in left frame of annotate browser Milian Wolff <milian.wolff@kdab.com> - 2017-05-04 11:10 +0200
Re: [PATCH v1 2/2] perf report: Display titles in left frame of annotate browser Arnaldo Carvalho de Melo <acme@kernel.org> - 2017-05-04 15:20 +0200
Re: [PATCH v1 2/2] perf report: Display titles in left frame of annotate browser Milian Wolff <milian.wolff@kdab.com> - 2017-05-04 16:10 +0200
Re: [PATCH v1 2/2] perf report: Display titles in left frame of annotate browser Arnaldo Carvalho de Melo <acme@kernel.org> - 2017-05-04 17:00 +0200
| From | Jin Yao <yao.jin@linux.intel.com> |
|---|---|
| Date | 2017-05-04 09:10 +0200 |
| Subject | [PATCH v1 2/2] perf report: Display titles in left frame of annotate browser |
| Message-ID | <tDj3H-bE-19@gated-at.bofh.it> |
The annotate browser is divided into 2 frames. Left frame
contains 3 columns (some platforms only have one column).
For example:
│26 int compute_flag()
│27 {
22.80 1.20 │ sub $0x8,%rsp
│25 int i;
│
│27 i = rand() % 2;
22.78 1.20 1 │ → callq rand@plt
While it's hard for user to understand what the data is.
This patch adds the titles "Percent", "IPC" and "Cycle"
on columns.
Percnt IPC Cycle │
│25 __attribute__((noinline))
│26 int compute_flag()
│27 {
22.80 1.20 │ sub $0x8,%rsp
│25 int i;
│
│27 i = rand() % 2;
22.78 1.20 1 │ → callq rand@plt
The titles are displayed at row 0 of annotate browser if row 0
doesn't have values of percent, ipc and cycle.
Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
---
tools/perf/ui/browsers/annotate.c | 27 ++++++++++++++++++++++++---
1 file changed, 24 insertions(+), 3 deletions(-)
diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index 52c1e8d..b1da5fb 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -125,12 +125,21 @@ static void annotate_browser__write(struct ui_browser *browser, void *entry, int
int i, pcnt_width = annotate_browser__pcnt_width(ab);
double percent_max = 0.0;
char bf[256];
+ bool show_title = false;
for (i = 0; i < ab->nr_events; i++) {
if (bdl->samples[i].percent > percent_max)
percent_max = bdl->samples[i].percent;
}
+ if ((row == 0) && (dl->offset == -1 || percent_max == 0.0)) {
+ if (ab->have_cycles) {
+ if (dl->ipc == 0.0 && dl->cycles == 0)
+ show_title = true;
+ } else
+ show_title = true;
+ }
+
if (dl->offset != -1 && percent_max != 0.0) {
for (i = 0; i < ab->nr_events; i++) {
ui_browser__set_percent_color(browser,
@@ -146,18 +155,30 @@ static void annotate_browser__write(struct ui_browser *browser, void *entry, int
}
} else {
ui_browser__set_percent_color(browser, 0, current_entry);
- ui_browser__write_nstring(browser, " ", 7 * ab->nr_events);
+
+ if (!show_title)
+ ui_browser__write_nstring(browser, " ",
+ 7 * ab->nr_events);
+ else
+ ui_browser__printf(browser, "%*s ", 6, "Percnt");
}
if (ab->have_cycles) {
if (dl->ipc)
ui_browser__printf(browser, "%*.2f ", IPC_WIDTH - 1, dl->ipc);
- else
+ else if (!show_title)
ui_browser__write_nstring(browser, " ", IPC_WIDTH);
+ else
+ ui_browser__printf(browser, "%*s ",
+ IPC_WIDTH - 1, "IPC");
+
if (dl->cycles)
ui_browser__printf(browser, "%*" PRIu64 " ",
CYCLES_WIDTH - 1, dl->cycles);
- else
+ else if (!show_title)
ui_browser__write_nstring(browser, " ", CYCLES_WIDTH);
+ else
+ ui_browser__printf(browser, "%*s ",
+ CYCLES_WIDTH - 1, "Cycle");
}
SLsmg_write_char(' ');
--
2.7.4
[toc] | [next] | [standalone]
| From | Milian Wolff <milian.wolff@kdab.com> |
|---|---|
| Date | 2017-05-04 11:10 +0200 |
| Message-ID | <tDkVR-1oS-33@gated-at.bofh.it> |
| In reply to | #1635443 |
On Thursday, May 4, 2017 4:58:15 PM CEST Jin Yao wrote:
> The annotate browser is divided into 2 frames. Left frame
> contains 3 columns (some platforms only have one column).
>
> For example:
>
> │26 int compute_flag()
> │27 {
> 22.80 1.20 │ sub $0x8,%rsp
> │25 int i;
> │
> │27 i = rand() % 2;
> 22.78 1.20 1 │ → callq rand@plt
>
> While it's hard for user to understand what the data is.
>
> This patch adds the titles "Percent", "IPC" and "Cycle"
> on columns.
>
> Percnt IPC Cycle │
> │25 __attribute__((noinline))
> │26 int compute_flag()
> │27 {
> 22.80 1.20 │ sub $0x8,%rsp
> │25 int i;
> │
> │27 i = rand() % 2;
> 22.78 1.20 1 │ → callq rand@plt
>
> The titles are displayed at row 0 of annotate browser if row 0
> doesn't have values of percent, ipc and cycle.
Functionality wise a really good improvement - thanks! But personally I find
the abbreviation of one character (i.e. "Percnt" instead of "Percent") not so
nice. If space really is an issue here, use "%"?
Also note though that it's unclear what this percentage actually is. I guess
it's a sample percentage? Maybe a header should be added that explains these
values to newbies. I bet many people won't even know what IPC is either.
Cheers
--
Milian Wolff | milian.wolff@kdab.com | Software Engineer
KDAB (Deutschland) GmbH&Co KG, a KDAB Group company
Tel: +49-30-521325470
KDAB - The Qt Experts
[toc] | [prev] | [next] | [standalone]
| From | Arnaldo Carvalho de Melo <acme@kernel.org> |
|---|---|
| Date | 2017-05-04 15:20 +0200 |
| Subject | Re: [PATCH v1 2/2] perf report: Display titles in left frame of annotate browser |
| Message-ID | <tDoPL-3W2-3@gated-at.bofh.it> |
| In reply to | #1635507 |
Em Thu, May 04, 2017 at 11:01:48AM +0200, Milian Wolff escreveu:
> On Thursday, May 4, 2017 4:58:15 PM CEST Jin Yao wrote:
> > This patch adds the titles "Percent", "IPC" and "Cycle"
> > on columns.
> > Percnt IPC Cycle │
> > │25 __attribute__((noinline))
> > │26 int compute_flag()
> > │27 {
> > 22.80 1.20 │ sub $0x8,%rsp
> > │25 int i;
> > │
> > │27 i = rand() % 2;
> > 22.78 1.20 1 │ → callq rand@plt
> > The titles are displayed at row 0 of annotate browser if row 0
> > doesn't have values of percent, ipc and cycle.
> Functionality wise a really good improvement - thanks! But personally I find
> the abbreviation of one character (i.e. "Percnt" instead of "Percent") not so
> nice. If space really is an issue here, use "%"?
Ok, will make it 'Percent' as we have space for that, and will add
Acked-by: Millian, ok?
> Also note though that it's unclear what this percentage actually is. I guess
> it's a sample percentage? Maybe a header should be added that explains these
> values to newbies. I bet many people won't even know what IPC is either.
Perhaps we could have some help files, then when the user presses 'h'
one of the lines would be:
h Report explanation (columns, etc)
?
- Arnaldo
[toc] | [prev] | [next] | [standalone]
| From | Milian Wolff <milian.wolff@kdab.com> |
|---|---|
| Date | 2017-05-04 16:10 +0200 |
| Message-ID | <tDpC9-4uB-7@gated-at.bofh.it> |
| In reply to | #1635724 |
[Multipart message — attachments visible in raw view] — view raw
On Thursday, May 4, 2017 3:12:50 PM CEST Arnaldo Carvalho de Melo wrote:
> Em Thu, May 04, 2017 at 11:01:48AM +0200, Milian Wolff escreveu:
> > On Thursday, May 4, 2017 4:58:15 PM CEST Jin Yao wrote:
> > > This patch adds the titles "Percent", "IPC" and "Cycle"
> > > on columns.
> > >
> > > Percnt IPC Cycle │
> > >
> > > │25 __attribute__((noinline))
> > > │26 int compute_flag()
> > > │27 {
> > >
> > > 22.80 1.20 │ sub $0x8,%rsp
> > >
> > > │25 int i;
> > > │
> > > │27 i = rand() % 2;
> > >
> > > 22.78 1.20 1 │ → callq rand@plt
> > >
> > > The titles are displayed at row 0 of annotate browser if row 0
> > > doesn't have values of percent, ipc and cycle.
> >
> > Functionality wise a really good improvement - thanks! But personally I
> > find the abbreviation of one character (i.e. "Percnt" instead of
> > "Percent") not so nice. If space really is an issue here, use "%"?
>
> Ok, will make it 'Percent' as we have space for that, and will add
> Acked-by: Millian, ok?
Just one L, but otherwise yes :)
> > Also note though that it's unclear what this percentage actually is. I
> > guess it's a sample percentage? Maybe a header should be added that
> > explains these values to newbies. I bet many people won't even know what
> > IPC is either.
> Perhaps we could have some help files, then when the user presses 'h'
> one of the lines would be:
>
> h Report explanation (columns, etc)
Yeah I think that would be a good addition for the future.
Cheers
--
Milian Wolff | milian.wolff@kdab.com | Software Engineer
KDAB (Deutschland) GmbH&Co KG, a KDAB Group company
Tel: +49-30-521325470
KDAB - The Qt Experts
[toc] | [prev] | [next] | [standalone]
| From | Arnaldo Carvalho de Melo <acme@kernel.org> |
|---|---|
| Date | 2017-05-04 17:00 +0200 |
| Subject | Re: [PATCH v1 2/2] perf report: Display titles in left frame of annotate browser |
| Message-ID | <tDqoy-4PF-11@gated-at.bofh.it> |
| In reply to | #1635752 |
Em Thu, May 04, 2017 at 04:04:42PM +0200, Milian Wolff escreveu:
> On Thursday, May 4, 2017 3:12:50 PM CEST Arnaldo Carvalho de Melo wrote:
> > Em Thu, May 04, 2017 at 11:01:48AM +0200, Milian Wolff escreveu:
> > > On Thursday, May 4, 2017 4:58:15 PM CEST Jin Yao wrote:
> > > > This patch adds the titles "Percent", "IPC" and "Cycle"
> > > > on columns.
> > > >
> > > > Percnt IPC Cycle │
> > > >
> > > > │25 __attribute__((noinline))
> > > > │26 int compute_flag()
> > > > │27 {
> > > >
> > > > 22.80 1.20 │ sub $0x8,%rsp
> > > >
> > > > │25 int i;
> > > > │
> > > > │27 i = rand() % 2;
> > > >
> > > > 22.78 1.20 1 │ → callq rand@plt
> > > >
> > > > The titles are displayed at row 0 of annotate browser if row 0
> > > > doesn't have values of percent, ipc and cycle.
> > >
> > > Functionality wise a really good improvement - thanks! But personally I
> > > find the abbreviation of one character (i.e. "Percnt" instead of
> > > "Percent") not so nice. If space really is an issue here, use "%"?
> >
> > Ok, will make it 'Percent' as we have space for that, and will add
> > Acked-by: Millian, ok?
>
> Just one L, but otherwise yes :)
Sure, sorry about that, c'n'pasted from a message from you, so should be
all well.
> > > Also note though that it's unclear what this percentage actually is. I
> > > guess it's a sample percentage? Maybe a header should be added that
> > > explains these values to newbies. I bet many people won't even know what
> > > IPC is either.
> > Perhaps we could have some help files, then when the user presses 'h'
> > one of the lines would be:
> >
> > h Report explanation (columns, etc)
>
> Yeah I think that would be a good addition for the future.
:-)
- Arnaldo
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web