Skip to contentSkip to conversation

perf report: Add titles for percent/ipc/cycle in annotate browser

PATCH Superseded Jin Yao 4 participants 9 messages May 4, 2017 – Jun 20, 2017 perf events

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Jin Yao

author
#0 lore raw

cover perf report: Add titles for percent/ipc/cycle in annotate browser

The patch series adds the titles "Percnt", "IPC" and "Cycle" in annotate browser. It helps user to understand easily for what the columns in left frame represents for.

For example:

    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 patch "perf report: Remove the unnecessary checking in annotate_browser_write" removes the invalid condition checking in current code.

Jin Yao (2):
perf report: Remove the unnecessary checking in annotate_browser_write perf report: Display titles in left frame of annotate browser

 tools/perf/ui/browsers/annotate.c | 51 ++++++++++++++++++++++++++-------------
 1 file changed, 34 insertions(+), 17 deletions(-)
Signature · 1 line
2.7.4
author
#1 lore raw

PATCH 1/2 perf report: Remove the unnecessary checking in annotate_browser_write

In annotate_browser_write(),

        if (dl->offset != -1 && percent_max != 0.0) {
                if (percent_max != 0.0) {
			...
                }
                ...
        }

The second checking of (percent_max != 0.0) is not necessary, so remove it.

Signed-off-by Jin Yao <[email protected]>
---
 tools/perf/ui/browsers/annotate.c | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)
tools/perf/ui/browsers/annotate.c +10 -14
index d990ad0..52c1e8d 100644
@@ -132,21 +132,17 @@ static void annotate_browser__write(struct ui_browser *browser, void *entry, int
 	}
 
 	if (dl->offset != -1 && percent_max != 0.0) {
-		if (percent_max != 0.0) {
-			for (i = 0; i < ab->nr_events; i++) {
-				ui_browser__set_percent_color(browser,
-							bdl->samples[i].percent,
-							current_entry);
-				if (annotate_browser__opts.show_total_period) {
-					ui_browser__printf(browser, "%6" PRIu64 " ",
-							   bdl->samples[i].nr);
-				} else {
-					ui_browser__printf(browser, "%6.2f ",
-							   bdl->samples[i].percent);
-				}
+		for (i = 0; i < ab->nr_events; i++) {
+			ui_browser__set_percent_color(browser,
+						bdl->samples[i].percent,
+						current_entry);
+			if (annotate_browser__opts.show_total_period) {
+				ui_browser__printf(browser, "%6" PRIu64 " ",
+						   bdl->samples[i].nr);
+			} else {
+				ui_browser__printf(browser, "%6.2f ",
+						   bdl->samples[i].percent);
 			}
-		} else {
-			ui_browser__write_nstring(browser, " ", 7 * ab->nr_events);
 		}
 	} else {
 		ui_browser__set_percent_color(browser, 0, current_entry);
Signature · 1 line
2.7.4
merge bot
#2 lore raw

[tip:perf/core] perf report: Remove unnecessary check in annotate_browser_write()

perf report: Remove unnecessary check in annotate_browser_write()
Commit-ID:  c564f0db92b7f8d734ce530e42a540e12ae3d583
Gitweb:     http://git.kernel.org/tip/c564f0db92b7f8d734ce530e42a540e12ae3d583
Author:     Jin Yao <[email protected]>
AuthorDate: Thu, 4 May 2017 22:58:14 +0800
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Mon, 19 Jun 2017 15:14:57 -0300

perf report: Remove unnecessary check in annotate_browser_write()

In annotate_browser_write(),

        if (dl->offset != -1 && percent_max != 0.0) {
                if (percent_max != 0.0) {
			...
                }
                ...
        }

The second check of (percent_max != 0.0) is not necessary, remove it.

Signed-off-by Yao Jin <[email protected]>
Acked-by Milian Wolff <[email protected]>
Cc Alexander Shishkin <[email protected]>
Cc Andi Kleen <[email protected]>
Cc Jiri Olsa <[email protected]>
Cc Kan Liang <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Yao Jin <[email protected]>
Link http://lkml.kernel.org/r/[email protected] open here
Signed-off-by Arnaldo Carvalho de Melo <[email protected]>
---
 tools/perf/ui/browsers/annotate.c | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)
tools/perf/ui/browsers/annotate.c +10 -14
index d990ad0..52c1e8d 100644
@@ -132,21 +132,17 @@ static void annotate_browser__write(struct ui_browser *browser, void *entry, int
 	}
 
 	if (dl->offset != -1 && percent_max != 0.0) {
-		if (percent_max != 0.0) {
-			for (i = 0; i < ab->nr_events; i++) {
-				ui_browser__set_percent_color(browser,
-							bdl->samples[i].percent,
-							current_entry);
-				if (annotate_browser__opts.show_total_period) {
-					ui_browser__printf(browser, "%6" PRIu64 " ",
-							   bdl->samples[i].nr);
-				} else {
-					ui_browser__printf(browser, "%6.2f ",
-							   bdl->samples[i].percent);
-				}
+		for (i = 0; i < ab->nr_events; i++) {
+			ui_browser__set_percent_color(browser,
+						bdl->samples[i].percent,
+						current_entry);
+			if (annotate_browser__opts.show_total_period) {
+				ui_browser__printf(browser, "%6" PRIu64 " ",
+						   bdl->samples[i].nr);
+			} else {
+				ui_browser__printf(browser, "%6.2f ",
+						   bdl->samples[i].percent);
 			}
-		} else {
-			ui_browser__write_nstring(browser, " ", 7 * ab->nr_events);
 		}
 	} else {
 		ui_browser__set_percent_color(browser, 0, current_entry);
author
#3 lore raw

PATCH 2/2 perf report: Display titles in left frame of annotate browser

In reply to Jin Yao

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 <[email protected]>
---
 tools/perf/ui/browsers/annotate.c | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)
tools/perf/ui/browsers/annotate.c +24 -3
index 52c1e8d..b1da5fb 100644
@@ -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(' ');
Signature · 1 line
2.7.4
#4 lore raw

Re: [PATCH v1 2/2] perf report: Display titles in left frame of annotate browser

Quoted 28 lines · CEST Jin Yao

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
Quoted 2 lines

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

Signature · 4 lines
Milian Wolff | [email protected] | Software Engineer
KDAB (Deutschland) GmbH&Co KG, a KDAB Group company
Tel: +49-30-521325470
KDAB - The Qt Experts
maintainer
#5 lore raw

Re: [PATCH v1 2/2] perf report: Display titles in left frame of annotate browser

Em Thu, May 04, 2017 at 11:01:48AM +0200, Milian Wolff escreveu:

Quoted 14 lines · CEST Jin Yao
Quoted 12 lines

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
Quoted 6 lines
Quoted 2 lines

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?

Quoted 3 lines

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

#6 lore raw

Re: [PATCH v1 2/2] perf report: Display titles in left frame of annotate browser

Quoted 22 lines · CEST Arnaldo Carvalho de Melo

Em Thu, May 04, 2017 at 11:01:48AM +0200, Milian Wolff escreveu:

Quoted 21 lines · CEST Jin Yao
Quoted 19 lines

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.

Quoted 6 lines
Quoted 3 lines

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 :)

Quoted 8 lines
Quoted 4 lines

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

Signature · 4 lines
Milian Wolff | [email protected] | Software Engineer
KDAB (Deutschland) GmbH&Co KG, a KDAB Group company
Tel: +49-30-521325470
KDAB - The Qt Experts
maintainer
#7 lore raw

Re: [PATCH v1 2/2] perf report: Display titles in left frame of annotate browser

Em Thu, May 04, 2017 at 04:04:42PM +0200, Milian Wolff escreveu:

Quoted 23 lines · CEST Arnaldo Carvalho de Melo
Quoted 22 lines

Em Thu, May 04, 2017 at 11:01:48AM +0200, Milian Wolff escreveu:

Quoted 21 lines · CEST Jin Yao
Quoted 19 lines

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.

Quoted 8 lines
Quoted 6 lines
Quoted 3 lines

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.

Quoted 10 lines
Quoted 8 lines
Quoted 4 lines

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

merge bot
#8 lore raw

[tip:perf/core] perf annotate browser: Display titles in left frame

In reply to Jin Yao
perf annotate browser: Display titles in left frame
Commit-ID:  ec27ae1892f7f8119ce82535ffcc2889ea3bb3d8
Gitweb:     http://git.kernel.org/tip/ec27ae1892f7f8119ce82535ffcc2889ea3bb3d8
Author:     Jin Yao <[email protected]>
AuthorDate: Thu, 4 May 2017 22:58:15 +0800
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Mon, 19 Jun 2017 15:14:57 -0300

perf annotate browser: Display titles in left frame

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.

Percent  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 Yao Jin <[email protected]>
Acked-by Milian Wolff <[email protected]>
Cc Alexander Shishkin <[email protected]>
Cc Andi Kleen <[email protected]>
Cc Jiri Olsa <[email protected]>
Cc Kan Liang <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Yao Jin <[email protected]>
Link http://lkml.kernel.org/r/[email protected] open here
Signed-off-by Arnaldo Carvalho de Melo <[email protected]>
---
 tools/perf/ui/browsers/annotate.c | 24 +++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)
tools/perf/ui/browsers/annotate.c +21 -3
index 52c1e8d..7a03389 100644
@@ -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,27 @@ 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", 7, "Percent");
 	}
 	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(' ');