perf tools: Cleanup hist printing code (v4)

PATCH RESEND In review Namhyung Kim 5 participants 25 messages Sep 3, 2012 – Sep 19, 2012 perf events

lore

Share & tools

lore

Namhyung Kim

author maintainer

2012-09-03 03:00 UTC #0 lore raw

cover perf tools: Cleanup hist printing code (v4)

Hi,

This is a cleanup and refactoring patchset for the hist printing code by adding perf_hpp__format functions and perf_hpp. I believe it makes the code easy to maintain and to add new features like upcoming group viewing and callchain accumulation.

Any comments are welcome, thanks.
Namhyung

v3 -> v4:
* Rebase to current acme/perf/core
* The first two in the previous series have been merged
* Rename hist_period_print to perf_hpp_fmt (Arnaldo)
* Rename ctx->s to hpp->buf (Arnaldo)

v2 -> v3:
* Move fprintf code to ui/stdio/hist.c (Arnaldo)
* Add ack from Pekka

Namhyung Kim (5):
perf hists: Introduce perf_hpp for hist period printing perf hists: Handle field separator properly
perf hists: Use perf_hpp__format->width to calculate the column widths perf ui/browser: Use perf_hpp__format functions
perf gtk/browser: Use perf_hpp__format functions

 tools/perf/Makefile            |   2 +
 tools/perf/builtin-diff.c      |   1 +
 tools/perf/ui/browsers/hists.c |  94 ++++++++---
 tools/perf/ui/gtk/browser.c    | 101 ++++++++++--
 tools/perf/ui/gtk/gtk.h        |   1 +
 tools/perf/ui/gtk/setup.c      |   1 +
 tools/perf/ui/hist.c           | 366 +++++++++++++++++++++++++++++++++++++++++
 tools/perf/ui/setup.c          |   8 +-
 tools/perf/ui/stdio/hist.c     | 239 +++++----------------------
 tools/perf/ui/tui/setup.c      |   4 +
 tools/perf/util/hist.c         |  33 ----
 tools/perf/util/hist.h         |  37 +++++
 12 files changed, 616 insertions(+), 271 deletions(-)
 create mode 100644 tools/perf/ui/hist.c

Signature · 1 line

1.7.11.4

Namhyung Kim

author maintainer

2012-09-03 03:00 UTC #1 lore raw

PATCH 1/5 perf hists: Introduce perf_hpp for hist period printing

From: Namhyung Kim <[email protected]>

Current hist print functions are messy because it has to consider many of command line options and the code doing that is scattered around to places. So when someone wants to add an option to manipulate the hist output it'd very easy to miss to update all of them in sync. And things getting worse as more options/features are added continuously.

So I'd like to refactor them using hpp formats and move common code to ui/hist.c in order to make it easy to maintain and to add new features.

Signed-off-by Namhyung Kim <[email protected]>

---
 tools/perf/Makefile        |   2 +
 tools/perf/builtin-diff.c  |   1 +
 tools/perf/ui/hist.c       | 316 +++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/ui/setup.c      |   8 +-
 tools/perf/ui/stdio/hist.c | 238 ++++++----------------------------
 tools/perf/util/hist.h     |  37 ++++++
 6 files changed, 402 insertions(+), 200 deletions(-)
 create mode 100644 tools/perf/ui/hist.c

tools/perf/Makefile +2 -0

index 722ddee61f9f..23e5c6ab4276 100644
@@ -403,7 +403,9 @@ LIB_OBJS += $(OUTPUT)util/cgroup.o
 LIB_OBJS += $(OUTPUT)util/target.o
 LIB_OBJS += $(OUTPUT)util/rblist.o
 LIB_OBJS += $(OUTPUT)util/intlist.o
+
 LIB_OBJS += $(OUTPUT)ui/helpline.o
+LIB_OBJS += $(OUTPUT)ui/hist.o
 LIB_OBJS += $(OUTPUT)ui/stdio/hist.o
 
 BUILTIN_OBJS += $(OUTPUT)builtin-annotate.o

tools/perf/builtin-diff.c +1 -0

index d29d350fb2b7..3fc53f8b098e 100644
@@ -235,6 +235,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix __used)
 	if (symbol__init() < 0)
 		return -1;
 
+	perf_hpp__init(true, show_displacement);
 	setup_sorting(diff_usage, options);
 	setup_pager();

tools/perf/ui/hist.c new file +316 -0

316 lines not on this page · raw message

tools/perf/ui/setup.c +5 -3

index c7820e569660..bd7d460f844c 100644
@@ -1,8 +1,8 @@
 #include <pthread.h>
 
-#include "../cache.h"
-#include "../debug.h"
-
+#include "../util/cache.h"
+#include "../util/debug.h"
+#include "../util/hist.h"
 
 pthread_mutex_t ui__lock = PTHREAD_MUTEX_INITIALIZER;

@@ -29,6 +29,8 @@ void setup_browser(bool fallback_to_pager)
 		use_browser = 0;
 		if (fallback_to_pager)
 			setup_pager();
+
+		perf_hpp__init(false, false);
 		break;
 	}
 }

tools/perf/ui/stdio/hist.c +41 ~~-197~~

287 lines not on this page · raw message

tools/perf/util/hist.h +37 -0

index 2e650ffb7d23..4146f51124f0 100644
@@ -115,6 +115,43 @@ bool hists__new_col_len(struct hists *self, enum hist_column col, u16 len);
 void hists__reset_col_len(struct hists *hists);
 void hists__calc_col_len(struct hists *hists, struct hist_entry *he);
 
+struct perf_hpp {
+	char *buf;
+	size_t size;
+	u64 total_period;
+	const char *sep;
+	long displacement;
+	void *ptr;
+};
+
+struct perf_hpp_fmt {
+	bool cond;
+	int (*header)(struct perf_hpp *hpp);
+	int (*width)(struct perf_hpp *hpp);
+	int (*color)(struct perf_hpp *hpp, struct hist_entry *he);
+	int (*entry)(struct perf_hpp *hpp, struct hist_entry *he);
+};
+
+extern struct perf_hpp_fmt perf_hpp__format[];
+
+enum {
+	PERF_HPP__OVERHEAD,
+	PERF_HPP__OVERHEAD_SYS,
+	PERF_HPP__OVERHEAD_US,
+	PERF_HPP__OVERHEAD_GUEST_SYS,
+	PERF_HPP__OVERHEAD_GUEST_US,
+	PERF_HPP__SAMPLES,
+	PERF_HPP__PERIOD,
+	PERF_HPP__DELTA,
+	PERF_HPP__DISPL,
+
+	PERF_HPP__MAX_INDEX
+};
+
+void perf_hpp__init(bool need_pair, bool show_displacement);
+int hist_entry__period_snprintf(struct perf_hpp *hpp, struct hist_entry *he,
+				bool color);
+
 struct perf_evlist;
 
 #ifdef NO_NEWT_SUPPORT

Signature · 1 line

1.7.11.4

tip-bot for Namhyung Kim

2012-09-09 08:55 UTC #2 lore raw

[tip:perf/core] perf hists: Introduce perf_hpp for hist period printing

Commit-ID:  ea251d51d2c7d7233790123227f787c477f567f5
Gitweb:     http://git.kernel.org/tip/ea251d51d2c7d7233790123227f787c477f567f5
Author:     Namhyung Kim <[email protected]>
AuthorDate: Mon, 3 Sep 2012 11:53:06 +0900
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Sat, 8 Sep 2012 13:19:44 -0300

perf hists: Introduce perf_hpp for hist period printing

So I'd like to refactor them using hpp formats and move common code to ui/hist.c in order to make it easy to maintain and to add new features.

Signed-off-by Namhyung Kim <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc Paul Mackerras <[email protected]>

Cc Peter Zijlstra <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Arnaldo Carvalho de Melo <[email protected]>

---
 tools/perf/Makefile        |    2 +
 tools/perf/builtin-diff.c  |    1 +
 tools/perf/ui/hist.c       |  340 ++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/ui/setup.c      |    8 +-
 tools/perf/ui/stdio/hist.c |  238 ++++++-------------------------
 tools/perf/util/hist.h     |   37 +++++
 6 files changed, 426 insertions(+), 200 deletions(-)

tools/perf/Makefile +2 -0

index 3eda492..e4b2e8f 100644
@@ -403,7 +403,9 @@ LIB_OBJS += $(OUTPUT)util/cgroup.o
 LIB_OBJS += $(OUTPUT)util/target.o
 LIB_OBJS += $(OUTPUT)util/rblist.o
 LIB_OBJS += $(OUTPUT)util/intlist.o
+
 LIB_OBJS += $(OUTPUT)ui/helpline.o
+LIB_OBJS += $(OUTPUT)ui/hist.o
 LIB_OBJS += $(OUTPUT)ui/stdio/hist.o
 
 BUILTIN_OBJS += $(OUTPUT)builtin-annotate.o

tools/perf/builtin-diff.c +1 -0

index e9933fd..c4c6d76 100644
@@ -264,6 +264,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix __used)
 	if (symbol__init() < 0)
 		return -1;
 
+	perf_hpp__init(true, show_displacement);
 	setup_sorting(diff_usage, options);
 	setup_pager();

tools/perf/ui/hist.c new file +340 -0

340 lines not on this page · raw message

tools/perf/ui/setup.c +5 -3

index c7820e5..bd7d460 100644
@@ -1,8 +1,8 @@
 #include <pthread.h>
 
-#include "../cache.h"
-#include "../debug.h"
-
+#include "../util/cache.h"
+#include "../util/debug.h"
+#include "../util/hist.h"
 
 pthread_mutex_t ui__lock = PTHREAD_MUTEX_INITIALIZER;

@@ -29,6 +29,8 @@ void setup_browser(bool fallback_to_pager)
 		use_browser = 0;
 		if (fallback_to_pager)
 			setup_pager();
+
+		perf_hpp__init(false, false);
 		break;
 	}
 }

tools/perf/ui/stdio/hist.c +41 ~~-197~~

287 lines not on this page · raw message

tools/perf/util/hist.h +37 -0

index 2e650ff..4146f51 100644
@@ -115,6 +115,43 @@ bool hists__new_col_len(struct hists *self, enum hist_column col, u16 len);
 void hists__reset_col_len(struct hists *hists);
 void hists__calc_col_len(struct hists *hists, struct hist_entry *he);
 
+struct perf_hpp {
+	char *buf;
+	size_t size;
+	u64 total_period;
+	const char *sep;
+	long displacement;
+	void *ptr;
+};
+
+struct perf_hpp_fmt {
+	bool cond;
+	int (*header)(struct perf_hpp *hpp);
+	int (*width)(struct perf_hpp *hpp);
+	int (*color)(struct perf_hpp *hpp, struct hist_entry *he);
+	int (*entry)(struct perf_hpp *hpp, struct hist_entry *he);
+};
+
+extern struct perf_hpp_fmt perf_hpp__format[];
+
+enum {
+	PERF_HPP__OVERHEAD,
+	PERF_HPP__OVERHEAD_SYS,
+	PERF_HPP__OVERHEAD_US,
+	PERF_HPP__OVERHEAD_GUEST_SYS,
+	PERF_HPP__OVERHEAD_GUEST_US,
+	PERF_HPP__SAMPLES,
+	PERF_HPP__PERIOD,
+	PERF_HPP__DELTA,
+	PERF_HPP__DISPL,
+
+	PERF_HPP__MAX_INDEX
+};
+
+void perf_hpp__init(bool need_pair, bool show_displacement);
+int hist_entry__period_snprintf(struct perf_hpp *hpp, struct hist_entry *he,
+				bool color);
+
 struct perf_evlist;
 
 #ifdef NO_NEWT_SUPPORT

Robert Richter

2012-09-12 17:28 UTC #3 lore raw

Re: [tip:perf/core] perf hists: Introduce perf_hpp for hist period printing

Quoted 24 lines · tip-bot for Namhyung Kim

Commit-ID:  ea251d51d2c7d7233790123227f787c477f567f5
Gitweb:     http://git.kernel.org/tip/ea251d51d2c7d7233790123227f787c477f567f5
Author:     Namhyung Kim <[email protected]>
AuthorDate: Mon, 3 Sep 2012 11:53:06 +0900
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Sat, 8 Sep 2012 13:19:44 -0300
perf hists: Introduce perf_hpp for hist period printing
Current hist print functions are messy because it has to consider many of command line options and the code doing that is scattered around to places. So when someone wants to add an option to manipulate the hist output it'd very easy to miss to update all of them in sync. And things getting worse as more options/features are added continuously.
So I'd like to refactor them using hpp formats and move common code to ui/hist.c in order to make it easy to maintain and to add new features.
Signed-off-by Namhyung Kim <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Paul Mackerras <[email protected]>
Cc Peter Zijlstra <[email protected]>
Link http://lkml.kernel.org/r/[email protected] open here
Signed-off-by Arnaldo Carvalho de Melo <[email protected]>

Quoted 8 lines

---
 tools/perf/Makefile        |    2 +
 tools/perf/builtin-diff.c  |    1 +
 tools/perf/ui/hist.c       |  340 ++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/ui/setup.c      |    8 +-
 tools/perf/ui/stdio/hist.c |  238 ++++++-------------------------
 tools/perf/util/hist.h     |   37 +++++
 6 files changed, 426 insertions(+), 200 deletions(-)

This patch breaks perf-record/report that the number of samples can't be shown in pipe mode:

 # perf record -e cycles -aq sleep 1 ; perf report -n --sort comm,dso | sed '/%/q;d' ; \
   perf record -e cycles -aq sleep 1 | perf report -n --sort comm,dso | sed '/%/q;d'
     99.86%        11804       swapper  [kernel.kallsyms]
     91.57%      swapper  [kernel.kallsyms]
           ^^^^^^
           number of samples missing

Moving and changing the code at the same time make the patch unreviewable. So no clue that's the problem here.

-Robert

Signature · 2 lines

Advanced Micro Devices, Inc.
Operating System Research Center

Arnaldo Carvalho de Melo

2012-09-12 18:48 UTC #4 lore raw

Re: [tip:perf/core] perf hists: Introduce perf_hpp for hist period printing

Em Wed, Sep 12, 2012 at 07:26:41PM +0200, Robert Richter escreveu:

Quoted 11 lines · tip-bot for Namhyung Kim

 # perf record -e cycles -aq sleep 1 ; perf report -n --sort comm,dso | sed '/%/q;d' ; \
   perf record -e cycles -aq sleep 1 | perf report -n --sort comm,dso | sed '/%/q;d'
     99.86%        11804       swapper  [kernel.kallsyms]
     91.57%      swapper  [kernel.kallsyms]
           ^^^^^^
           number of samples missing

Moving and changing the code at the same time make the patch unreviewable. So no clue that's the problem here.

Yeah, that was unfortunate (movind and changing), we need to try hard not to do that...

Anyway, Namhyung, can you take a look at this?

- Arnaldo

Namhyung Kim

author maintainer

2012-09-13 02:50 UTC #5 lore raw

Re: [tip:perf/core] perf hists: Introduce perf_hpp for hist period printing

Hi Arnaldo and Robert,

Quoted 10 lines · Arnaldo Carvalho de Melo

Em Wed, Sep 12, 2012 at 07:26:41PM +0200, Robert Richter escreveu:

Quoted 9 lines · tip-bot for Namhyung Kim

 # perf record -e cycles -aq sleep 1 ; perf report -n --sort comm,dso | sed '/%/q;d' ; \
   perf record -e cycles -aq sleep 1 | perf report -n --sort comm,dso | sed '/%/q;d'
     99.86%        11804       swapper  [kernel.kallsyms]
     91.57%      swapper  [kernel.kallsyms]
           ^^^^^^
           number of samples missing

Quoted 5 lines

Quoted 2 lines
Moving and changing the code at the same time make the patch unreviewable. So no clue that's the problem here.
Yeah, that was unfortunate (movind and changing), we need to try hard not to do that...

Okay, will be careful next time.

Quoted 2 lines

Anyway, Namhyung, can you take a look at this?

Sure, I'll take a look.

Thanks,
Namhyung

Namhyung Kim

author maintainer

2012-09-13 04:21 UTC #6 lore raw

PATCH 1/1 perf record: Add missing perf_hpp__init for pipe-mode

From: Namhyung Kim <[email protected]>

The perf_hpp__init() function was only called from setup_browser() so that the pipe-mode missed the initialization thus didn't respond to related options. Fix it.

Reported-by Robert Richter <[email protected]>

Signed-off-by Namhyung Kim <[email protected]>

---
 tools/perf/builtin-report.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

tools/perf/builtin-report.c +3 -1

index 97b2e6300f4c..279155a47d1c 100644
@@ -689,8 +689,10 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
 
 	if (strcmp(report.input_name, "-") != 0)
 		setup_browser(true);
-	else
+	else {
 		use_browser = 0;
+		perf_hpp__init(false, false);
+	}
 
 	/*
 	 * Only in the newt browser we are doing integrated annotation,

Signature · 1 line

1.7.11.4

Namhyung Kim

author maintainer

2012-09-13 04:28 UTC #7 lore raw

Re: [PATCH] perf record: Add missing perf_hpp__init for pipe-mode

Oops, please do s/record/report/ on the subject line, sorry! ;-)

Also I don't see the issue on the other users of the setup_browser() - i.e. perf annotate and perf top - since they call it unconditionally.

Thanks,
Namhyung

Robert Richter

2012-09-13 09:48 UTC #8 lore raw

Re: [PATCH] perf record: Add missing perf_hpp__init for pipe-mode

In reply to Namhyung Kim

Quoted 11 lines · Namhyung Kim

From: Namhyung Kim <[email protected]>
The perf_hpp__init() function was only called from setup_browser() so that the pipe-mode missed the initialization thus didn't respond to related options. Fix it.
Reported-by Robert Richter <[email protected]>
Signed-off-by Namhyung Kim <[email protected]>
---
 tools/perf/builtin-report.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

This patch fixes it.

Thanks,

-Robert

Quoted 21 lines

tools/perf/builtin-report.c +3 -1

index 97b2e6300f4c..279155a47d1c 100644
@@ -689,8 +689,10 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
 
 	if (strcmp(report.input_name, "-") != 0)
 		setup_browser(true);
-	else
+	else {
 		use_browser = 0;
+		perf_hpp__init(false, false);
+	}
 
 	/*
 	 * Only in the newt browser we are doing integrated annotation,

Signature · 1 line

1.7.11.4

Signature · 2 lines

Advanced Micro Devices, Inc.
Operating System Research Center

tip-bot for Namhyung Kim

2012-09-19 15:25 UTC #9 lore raw

[tip:perf/core] perf report: Add missing perf_hpp__init for pipe-mode

In reply to Namhyung Kim

Commit-ID:  60e5c706b3ea56f87afc2a4a3096118d28f9cc24
Gitweb:     http://git.kernel.org/tip/60e5c706b3ea56f87afc2a4a3096118d28f9cc24
Author:     Namhyung Kim <[email protected]>
AuthorDate: Thu, 13 Sep 2012 13:14:30 +0900
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Mon, 17 Sep 2012 13:11:33 -0300

perf report: Add missing perf_hpp__init for pipe-mode

The perf_hpp__init() function was only called from setup_browser() so that the pipe-mode missed the initialization thus didn't respond to related options. Fix it.

Reported-by Robert Richter <[email protected]>

Tested-by Robert Richter <[email protected]>

Signed-off-by Namhyung Kim <[email protected]>

Cc H. Peter Anvin <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc Paul Mackerras <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Robert Richter <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc [email protected]

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Arnaldo Carvalho de Melo <[email protected]>

---
 tools/perf/builtin-report.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

tools/perf/builtin-report.c +3 -1

index b6696dd..1da243d 100644
@@ -689,8 +689,10 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
 
 	if (strcmp(report.input_name, "-") != 0)
 		setup_browser(true);
-	else
+	else {
 		use_browser = 0;
+		perf_hpp__init(false, false);
+	}
 
 	setup_sorting(report_usage, options);

Arnaldo Carvalho de Melo

2012-09-13 14:42 UTC #10 lore raw

Re: [tip:perf/core] perf hists: Introduce perf_hpp for hist period printing

In reply to Namhyung Kim

Em Thu, Sep 13, 2012 at 11:43:08AM +0900, Namhyung Kim escreveu:

Quoted 9 lines · Arnaldo Carvalho de Melo

Quoted 6 lines
Em Wed, Sep 12, 2012 at 07:26:41PM +0200, Robert Richter escreveu:
Quoted 2 lines
Moving and changing the code at the same time make the patch unreviewable. So no clue that's the problem here.
Yeah, that was unfortunate (movind and changing), we need to try hard not to do that...
Okay, will be careful next time.

Its a neverending discipline to follow :-\ But it can be fun, keep it up!

Quoted 3 lines

Quoted 1 line
Anyway, Namhyung, can you take a look at this?
Sure, I'll take a look.

Thanks!

- Arnaldo

Namhyung Kim

author maintainer

2012-09-03 03:00 UTC #11 lore raw

PATCH 2/5 perf hists: Handle field separator properly

In reply to Namhyung Kim

From: Namhyung Kim <[email protected]>

When a field separator is given, the output format doesn't need to be fancy like aligning to column length, coloring the percent value and so on. And since there's a slight difference to normal format, fix it not to break backward compatibility.

Signed-off-by Namhyung Kim <[email protected]>

---
 tools/perf/ui/hist.c       | 75 ++++++++++++++++++++++++++++++----------------
 tools/perf/ui/stdio/hist.c |  3 +-
 2 files changed, 51 insertions(+), 27 deletions(-)

tools/perf/ui/hist.c +49 ~~-26~~

index c9a566cfd9c2..802a8659c15a 100644
@@ -8,10 +8,9 @@
 /* hist period print (hpp) functions */
 static int hpp__header_overhead(struct perf_hpp *hpp)
 {
-	if (hpp->ptr)
-		return scnprintf(hpp->buf, hpp->size, "Baseline");
-	else
-		return scnprintf(hpp->buf, hpp->size, "Overhead");
+	const char *fmt = hpp->ptr ? "Baseline" : "Overhead";
+
+	return scnprintf(hpp->buf, hpp->size, fmt);
 }
 
 static int hpp__width_overhead(struct perf_hpp *hpp __used)

@@ -28,12 +27,16 @@ static int hpp__color_overhead(struct perf_hpp *hpp, struct hist_entry *he)
 static int hpp__entry_overhead(struct perf_hpp *hpp, struct hist_entry *he)
 {
 	double percent = 100.0 * he->period / hpp->total_period;
-	return scnprintf(hpp->buf, hpp->size, "  %5.2f%%", percent);
+	const char *fmt = symbol_conf.field_sep ? "%.2f" : "  %5.2f%%";
+
+	return scnprintf(hpp->buf, hpp->size, fmt, percent);
 }
 
 static int hpp__header_overhead_sys(struct perf_hpp *hpp)
 {
-	return scnprintf(hpp->buf, hpp->size, " sys  ");
+	const char *fmt = symbol_conf.field_sep ? "%s" : "%6s";
+
+	return scnprintf(hpp->buf, hpp->size, fmt, "sys");
 }
 
 static int hpp__width_overhead_sys(struct perf_hpp *hpp __used)

@@ -50,12 +53,16 @@ static int hpp__color_overhead_sys(struct perf_hpp *hpp, struct hist_entry *he)
 static int hpp__entry_overhead_sys(struct perf_hpp *hpp, struct hist_entry *he)
 {
 	double percent = 100.0 * he->period_sys / hpp->total_period;
-	return scnprintf(hpp->buf, hpp->size, "%5.2f%%", percent);
+	const char *fmt = symbol_conf.field_sep ? "%.2f" : "%5.2f%%";
+
+	return scnprintf(hpp->buf, hpp->size, fmt, percent);
 }
 
 static int hpp__header_overhead_us(struct perf_hpp *hpp)
 {
-	return scnprintf(hpp->buf, hpp->size, " user ");
+	const char *fmt = symbol_conf.field_sep ? "%s" : "%6s";
+
+	return scnprintf(hpp->buf, hpp->size, fmt, "user");
 }
 
 static int hpp__width_overhead_us(struct perf_hpp *hpp __used)

@@ -72,7 +79,9 @@ static int hpp__color_overhead_us(struct perf_hpp *hpp, struct hist_entry *he)
 static int hpp__entry_overhead_us(struct perf_hpp *hpp, struct hist_entry *he)
 {
 	double percent = 100.0 * he->period_us / hpp->total_period;
-	return scnprintf(hpp->buf, hpp->size, "%5.2f%%", percent);
+	const char *fmt = symbol_conf.field_sep ? "%.2f" : "%5.2f%%";
+
+	return scnprintf(hpp->buf, hpp->size, fmt, percent);
 }
 
 static int hpp__header_overhead_guest_sys(struct perf_hpp *hpp)

@@ -96,7 +105,9 @@ static int hpp__entry_overhead_guest_sys(struct perf_hpp *hpp,
 					 struct hist_entry *he)
 {
 	double percent = 100.0 * he->period_guest_sys / hpp->total_period;
-	return scnprintf(hpp->buf, hpp->size, "  %5.2f%% ", percent);
+	const char *fmt = symbol_conf.field_sep ? "%.2f" : "  %5.2f%% ";
+
+	return scnprintf(hpp->buf, hpp->size, fmt, percent);
 }
 
 static int hpp__header_overhead_guest_us(struct perf_hpp *hpp)

@@ -120,12 +131,16 @@ static int hpp__entry_overhead_guest_us(struct perf_hpp *hpp,
 					struct hist_entry *he)
 {
 	double percent = 100.0 * he->period_guest_us / hpp->total_period;
-	return scnprintf(hpp->buf, hpp->size, "  %5.2f%% ", percent);
+	const char *fmt = symbol_conf.field_sep ? "%.2f" : "  %5.2f%% ";
+
+	return scnprintf(hpp->buf, hpp->size, fmt, percent);
 }
 
 static int hpp__header_samples(struct perf_hpp *hpp)
 {
-	return scnprintf(hpp->buf, hpp->size, "  Samples  ");
+	const char *fmt = symbol_conf.field_sep ? "%s" : "%11s";
+
+	return scnprintf(hpp->buf, hpp->size, fmt, "Samples");
 }
 
 static int hpp__width_samples(struct perf_hpp *hpp __used)

@@ -135,12 +150,16 @@ static int hpp__width_samples(struct perf_hpp *hpp __used)
 
 static int hpp__entry_samples(struct perf_hpp *hpp, struct hist_entry *he)
 {
-	return scnprintf(hpp->buf, hpp->size, "%11" PRIu64, he->nr_events);
+	const char *fmt = symbol_conf.field_sep ? "%" PRIu64 : "%11" PRIu64;
+
+	return scnprintf(hpp->buf, hpp->size, fmt, he->nr_events);
 }
 
 static int hpp__header_period(struct perf_hpp *hpp)
 {
-	return scnprintf(hpp->buf, hpp->size, "   Period   ");
+	const char *fmt = symbol_conf.field_sep ? "%s" : "%12s";
+
+	return scnprintf(hpp->buf, hpp->size, fmt, "Period");
 }
 
 static int hpp__width_period(struct perf_hpp *hpp __used)

@@ -150,12 +169,16 @@ static int hpp__width_period(struct perf_hpp *hpp __used)
 
 static int hpp__entry_period(struct perf_hpp *hpp, struct hist_entry *he)
 {
-	return scnprintf(hpp->buf, hpp->size, "%12" PRIu64, he->period);
+	const char *fmt = symbol_conf.field_sep ? "%" PRIu64 : "%12" PRIu64;
+
+	return scnprintf(hpp->buf, hpp->size, fmt, he->period);
 }
 
 static int hpp__header_delta(struct perf_hpp *hpp)
 {
-	return scnprintf(hpp->buf, hpp->size, " Delta ");
+	const char *fmt = symbol_conf.field_sep ? "%s" : "%7s";
+
+	return scnprintf(hpp->buf, hpp->size, fmt, "Delta");
 }
 
 static int hpp__width_delta(struct perf_hpp *hpp __used)

@@ -169,7 +192,8 @@ static int hpp__entry_delta(struct perf_hpp *hpp, struct hist_entry *he)
 	u64 old_total, new_total;
 	double old_percent = 0, new_percent = 0;
 	double diff;
-	char buf[32];
+	const char *fmt = symbol_conf.field_sep ? "%s" : "%7.7s";
+	char buf[32] = " ";
 
 	old_total = pair_hists->stats.total_period;
 	if (old_total > 0)

@@ -180,11 +204,10 @@ static int hpp__entry_delta(struct perf_hpp *hpp, struct hist_entry *he)
 		new_percent = 100.0 * he->period / new_total;
 
 	diff = new_percent - old_percent;
-	if (fabs(diff) < 0.01)
-		return scnprintf(hpp->buf, hpp->size, "       ");
+	if (fabs(diff) >= 0.01)
+		scnprintf(buf, sizeof(buf), "%+4.2F%%", diff);
 
-	scnprintf(buf, sizeof(buf), "%+4.2F%%", diff);
-	return scnprintf(hpp->buf, hpp->size, "%7.7s", buf);
+	return scnprintf(hpp->buf, hpp->size, fmt, buf);
 }
 
 static int hpp__header_displ(struct perf_hpp *hpp)

@@ -199,13 +222,13 @@ static int hpp__width_displ(struct perf_hpp *hpp __used)
 
 static int hpp__entry_displ(struct perf_hpp *hpp, struct hist_entry *he __used)
 {
-	char buf[32];
+	const char *fmt = symbol_conf.field_sep ? "%s" : "%6.6s";
+	char buf[32] = " ";
 
-	if (!hpp->displacement)
-		return scnprintf(hpp->buf, hpp->size, "     ");
+	if (hpp->displacement)
+		scnprintf(buf, sizeof(buf), "%+4ld", hpp->displacement);
 
-	scnprintf(buf, sizeof(buf), "%+4ld", hpp->displacement);
-	return scnprintf(hpp->buf, hpp->size, "%6.6s", buf);
+	return scnprintf(hpp->buf, hpp->size, fmt, buf);
 }
 
 #define HPP__COLOR_PRINT_FNS(_name)		\

tools/perf/ui/stdio/hist.c +2 -1

index 4228b4c6b72d..882461a42830 100644
@@ -319,11 +319,12 @@ static int hist_entry__fprintf(struct hist_entry *he, size_t size,
 		.displacement	= displacement,
 		.ptr		= pair_hists,
 	};
+	bool color = !symbol_conf.field_sep;
 
 	if (size == 0 || size > sizeof(bf))
 		size = hpp.size = sizeof(bf);
 
-	ret = hist_entry__period_snprintf(&hpp, he, true);
+	ret = hist_entry__period_snprintf(&hpp, he, color);
 	hist_entry__sort_snprintf(he, bf + ret, size - ret, hists);
 
 	ret = fprintf(fp, "%s\n", bf);

Signature · 1 line

1.7.11.4

tip-bot for Namhyung Kim

2012-09-09 08:55 UTC #12 lore raw

[tip:perf/core] perf hists: Handle field separator properly

Commit-ID:  9ffad987ff565999d91fc2783dd77f08094a743b
Gitweb:     http://git.kernel.org/tip/9ffad987ff565999d91fc2783dd77f08094a743b
Author:     Namhyung Kim <[email protected]>
AuthorDate: Mon, 3 Sep 2012 11:53:07 +0900
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Sat, 8 Sep 2012 13:19:55 -0300

perf hists: Handle field separator properly

Signed-off-by Namhyung Kim <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc Paul Mackerras <[email protected]>

Cc Peter Zijlstra <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Arnaldo Carvalho de Melo <[email protected]>

---
 tools/perf/ui/hist.c       |   74 ++++++++++++++++++++++++++++---------------
 tools/perf/ui/stdio/hist.c |    3 +-
 2 files changed, 50 insertions(+), 27 deletions(-)

tools/perf/ui/hist.c +48 ~~-26~~

index 8ccd1f2..009adf20 100644
@@ -8,10 +8,9 @@
 /* hist period print (hpp) functions */
 static int hpp__header_overhead(struct perf_hpp *hpp)
 {
-	if (hpp->ptr)
-		return scnprintf(hpp->buf, hpp->size, "Baseline");
-	else
-		return scnprintf(hpp->buf, hpp->size, "Overhead");
+	const char *fmt = hpp->ptr ? "Baseline" : "Overhead";
+
+	return scnprintf(hpp->buf, hpp->size, fmt);
 }
 
 static int hpp__width_overhead(struct perf_hpp *hpp __used)

@@ -40,6 +39,7 @@ static int hpp__color_overhead(struct perf_hpp *hpp, struct hist_entry *he)
 static int hpp__entry_overhead(struct perf_hpp *hpp, struct hist_entry *he)
 {
 	double percent = 100.0 * he->period / hpp->total_period;
+	const char *fmt = symbol_conf.field_sep ? "%.2f" : "  %5.2f%%";
 
 	if (hpp->ptr) {
 		struct hists *old_hists = hpp->ptr;

@@ -52,12 +52,14 @@ static int hpp__entry_overhead(struct perf_hpp *hpp, struct hist_entry *he)
 			percent = 0.0;
 	}
 
-	return scnprintf(hpp->buf, hpp->size, "  %5.2f%%", percent);
+	return scnprintf(hpp->buf, hpp->size, fmt, percent);
 }
 
 static int hpp__header_overhead_sys(struct perf_hpp *hpp)
 {
-	return scnprintf(hpp->buf, hpp->size, " sys  ");
+	const char *fmt = symbol_conf.field_sep ? "%s" : "%6s";
+
+	return scnprintf(hpp->buf, hpp->size, fmt, "sys");
 }
 
 static int hpp__width_overhead_sys(struct perf_hpp *hpp __used)

@@ -74,12 +76,16 @@ static int hpp__color_overhead_sys(struct perf_hpp *hpp, struct hist_entry *he)
 static int hpp__entry_overhead_sys(struct perf_hpp *hpp, struct hist_entry *he)
 {
 	double percent = 100.0 * he->period_sys / hpp->total_period;
-	return scnprintf(hpp->buf, hpp->size, "%5.2f%%", percent);
+	const char *fmt = symbol_conf.field_sep ? "%.2f" : "%5.2f%%";
+
+	return scnprintf(hpp->buf, hpp->size, fmt, percent);
 }
 
 static int hpp__header_overhead_us(struct perf_hpp *hpp)
 {
-	return scnprintf(hpp->buf, hpp->size, " user ");
+	const char *fmt = symbol_conf.field_sep ? "%s" : "%6s";
+
+	return scnprintf(hpp->buf, hpp->size, fmt, "user");
 }
 
 static int hpp__width_overhead_us(struct perf_hpp *hpp __used)

@@ -96,7 +102,9 @@ static int hpp__color_overhead_us(struct perf_hpp *hpp, struct hist_entry *he)
 static int hpp__entry_overhead_us(struct perf_hpp *hpp, struct hist_entry *he)
 {
 	double percent = 100.0 * he->period_us / hpp->total_period;
-	return scnprintf(hpp->buf, hpp->size, "%5.2f%%", percent);
+	const char *fmt = symbol_conf.field_sep ? "%.2f" : "%5.2f%%";
+
+	return scnprintf(hpp->buf, hpp->size, fmt, percent);
 }
 
 static int hpp__header_overhead_guest_sys(struct perf_hpp *hpp)

@@ -120,7 +128,9 @@ static int hpp__entry_overhead_guest_sys(struct perf_hpp *hpp,
 					 struct hist_entry *he)
 {
 	double percent = 100.0 * he->period_guest_sys / hpp->total_period;
-	return scnprintf(hpp->buf, hpp->size, "  %5.2f%% ", percent);
+	const char *fmt = symbol_conf.field_sep ? "%.2f" : "  %5.2f%% ";
+
+	return scnprintf(hpp->buf, hpp->size, fmt, percent);
 }
 
 static int hpp__header_overhead_guest_us(struct perf_hpp *hpp)

@@ -144,12 +154,16 @@ static int hpp__entry_overhead_guest_us(struct perf_hpp *hpp,
 					struct hist_entry *he)
 {
 	double percent = 100.0 * he->period_guest_us / hpp->total_period;
-	return scnprintf(hpp->buf, hpp->size, "  %5.2f%% ", percent);
+	const char *fmt = symbol_conf.field_sep ? "%.2f" : "  %5.2f%% ";
+
+	return scnprintf(hpp->buf, hpp->size, fmt, percent);
 }
 
 static int hpp__header_samples(struct perf_hpp *hpp)
 {
-	return scnprintf(hpp->buf, hpp->size, "  Samples  ");
+	const char *fmt = symbol_conf.field_sep ? "%s" : "%11s";
+
+	return scnprintf(hpp->buf, hpp->size, fmt, "Samples");
 }
 
 static int hpp__width_samples(struct perf_hpp *hpp __used)

@@ -159,12 +173,16 @@ static int hpp__width_samples(struct perf_hpp *hpp __used)
 
 static int hpp__entry_samples(struct perf_hpp *hpp, struct hist_entry *he)
 {
-	return scnprintf(hpp->buf, hpp->size, "%11" PRIu64, he->nr_events);
+	const char *fmt = symbol_conf.field_sep ? "%" PRIu64 : "%11" PRIu64;
+
+	return scnprintf(hpp->buf, hpp->size, fmt, he->nr_events);
 }
 
 static int hpp__header_period(struct perf_hpp *hpp)
 {
-	return scnprintf(hpp->buf, hpp->size, "   Period   ");
+	const char *fmt = symbol_conf.field_sep ? "%s" : "%12s";
+
+	return scnprintf(hpp->buf, hpp->size, fmt, "Period");
 }
 
 static int hpp__width_period(struct perf_hpp *hpp __used)

@@ -174,12 +192,16 @@ static int hpp__width_period(struct perf_hpp *hpp __used)
 
 static int hpp__entry_period(struct perf_hpp *hpp, struct hist_entry *he)
 {
-	return scnprintf(hpp->buf, hpp->size, "%12" PRIu64, he->period);
+	const char *fmt = symbol_conf.field_sep ? "%" PRIu64 : "%12" PRIu64;
+
+	return scnprintf(hpp->buf, hpp->size, fmt, he->period);
 }
 
 static int hpp__header_delta(struct perf_hpp *hpp)
 {
-	return scnprintf(hpp->buf, hpp->size, " Delta ");
+	const char *fmt = symbol_conf.field_sep ? "%s" : "%7s";
+
+	return scnprintf(hpp->buf, hpp->size, fmt, "Delta");
 }
 
 static int hpp__width_delta(struct perf_hpp *hpp __used)

@@ -193,7 +215,8 @@ static int hpp__entry_delta(struct perf_hpp *hpp, struct hist_entry *he)
 	u64 old_total, new_total;
 	double old_percent = 0, new_percent = 0;
 	double diff;
-	char buf[32];
+	const char *fmt = symbol_conf.field_sep ? "%s" : "%7.7s";
+	char buf[32] = " ";
 
 	old_total = pair_hists->stats.total_period;
 	if (old_total > 0 && he->pair)

@@ -204,11 +227,10 @@ static int hpp__entry_delta(struct perf_hpp *hpp, struct hist_entry *he)
 		new_percent = 100.0 * he->period / new_total;
 
 	diff = new_percent - old_percent;
-	if (fabs(diff) < 0.01)
-		return scnprintf(hpp->buf, hpp->size, "       ");
+	if (fabs(diff) >= 0.01)
+		scnprintf(buf, sizeof(buf), "%+4.2F%%", diff);
 
-	scnprintf(buf, sizeof(buf), "%+4.2F%%", diff);
-	return scnprintf(hpp->buf, hpp->size, "%7.7s", buf);
+	return scnprintf(hpp->buf, hpp->size, fmt, buf);
 }
 
 static int hpp__header_displ(struct perf_hpp *hpp)

@@ -223,13 +245,13 @@ static int hpp__width_displ(struct perf_hpp *hpp __used)
 
 static int hpp__entry_displ(struct perf_hpp *hpp, struct hist_entry *he __used)
 {
-	char buf[32];
+	const char *fmt = symbol_conf.field_sep ? "%s" : "%6.6s";
+	char buf[32] = " ";
 
-	if (!hpp->displacement)
-		return scnprintf(hpp->buf, hpp->size, "     ");
+	if (hpp->displacement)
+		scnprintf(buf, sizeof(buf), "%+4ld", hpp->displacement);
 
-	scnprintf(buf, sizeof(buf), "%+4ld", hpp->displacement);
-	return scnprintf(hpp->buf, hpp->size, "%6.6s", buf);
+	return scnprintf(hpp->buf, hpp->size, fmt, buf);
 }
 
 #define HPP__COLOR_PRINT_FNS(_name)		\

tools/perf/ui/stdio/hist.c +2 -1

index 4228b4c..882461a 100644
@@ -319,11 +319,12 @@ static int hist_entry__fprintf(struct hist_entry *he, size_t size,
 		.displacement	= displacement,
 		.ptr		= pair_hists,
 	};
+	bool color = !symbol_conf.field_sep;
 
 	if (size == 0 || size > sizeof(bf))
 		size = hpp.size = sizeof(bf);
 
-	ret = hist_entry__period_snprintf(&hpp, he, true);
+	ret = hist_entry__period_snprintf(&hpp, he, color);
 	hist_entry__sort_snprintf(he, bf + ret, size - ret, hists);
 
 	ret = fprintf(fp, "%s\n", bf);

Namhyung Kim

author maintainer

2012-09-03 03:00 UTC #13 lore raw

PATCH 3/5 perf hists: Use perf_hpp__format->width to calculate the column widths

In reply to Namhyung Kim

From: Namhyung Kim <[email protected]>

Signed-off-by Namhyung Kim <[email protected]>

---
 tools/perf/ui/hist.c   | 27 +++++++++++++++++++++++++++
 tools/perf/util/hist.c | 33 ---------------------------------
 2 files changed, 27 insertions(+), 33 deletions(-)

tools/perf/ui/hist.c +27 -0

index 802a8659c15a..16dc486d02be 100644
@@ -337,3 +337,30 @@ int hist_entry__sort_snprintf(struct hist_entry *he, char *s, size_t size,
 
 	return ret;
 }
+
+/*
+ * See hists__fprintf to match the column widths
+ */
+unsigned int hists__sort_list_width(struct hists *hists)
+{
+	struct sort_entry *se;
+	int i, ret = 0;
+
+	for (i = 0; i < PERF_HPP__MAX_INDEX; i++) {
+		if (!perf_hpp__format[i].cond)
+			continue;
+		if (i)
+			ret += 2;
+
+		ret += perf_hpp__format[i].width(NULL);
+	}
+
+	list_for_each_entry(se, &hist_entry__sort_list, list)
+		if (!se->elide)
+			ret += 2 + hists__col_len(hists, se->se_width_idx);
+
+	if (verbose) /* Addr + origin */
+		ret += 3 + BITS_PER_LONG / 4;
+
+	return ret;
+}

tools/perf/util/hist.c +0 ~~-33~~

index b1817f15bb87..0ba65ad07cd1 100644
@@ -563,39 +563,6 @@ void hists__output_resort_threaded(struct hists *hists)
 	return __hists__output_resort(hists, true);
 }
 
-/*
- * See hists__fprintf to match the column widths
- */
-unsigned int hists__sort_list_width(struct hists *hists)
-{
-	struct sort_entry *se;
-	int ret = 9; /* total % */
-
-	if (symbol_conf.show_cpu_utilization) {
-		ret += 7; /* count_sys % */
-		ret += 6; /* count_us % */
-		if (perf_guest) {
-			ret += 13; /* count_guest_sys % */
-			ret += 12; /* count_guest_us % */
-		}
-	}
-
-	if (symbol_conf.show_nr_samples)
-		ret += 11;
-
-	if (symbol_conf.show_total_period)
-		ret += 13;
-
-	list_for_each_entry(se, &hist_entry__sort_list, list)
-		if (!se->elide)
-			ret += 2 + hists__col_len(hists, se->se_width_idx);
-
-	if (verbose) /* Addr + origin */
-		ret += 3 + BITS_PER_LONG / 4;
-
-	return ret;
-}
-
 static void hists__remove_entry_filter(struct hists *hists, struct hist_entry *h,
 				       enum hist_filter filter)
 {

Signature · 1 line

1.7.11.4

tip-bot for Namhyung Kim

2012-09-09 08:56 UTC #14 lore raw

[tip:perf/core] perf hists: Use perf_hpp__format-> width to calculate the column widths

Commit-ID:  7e62ef44e89e7b7f2c48090a048f2a5dffa838c7
Gitweb:     http://git.kernel.org/tip/7e62ef44e89e7b7f2c48090a048f2a5dffa838c7
Author:     Namhyung Kim <[email protected]>
AuthorDate: Mon, 3 Sep 2012 11:53:08 +0900
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Sat, 8 Sep 2012 13:20:05 -0300

perf hists: Use perf_hpp__format->width to calculate the column widths

Signed-off-by Namhyung Kim <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc Paul Mackerras <[email protected]>

Cc Peter Zijlstra <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Arnaldo Carvalho de Melo <[email protected]>

---
 tools/perf/ui/hist.c   |   27 +++++++++++++++++++++++++++
 tools/perf/util/hist.c |   33 ---------------------------------
 2 files changed, 27 insertions(+), 33 deletions(-)

tools/perf/ui/hist.c +27 -0

index 009adf20..031b349 100644
@@ -360,3 +360,30 @@ int hist_entry__sort_snprintf(struct hist_entry *he, char *s, size_t size,
 
 	return ret;
 }
+
+/*
+ * See hists__fprintf to match the column widths
+ */
+unsigned int hists__sort_list_width(struct hists *hists)
+{
+	struct sort_entry *se;
+	int i, ret = 0;
+
+	for (i = 0; i < PERF_HPP__MAX_INDEX; i++) {
+		if (!perf_hpp__format[i].cond)
+			continue;
+		if (i)
+			ret += 2;
+
+		ret += perf_hpp__format[i].width(NULL);
+	}
+
+	list_for_each_entry(se, &hist_entry__sort_list, list)
+		if (!se->elide)
+			ret += 2 + hists__col_len(hists, se->se_width_idx);
+
+	if (verbose) /* Addr + origin */
+		ret += 3 + BITS_PER_LONG / 4;
+
+	return ret;
+}

tools/perf/util/hist.c +0 ~~-33~~

index b1817f1..0ba65ad 100644
@@ -563,39 +563,6 @@ void hists__output_resort_threaded(struct hists *hists)
 	return __hists__output_resort(hists, true);
 }
 
-/*
- * See hists__fprintf to match the column widths
- */
-unsigned int hists__sort_list_width(struct hists *hists)
-{
-	struct sort_entry *se;
-	int ret = 9; /* total % */
-
-	if (symbol_conf.show_cpu_utilization) {
-		ret += 7; /* count_sys % */
-		ret += 6; /* count_us % */
-		if (perf_guest) {
-			ret += 13; /* count_guest_sys % */
-			ret += 12; /* count_guest_us % */
-		}
-	}
-
-	if (symbol_conf.show_nr_samples)
-		ret += 11;
-
-	if (symbol_conf.show_total_period)
-		ret += 13;
-
-	list_for_each_entry(se, &hist_entry__sort_list, list)
-		if (!se->elide)
-			ret += 2 + hists__col_len(hists, se->se_width_idx);
-
-	if (verbose) /* Addr + origin */
-		ret += 3 + BITS_PER_LONG / 4;
-
-	return ret;
-}
-
 static void hists__remove_entry_filter(struct hists *hists, struct hist_entry *h,
 				       enum hist_filter filter)
 {

Namhyung Kim

author maintainer

2012-09-03 03:00 UTC #15 lore raw

PATCH 4/5 perf ui/browser: Use perf_hpp__format functions

In reply to Namhyung Kim

From: Namhyung Kim <[email protected]>

Override hpp->color functions for TUI. Because line coloring is done outside of the function, it just sets the percent value and pass it.

Signed-off-by Namhyung Kim <[email protected]>

---
 tools/perf/ui/browsers/hists.c | 94 ++++++++++++++++++++++++++++++++----------
 tools/perf/ui/tui/setup.c      |  4 ++
 2 files changed, 76 insertions(+), 22 deletions(-)

tools/perf/ui/browsers/hists.c +72 ~~-22~~

index 81bd8c2af730..144d7be2872e 100644
@@ -28,6 +28,8 @@ struct hist_browser {
 	bool		     has_symbols;
 };
 
+extern void hist_browser__init_hpp(void);
+
 static int hists__browser_title(struct hists *hists, char *bf, size_t size,
 				const char *ev_name);

@@ -563,14 +565,47 @@ static int hist_browser__show_callchain(struct hist_browser *browser,
 	return row - first_row;
 }
 
+#define HPP__COLOR_FN(_name, _field)					\
+static int hist_browser__hpp_color_ ## _name(struct perf_hpp *hpp,	\
+					     struct hist_entry *he)	\
+{									\
+	double percent = 100.0 * he->_field / hpp->total_period;	\
+	*(double *)hpp->ptr = percent;					\
+	return scnprintf(hpp->buf, hpp->size, "%5.2f%%", percent);	\
+}
+
+HPP__COLOR_FN(overhead, period)
+HPP__COLOR_FN(overhead_sys, period_sys)
+HPP__COLOR_FN(overhead_us, period_us)
+HPP__COLOR_FN(overhead_guest_sys, period_guest_sys)
+HPP__COLOR_FN(overhead_guest_us, period_guest_us)
+
+#undef HPP__COLOR_FN
+
+void hist_browser__init_hpp(void)
+{
+	perf_hpp__init(false, false);
+
+	perf_hpp__format[PERF_HPP__OVERHEAD].color =
+				hist_browser__hpp_color_overhead;
+	perf_hpp__format[PERF_HPP__OVERHEAD_SYS].color =
+				hist_browser__hpp_color_overhead_sys;
+	perf_hpp__format[PERF_HPP__OVERHEAD_US].color =
+				hist_browser__hpp_color_overhead_us;
+	perf_hpp__format[PERF_HPP__OVERHEAD_GUEST_SYS].color =
+				hist_browser__hpp_color_overhead_guest_sys;
+	perf_hpp__format[PERF_HPP__OVERHEAD_GUEST_US].color =
+				hist_browser__hpp_color_overhead_guest_us;
+}
+
 static int hist_browser__show_entry(struct hist_browser *browser,
 				    struct hist_entry *entry,
 				    unsigned short row)
 {
 	char s[256];
 	double percent;
-	int printed = 0;
-	int width = browser->b.width - 6; /* The percentage */
+	int i, printed = 0;
+	int width = browser->b.width;
 	char folded_sign = ' ';
 	bool current_entry = ui_browser__is_current_entry(&browser->b, row);
 	off_t row_offset = entry->row_offset;

@@ -586,35 +621,50 @@ static int hist_browser__show_entry(struct hist_browser *browser,
 	}
 
 	if (row_offset == 0) {
-		hist_entry__sort_snprintf(entry, s, sizeof(s), browser->hists);
-		percent = (entry->period * 100.0) / browser->hists->stats.total_period;
+		struct perf_hpp hpp = {
+			.buf		= s,
+			.size		= sizeof(s),
+			.total_period	= browser->hists->stats.total_period,
+		};
 
-		ui_browser__set_percent_color(&browser->b, percent, current_entry);
 		ui_browser__gotorc(&browser->b, row, 0);
-		if (symbol_conf.use_callchain) {
-			slsmg_printf("%c ", folded_sign);
-			width -= 2;
-		}
 
-		slsmg_printf(" %5.2f%%", percent);
+		for (i = 0; i < PERF_HPP__MAX_INDEX; i++) {
+			if (!perf_hpp__format[i].cond)
+				continue;
 
-		/* The scroll bar isn't being used */
-		if (!browser->b.navkeypressed)
-			width += 1;
+			if (i) {
+				slsmg_printf("  ");
+				width -= 2;
+			}
 
-		if (!current_entry || !browser->b.navkeypressed)
-			ui_browser__set_color(&browser->b, HE_COLORSET_NORMAL);
+			if (perf_hpp__format[i].color) {
+				hpp.ptr = &percent;
+				/* It will set percent for us. See HPP__COLOR_FN above. */
+				width -= perf_hpp__format[i].color(&hpp, entry);
 
-		if (symbol_conf.show_nr_samples) {
-			slsmg_printf(" %11u", entry->nr_events);
-			width -= 12;
-		}
+				ui_browser__set_percent_color(&browser->b, percent, current_entry);
+
+				if (i == 0 && symbol_conf.use_callchain) {
+					slsmg_printf("%c ", folded_sign);
+					width -= 2;
+				}
+
+				slsmg_printf("%s", s);
 
-		if (symbol_conf.show_total_period) {
-			slsmg_printf(" %12" PRIu64, entry->period);
-			width -= 13;
+				if (!current_entry || !browser->b.navkeypressed)
+					ui_browser__set_color(&browser->b, HE_COLORSET_NORMAL);
+			} else {
+				width -= perf_hpp__format[i].entry(&hpp, entry);
+				slsmg_printf("%s", s);
+			}
 		}
 
+		/* The scroll bar isn't being used */
+		if (!browser->b.navkeypressed)
+			width += 1;
+
+		hist_entry__sort_snprintf(entry, s, sizeof(s), browser->hists);
 		slsmg_write_nstring(s, width);
 		++row;
 		++printed;

tools/perf/ui/tui/setup.c +4 -0

index 4c936e09931c..4dc0887c04f1 100644
@@ -15,6 +15,8 @@ static volatile int ui__need_resize;
 
 extern struct perf_error_ops perf_tui_eops;
 
+extern void hist_browser__init_hpp(void);
+
 void ui__refresh_dimensions(bool force)
 {
 	if (force || ui__need_resize) {

@@ -124,6 +126,8 @@ int ui__init(void)
 	signal(SIGTERM, ui__signal);
 
 	perf_error__register(&perf_tui_eops);
+
+	hist_browser__init_hpp();
 out:
 	return err;
 }

Signature · 1 line

1.7.11.4

Arnaldo Carvalho de Melo

2012-09-08 00:54 UTC #16 lore raw

Re: [PATCH 4/5] perf ui/browser: Use perf_hpp__format functions

Em Mon, Sep 03, 2012 at 11:53:09AM +0900, Namhyung Kim escreveu:

Quoted 16 lines

From: Namhyung Kim <[email protected]>
Override hpp->color functions for TUI. Because line coloring is done outside of the function, it just sets the percent value and pass it.
Signed-off-by Namhyung Kim <[email protected]>
---
 tools/perf/ui/browsers/hists.c | 94 ++++++++++++++++++++++++++++++++----------
 tools/perf/ui/tui/setup.c      |  4 ++
 2 files changed, 76 insertions(+), 22 deletions(-)
tools/perf/ui/browsers/hists.c +0 -0
index 81bd8c2af730..144d7be2872e 100644

tools/perf/ui/browsers/hists.c struct hist_browser { · line 31

Quoted 4 lines

tools/perf/ui/browsers/hists.c +1 -0

@@ -28,6 +28,8 @@ struct hist_browser {
 	bool		     has_symbols;
 };
 
+extern void hist_browser__init_hpp(void);

I folded the patch below so that we have the lines starting at column 1 like before, i.e. we were using " %5.2f" for the overhead and now you changed it to "%5.2f".

- Arnaldo

tools/perf/ui/browsers/hists.c +2 -2

index 144d7be..5a5739b 100644
@@ -605,7 +605,7 @@ static int hist_browser__show_entry(struct hist_browser *browser,
 	char s[256];
 	double percent;
 	int i, printed = 0;
-	int width = browser->b.width;
+	int width = browser->b.width - 1;
 	char folded_sign = ' ';
 	bool current_entry = ui_browser__is_current_entry(&browser->b, row);
 	off_t row_offset = entry->row_offset;

@@ -627,7 +627,7 @@ static int hist_browser__show_entry(struct hist_browser *browser,
 			.total_period	= browser->hists->stats.total_period,
 		};
 
-		ui_browser__gotorc(&browser->b, row, 0);
+		ui_browser__gotorc(&browser->b, row, 1);
 
 		for (i = 0; i < PERF_HPP__MAX_INDEX; i++) {
 			if (!perf_hpp__format[i].cond)

Namhyung Kim

author maintainer

2012-09-08 14:05 UTC #17 lore raw

Re: [PATCH 4/5] perf ui/browser: Use perf_hpp__format functions

Quoted 20 lines · Arnaldo Carvalho de Melo

Em Mon, Sep 03, 2012 at 11:53:09AM +0900, Namhyung Kim escreveu:
Quoted 19 lines
From: Namhyung Kim <[email protected]>
Override hpp->color functions for TUI. Because line coloring is done outside of the function, it just sets the percent value and pass it.
Signed-off-by Namhyung Kim <[email protected]>
---
 tools/perf/ui/browsers/hists.c | 94 ++++++++++++++++++++++++++++++++----------
 tools/perf/ui/tui/setup.c      |  4 ++
 2 files changed, 76 insertions(+), 22 deletions(-)
tools/perf/ui/browsers/hists.c +0 -0
index 81bd8c2af730..144d7be2872e 100644
@@ -28,6 +28,8 @@ struct hist_browser {
 	bool		     has_symbols;
 };
 

Quoted 5 lines

tools/perf/ui/browsers/hists.c struct hist_browser { · line 31
Quoted 1 line
tools/perf/ui/browsers/hists.c +1 -0
+extern void hist_browser__init_hpp(void);
I folded the patch below so that we have the lines starting at column 1 like before, i.e. we were using " %5.2f" for the overhead and now you changed it to "%5.2f".

Looks good to me.

Thanks,
Namhyung

Quoted 23 lines

tools/perf/ui/browsers/hists.c +2 -2

index 144d7be..5a5739b 100644
@@ -605,7 +605,7 @@ static int hist_browser__show_entry(struct hist_browser *browser,
 	char s[256];
 	double percent;
 	int i, printed = 0;
-	int width = browser->b.width;
+	int width = browser->b.width - 1;
 	char folded_sign = ' ';
 	bool current_entry = ui_browser__is_current_entry(&browser->b, row);
 	off_t row_offset = entry->row_offset;

@@ -627,7 +627,7 @@ static int hist_browser__show_entry(struct hist_browser *browser,
 			.total_period	= browser->hists->stats.total_period,
 		};
 
-		ui_browser__gotorc(&browser->b, row, 0);
+		ui_browser__gotorc(&browser->b, row, 1);
 
 		for (i = 0; i < PERF_HPP__MAX_INDEX; i++) {
 			if (!perf_hpp__format[i].cond)

Namhyung Kim

author maintainer

2012-09-12 06:32 UTC #18 lore raw

Re: [PATCH 4/5] perf ui/browser: Use perf_hpp__format functions

Quoted 6 lines · Namhyung Kim

Quoted 3 lines · Arnaldo Carvalho de Melo
I folded the patch below so that we have the lines starting at column 1 like before, i.e. we were using " %5.2f" for the overhead and now you changed it to "%5.2f".
Looks good to me.

It wasn't good actually :-/ Simply because it cannot refresh the character at column 0 anymore. I will send a fix soon.

Thanks,
Namhyung

tip-bot for Namhyung Kim

2012-09-09 08:57 UTC #19 lore raw

[tip:perf/core] perf hists browser: Use perf_hpp__format functions

In reply to Namhyung Kim

Commit-ID:  f5951d56a2ab4181b96806a4746895aaa5a3d72e
Gitweb:     http://git.kernel.org/tip/f5951d56a2ab4181b96806a4746895aaa5a3d72e
Author:     Namhyung Kim <[email protected]>
AuthorDate: Mon, 3 Sep 2012 11:53:09 +0900
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Sat, 8 Sep 2012 13:20:13 -0300

perf hists browser: Use perf_hpp__format functions

Override hpp->color functions for TUI. Because line coloring is done outside of the function, it just sets the percent value and pass it.

Signed-off-by Namhyung Kim <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc Paul Mackerras <[email protected]>

Cc Peter Zijlstra <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

[ committer note: Keep previous layout by showing the overhead at column 1 ]

Signed-off-by Arnaldo Carvalho de Melo <[email protected]>

---
 tools/perf/ui/browsers/hists.c |   96 ++++++++++++++++++++++++++++++----------
 tools/perf/ui/tui/setup.c      |    4 ++
 2 files changed, 77 insertions(+), 23 deletions(-)

tools/perf/ui/browsers/hists.c +73 ~~-23~~

index 81bd8c2..5a5739b 100644
@@ -28,6 +28,8 @@ struct hist_browser {
 	bool		     has_symbols;
 };
 
+extern void hist_browser__init_hpp(void);
+
 static int hists__browser_title(struct hists *hists, char *bf, size_t size,
 				const char *ev_name);

@@ -563,14 +565,47 @@ static int hist_browser__show_callchain(struct hist_browser *browser,
 	return row - first_row;
 }
 
+#define HPP__COLOR_FN(_name, _field)					\
+static int hist_browser__hpp_color_ ## _name(struct perf_hpp *hpp,	\
+					     struct hist_entry *he)	\
+{									\
+	double percent = 100.0 * he->_field / hpp->total_period;	\
+	*(double *)hpp->ptr = percent;					\
+	return scnprintf(hpp->buf, hpp->size, "%5.2f%%", percent);	\
+}
+
+HPP__COLOR_FN(overhead, period)
+HPP__COLOR_FN(overhead_sys, period_sys)
+HPP__COLOR_FN(overhead_us, period_us)
+HPP__COLOR_FN(overhead_guest_sys, period_guest_sys)
+HPP__COLOR_FN(overhead_guest_us, period_guest_us)
+
+#undef HPP__COLOR_FN
+
+void hist_browser__init_hpp(void)
+{
+	perf_hpp__init(false, false);
+
+	perf_hpp__format[PERF_HPP__OVERHEAD].color =
+				hist_browser__hpp_color_overhead;
+	perf_hpp__format[PERF_HPP__OVERHEAD_SYS].color =
+				hist_browser__hpp_color_overhead_sys;
+	perf_hpp__format[PERF_HPP__OVERHEAD_US].color =
+				hist_browser__hpp_color_overhead_us;
+	perf_hpp__format[PERF_HPP__OVERHEAD_GUEST_SYS].color =
+				hist_browser__hpp_color_overhead_guest_sys;
+	perf_hpp__format[PERF_HPP__OVERHEAD_GUEST_US].color =
+				hist_browser__hpp_color_overhead_guest_us;
+}
+
 static int hist_browser__show_entry(struct hist_browser *browser,
 				    struct hist_entry *entry,
 				    unsigned short row)
 {
 	char s[256];
 	double percent;
-	int printed = 0;
-	int width = browser->b.width - 6; /* The percentage */
+	int i, printed = 0;
+	int width = browser->b.width - 1;
 	char folded_sign = ' ';
 	bool current_entry = ui_browser__is_current_entry(&browser->b, row);
 	off_t row_offset = entry->row_offset;

@@ -586,35 +621,50 @@ static int hist_browser__show_entry(struct hist_browser *browser,
 	}
 
 	if (row_offset == 0) {
-		hist_entry__sort_snprintf(entry, s, sizeof(s), browser->hists);
-		percent = (entry->period * 100.0) / browser->hists->stats.total_period;
+		struct perf_hpp hpp = {
+			.buf		= s,
+			.size		= sizeof(s),
+			.total_period	= browser->hists->stats.total_period,
+		};
 
-		ui_browser__set_percent_color(&browser->b, percent, current_entry);
-		ui_browser__gotorc(&browser->b, row, 0);
-		if (symbol_conf.use_callchain) {
-			slsmg_printf("%c ", folded_sign);
-			width -= 2;
-		}
+		ui_browser__gotorc(&browser->b, row, 1);
 
-		slsmg_printf(" %5.2f%%", percent);
+		for (i = 0; i < PERF_HPP__MAX_INDEX; i++) {
+			if (!perf_hpp__format[i].cond)
+				continue;
 
-		/* The scroll bar isn't being used */
-		if (!browser->b.navkeypressed)
-			width += 1;
+			if (i) {
+				slsmg_printf("  ");
+				width -= 2;
+			}
 
-		if (!current_entry || !browser->b.navkeypressed)
-			ui_browser__set_color(&browser->b, HE_COLORSET_NORMAL);
+			if (perf_hpp__format[i].color) {
+				hpp.ptr = &percent;
+				/* It will set percent for us. See HPP__COLOR_FN above. */
+				width -= perf_hpp__format[i].color(&hpp, entry);
 
-		if (symbol_conf.show_nr_samples) {
-			slsmg_printf(" %11u", entry->nr_events);
-			width -= 12;
-		}
+				ui_browser__set_percent_color(&browser->b, percent, current_entry);
+
+				if (i == 0 && symbol_conf.use_callchain) {
+					slsmg_printf("%c ", folded_sign);
+					width -= 2;
+				}
+
+				slsmg_printf("%s", s);
 
-		if (symbol_conf.show_total_period) {
-			slsmg_printf(" %12" PRIu64, entry->period);
-			width -= 13;
+				if (!current_entry || !browser->b.navkeypressed)
+					ui_browser__set_color(&browser->b, HE_COLORSET_NORMAL);
+			} else {
+				width -= perf_hpp__format[i].entry(&hpp, entry);
+				slsmg_printf("%s", s);
+			}
 		}
 
+		/* The scroll bar isn't being used */
+		if (!browser->b.navkeypressed)
+			width += 1;
+
+		hist_entry__sort_snprintf(entry, s, sizeof(s), browser->hists);
 		slsmg_write_nstring(s, width);
 		++row;
 		++printed;

tools/perf/ui/tui/setup.c +4 -0

index 4c936e0..4dc0887 100644
@@ -15,6 +15,8 @@ static volatile int ui__need_resize;
 
 extern struct perf_error_ops perf_tui_eops;
 
+extern void hist_browser__init_hpp(void);
+
 void ui__refresh_dimensions(bool force)
 {
 	if (force || ui__need_resize) {

@@ -124,6 +126,8 @@ int ui__init(void)
 	signal(SIGTERM, ui__signal);
 
 	perf_error__register(&perf_tui_eops);
+
+	hist_browser__init_hpp();
 out:
 	return err;
 }

Namhyung Kim

author maintainer

2012-09-03 03:01 UTC #20 lore raw

PATCH 5/5 perf gtk/browser: Use perf_hpp__format functions

In reply to Namhyung Kim

From: Namhyung Kim <[email protected]>

Now we can support color using pango markup with this change.

Acked-by Pekka Enberg <[email protected]>

Signed-off-by Namhyung Kim <[email protected]>

---
 tools/perf/ui/gtk/browser.c | 101 +++++++++++++++++++++++++++++++++++++-------
 tools/perf/ui/gtk/gtk.h     |   1 +
 tools/perf/ui/gtk/setup.c   |   1 +
 3 files changed, 87 insertions(+), 16 deletions(-)

tools/perf/ui/gtk/browser.c +85 ~~-16~~

index 26b5b652a8cd..3c16ab50e0f8 100644
@@ -36,6 +36,57 @@ static void perf_gtk__resize_window(GtkWidget *window)
 	gtk_window_resize(GTK_WINDOW(window), width, height);
 }
 
+static const char *perf_gtk__get_percent_color(double percent)
+{
+	if (percent >= MIN_RED)
+		return "<span fgcolor='red'>";
+	if (percent >= MIN_GREEN)
+		return "<span fgcolor='dark green'>";
+	return NULL;
+}
+
+#define HPP__COLOR_FN(_name, _field)						\
+static int perf_gtk__hpp_color_ ## _name(struct perf_hpp *hpp,			\
+					 struct hist_entry *he)			\
+{										\
+	double percent = 100.0 * he->_field / hpp->total_period;		\
+	const char *markup;							\
+	int ret = 0;								\
+										\
+	markup = perf_gtk__get_percent_color(percent);				\
+	if (markup)								\
+		ret += scnprintf(hpp->buf, hpp->size, "%s", markup);		\
+	ret += scnprintf(hpp->buf + ret, hpp->size - ret, "%5.2f%%", percent); 	\
+	if (markup)								\
+		ret += scnprintf(hpp->buf + ret, hpp->size - ret, "</span>"); 	\
+										\
+	return ret;								\
+}
+
+HPP__COLOR_FN(overhead, period)
+HPP__COLOR_FN(overhead_sys, period_sys)
+HPP__COLOR_FN(overhead_us, period_us)
+HPP__COLOR_FN(overhead_guest_sys, period_guest_sys)
+HPP__COLOR_FN(overhead_guest_us, period_guest_us)
+
+#undef HPP__COLOR_FN
+
+void perf_gtk__init_hpp(void)
+{
+	perf_hpp__init(false, false);
+
+	perf_hpp__format[PERF_HPP__OVERHEAD].color =
+				perf_gtk__hpp_color_overhead;
+	perf_hpp__format[PERF_HPP__OVERHEAD_SYS].color =
+				perf_gtk__hpp_color_overhead_sys;
+	perf_hpp__format[PERF_HPP__OVERHEAD_US].color =
+				perf_gtk__hpp_color_overhead_us;
+	perf_hpp__format[PERF_HPP__OVERHEAD_GUEST_SYS].color =
+				perf_gtk__hpp_color_overhead_guest_sys;
+	perf_hpp__format[PERF_HPP__OVERHEAD_GUEST_US].color =
+				perf_gtk__hpp_color_overhead_guest_us;
+}
+
 static void perf_gtk__show_hists(GtkWidget *window, struct hists *hists)
 {
 	GType col_types[MAX_COLUMNS];

@@ -43,15 +94,25 @@ static void perf_gtk__show_hists(GtkWidget *window, struct hists *hists)
 	struct sort_entry *se;
 	GtkListStore *store;
 	struct rb_node *nd;
-	u64 total_period;
 	GtkWidget *view;
-	int col_idx;
+	int i, col_idx;
 	int nr_cols;
+	char s[512];
+
+	struct perf_hpp hpp = {
+		.buf		= s,
+		.size		= sizeof(s),
+		.total_period	= hists->stats.total_period,
+	};
 
 	nr_cols = 0;
 
-	/* The percentage column */
-	col_types[nr_cols++] = G_TYPE_STRING;
+	for (i = 0; i < PERF_HPP__MAX_INDEX; i++) {
+		if (!perf_hpp__format[i].cond)
+			continue;
+
+		col_types[nr_cols++] = G_TYPE_STRING;
+	}
 
 	list_for_each_entry(se, &hist_entry__sort_list, list) {
 		if (se->elide)

@@ -68,11 +129,17 @@ static void perf_gtk__show_hists(GtkWidget *window, struct hists *hists)
 
 	col_idx = 0;
 
-	/* The percentage column */
-	gtk_tree_view_insert_column_with_attributes(GTK_TREE_VIEW(view),
-						    -1, "Overhead (%)",
-						    renderer, "text",
-						    col_idx++, NULL);
+	for (i = 0; i < PERF_HPP__MAX_INDEX; i++) {
+		if (!perf_hpp__format[i].cond)
+			continue;
+
+		perf_hpp__format[i].header(&hpp);
+
+		gtk_tree_view_insert_column_with_attributes(GTK_TREE_VIEW(view),
+							    -1, s,
+							    renderer, "markup",
+							    col_idx++, NULL);
+	}
 
 	list_for_each_entry(se, &hist_entry__sort_list, list) {
 		if (se->elide)

@@ -88,13 +155,9 @@ static void perf_gtk__show_hists(GtkWidget *window, struct hists *hists)
 
 	g_object_unref(GTK_TREE_MODEL(store));
 
-	total_period = hists->stats.total_period;
-
 	for (nd = rb_first(&hists->entries); nd; nd = rb_next(nd)) {
 		struct hist_entry *h = rb_entry(nd, struct hist_entry, rb_node);
 		GtkTreeIter iter;
-		double percent;
-		char s[512];
 
 		if (h->filtered)
 			continue;

@@ -103,11 +166,17 @@ static void perf_gtk__show_hists(GtkWidget *window, struct hists *hists)
 
 		col_idx = 0;
 
-		percent = (h->period * 100.0) / total_period;
+		for (i = 0; i < PERF_HPP__MAX_INDEX; i++) {
+			if (!perf_hpp__format[i].cond)
+				continue;
 
-		snprintf(s, ARRAY_SIZE(s), "%.2f", percent);
+			if (perf_hpp__format[i].color)
+				perf_hpp__format[i].color(&hpp, h);
+			else
+				perf_hpp__format[i].entry(&hpp, h);
 
-		gtk_list_store_set(store, &iter, col_idx++, s, -1);
+			gtk_list_store_set(store, &iter, col_idx++, s, -1);
+		}
 
 		list_for_each_entry(se, &hist_entry__sort_list, list) {
 			if (se->elide)

tools/perf/ui/gtk/gtk.h +1 -0

index 793cb6116ddf..687af0bba187 100644
@@ -30,6 +30,7 @@ struct perf_gtk_context *perf_gtk__activate_context(GtkWidget *window);
 int perf_gtk__deactivate_context(struct perf_gtk_context **ctx);
 
 void perf_gtk__init_helpline(void);
+void perf_gtk__init_hpp(void);
 
 #ifndef HAVE_GTK_INFO_BAR
 static inline GtkWidget *perf_gtk__setup_info_bar(void)

tools/perf/ui/gtk/setup.c +1 -0

index ec1ee26b485a..26429437e19e 100644
@@ -8,6 +8,7 @@ int perf_gtk__init(void)
 {
 	perf_error__register(&perf_gtk_eops);
 	perf_gtk__init_helpline();
+	perf_gtk__init_hpp();
 	return gtk_init_check(NULL, NULL) ? 0 : -1;
 }

Signature · 1 line

1.7.11.4

tip-bot for Namhyung Kim

2012-09-09 08:58 UTC #21 lore raw

[tip:perf/core] perf gtk/browser: Use perf_hpp__format functions

Commit-ID:  12ceaded6b276c13d905cf09f0c1a1322022ea58
Gitweb:     http://git.kernel.org/tip/12ceaded6b276c13d905cf09f0c1a1322022ea58
Author:     Namhyung Kim <[email protected]>
AuthorDate: Mon, 3 Sep 2012 11:53:10 +0900
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Sat, 8 Sep 2012 13:20:21 -0300

perf gtk/browser: Use perf_hpp__format functions

Now we can support color using pango markup with this change.

Signed-off-by Namhyung Kim <[email protected]>

Acked-by Pekka Enberg <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc Paul Mackerras <[email protected]>

Cc Peter Zijlstra <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Arnaldo Carvalho de Melo <[email protected]>

---
 tools/perf/ui/gtk/browser.c |  101 ++++++++++++++++++++++++++++++++++++-------
 tools/perf/ui/gtk/gtk.h     |    1 +
 tools/perf/ui/gtk/setup.c   |    1 +
 3 files changed, 87 insertions(+), 16 deletions(-)

tools/perf/ui/gtk/browser.c +85 ~~-16~~

index 26b5b65..3c16ab5 100644
@@ -36,6 +36,57 @@ static void perf_gtk__resize_window(GtkWidget *window)
 	gtk_window_resize(GTK_WINDOW(window), width, height);
 }
 
+static const char *perf_gtk__get_percent_color(double percent)
+{
+	if (percent >= MIN_RED)
+		return "<span fgcolor='red'>";
+	if (percent >= MIN_GREEN)
+		return "<span fgcolor='dark green'>";
+	return NULL;
+}
+
+#define HPP__COLOR_FN(_name, _field)						\
+static int perf_gtk__hpp_color_ ## _name(struct perf_hpp *hpp,			\
+					 struct hist_entry *he)			\
+{										\
+	double percent = 100.0 * he->_field / hpp->total_period;		\
+	const char *markup;							\
+	int ret = 0;								\
+										\
+	markup = perf_gtk__get_percent_color(percent);				\
+	if (markup)								\
+		ret += scnprintf(hpp->buf, hpp->size, "%s", markup);		\
+	ret += scnprintf(hpp->buf + ret, hpp->size - ret, "%5.2f%%", percent); 	\
+	if (markup)								\
+		ret += scnprintf(hpp->buf + ret, hpp->size - ret, "</span>"); 	\
+										\
+	return ret;								\
+}
+
+HPP__COLOR_FN(overhead, period)
+HPP__COLOR_FN(overhead_sys, period_sys)
+HPP__COLOR_FN(overhead_us, period_us)
+HPP__COLOR_FN(overhead_guest_sys, period_guest_sys)
+HPP__COLOR_FN(overhead_guest_us, period_guest_us)
+
+#undef HPP__COLOR_FN
+
+void perf_gtk__init_hpp(void)
+{
+	perf_hpp__init(false, false);
+
+	perf_hpp__format[PERF_HPP__OVERHEAD].color =
+				perf_gtk__hpp_color_overhead;
+	perf_hpp__format[PERF_HPP__OVERHEAD_SYS].color =
+				perf_gtk__hpp_color_overhead_sys;
+	perf_hpp__format[PERF_HPP__OVERHEAD_US].color =
+				perf_gtk__hpp_color_overhead_us;
+	perf_hpp__format[PERF_HPP__OVERHEAD_GUEST_SYS].color =
+				perf_gtk__hpp_color_overhead_guest_sys;
+	perf_hpp__format[PERF_HPP__OVERHEAD_GUEST_US].color =
+				perf_gtk__hpp_color_overhead_guest_us;
+}
+
 static void perf_gtk__show_hists(GtkWidget *window, struct hists *hists)
 {
 	GType col_types[MAX_COLUMNS];

@@ -43,15 +94,25 @@ static void perf_gtk__show_hists(GtkWidget *window, struct hists *hists)
 	struct sort_entry *se;
 	GtkListStore *store;
 	struct rb_node *nd;
-	u64 total_period;
 	GtkWidget *view;
-	int col_idx;
+	int i, col_idx;
 	int nr_cols;
+	char s[512];
+
+	struct perf_hpp hpp = {
+		.buf		= s,
+		.size		= sizeof(s),
+		.total_period	= hists->stats.total_period,
+	};
 
 	nr_cols = 0;
 
-	/* The percentage column */
-	col_types[nr_cols++] = G_TYPE_STRING;
+	for (i = 0; i < PERF_HPP__MAX_INDEX; i++) {
+		if (!perf_hpp__format[i].cond)
+			continue;
+
+		col_types[nr_cols++] = G_TYPE_STRING;
+	}
 
 	list_for_each_entry(se, &hist_entry__sort_list, list) {
 		if (se->elide)

@@ -68,11 +129,17 @@ static void perf_gtk__show_hists(GtkWidget *window, struct hists *hists)
 
 	col_idx = 0;
 
-	/* The percentage column */
-	gtk_tree_view_insert_column_with_attributes(GTK_TREE_VIEW(view),
-						    -1, "Overhead (%)",
-						    renderer, "text",
-						    col_idx++, NULL);
+	for (i = 0; i < PERF_HPP__MAX_INDEX; i++) {
+		if (!perf_hpp__format[i].cond)
+			continue;
+
+		perf_hpp__format[i].header(&hpp);
+
+		gtk_tree_view_insert_column_with_attributes(GTK_TREE_VIEW(view),
+							    -1, s,
+							    renderer, "markup",
+							    col_idx++, NULL);
+	}
 
 	list_for_each_entry(se, &hist_entry__sort_list, list) {
 		if (se->elide)

@@ -88,13 +155,9 @@ static void perf_gtk__show_hists(GtkWidget *window, struct hists *hists)
 
 	g_object_unref(GTK_TREE_MODEL(store));
 
-	total_period = hists->stats.total_period;
-
 	for (nd = rb_first(&hists->entries); nd; nd = rb_next(nd)) {
 		struct hist_entry *h = rb_entry(nd, struct hist_entry, rb_node);
 		GtkTreeIter iter;
-		double percent;
-		char s[512];
 
 		if (h->filtered)
 			continue;

@@ -103,11 +166,17 @@ static void perf_gtk__show_hists(GtkWidget *window, struct hists *hists)
 
 		col_idx = 0;
 
-		percent = (h->period * 100.0) / total_period;
+		for (i = 0; i < PERF_HPP__MAX_INDEX; i++) {
+			if (!perf_hpp__format[i].cond)
+				continue;
 
-		snprintf(s, ARRAY_SIZE(s), "%.2f", percent);
+			if (perf_hpp__format[i].color)
+				perf_hpp__format[i].color(&hpp, h);
+			else
+				perf_hpp__format[i].entry(&hpp, h);
 
-		gtk_list_store_set(store, &iter, col_idx++, s, -1);
+			gtk_list_store_set(store, &iter, col_idx++, s, -1);
+		}
 
 		list_for_each_entry(se, &hist_entry__sort_list, list) {
 			if (se->elide)

tools/perf/ui/gtk/gtk.h +1 -0

index 793cb61..687af0b 100644
@@ -30,6 +30,7 @@ struct perf_gtk_context *perf_gtk__activate_context(GtkWidget *window);
 int perf_gtk__deactivate_context(struct perf_gtk_context **ctx);
 
 void perf_gtk__init_helpline(void);
+void perf_gtk__init_hpp(void);
 
 #ifndef HAVE_GTK_INFO_BAR
 static inline GtkWidget *perf_gtk__setup_info_bar(void)

tools/perf/ui/gtk/setup.c +1 -0

index ec1ee26..2642943 100644
@@ -8,6 +8,7 @@ int perf_gtk__init(void)
 {
 	perf_error__register(&perf_gtk_eops);
 	perf_gtk__init_helpline();
+	perf_gtk__init_hpp();
 	return gtk_init_check(NULL, NULL) ? 0 : -1;
 }

Arnaldo Carvalho de Melo

2012-09-08 00:49 UTC #22 lore raw

In reply to Namhyung Kim

Em Mon, Sep 03, 2012 at 11:53:05AM +0900, Namhyung Kim escreveu:

Quoted 4 lines

This is a cleanup and refactoring patchset for the hist printing code by adding perf_hpp__format functions and perf_hpp. I believe it makes the code easy to maintain and to add new features like upcoming group viewing and callchain accumulation.

I applied this patch series to then get some patches from Jiri's 'perf diff' series, so that he can use what you did here, as you noticed the overlap when reviewing his series.

But then 'perf diff' segfaults :-\

I left your patch series + with that overhead column fixlet at my tree, branch tmp.perf/hpp.

The segfaults happens here:

Program received signal SIGSEGV, Segmentation fault.
0x0000000000498837 in hpp__entry_delta (hpp=0x7fffffffdeb0, he=0xcd6310) at ui/hist.c:200
200 old_percent = 100.0 * he->pair->period / old_total;
Missing separate debuginfos, use: debuginfo-install atk-1.28.0-2.el6.x86_64 bzip2-libs-1.0.5-7.el6_0.x86_64 elfutils-libelf-0.152-1.el6.x86_64 elfutils-libs-0.152-1.el6.x86_64 expat-2.0.1-11.el6_2.x86_64 fontconfig-2.8.0-3.el6.x86_64 freetype-2.3.11-6.el6_2.9.x86_64 glib2-2.22.5-7.el6.x86_64 gtk2-2.18.9-10.el6.x86_64 libX11-1.3-2.el6.x86_64 libXau-1.0.5-1.el6.x86_64 libXcomposite-0.4.1-2.el6.x86_64 libXcursor-1.1.10-2.el6.x86_64 libXdamage-1.1.2-1.el6.x86_64 libXext-1.1-3.el6.x86_64 libXfixes-4.0.4-1.el6.x86_64 libXi-1.3-3.el6.x86_64 libXinerama-1.1-1.el6.x86_64 libXrandr-1.3.0-4.el6.x86_64 libXrender-0.9.5-1.el6.x86_64 libpng-1.2.49-1.el6_2.x86_64 libselinux-2.0.94-5.3.el6.x86_64 libxcb-1.5-1.el6.x86_64 nss-softokn-freebl-3.12.9-11.el6.x86_64 pango-1.28.1-3.el6_0.5.x86_64 perl-libs-5.10.1-127.el6.x86_64 pixman-0.18.4-1.el6_0.1.x86_64 xz-libs-4.999.9-0.3.beta.20091007git.el6.x86_64 zlib-1.2.3-27.el6.x86_64 (gdb) p he->pair
$1 = (struct hist_entry *) 0x0

Please try with:

perf record -a usleep 1
perf record -a usleep 1
perf diff

it will use perf.data.old and perf.data and will segfault in that branch.

- Arnaldo

Namhyung Kim

author maintainer

2012-09-08 14:00 UTC #23 lore raw

Hi, Arnaldo

Quoted 25 lines · Arnaldo Carvalho de Melo

Em Mon, Sep 03, 2012 at 11:53:05AM +0900, Namhyung Kim escreveu:
Quoted 4 lines
This is a cleanup and refactoring patchset for the hist printing code by adding perf_hpp__format functions and perf_hpp. I believe it makes the code easy to maintain and to add new features like upcoming group viewing and callchain accumulation.
I applied this patch series to then get some patches from Jiri's 'perf diff' series, so that he can use what you did here, as you noticed the overlap when reviewing his series.
But then 'perf diff' segfaults :-\
I left your patch series + with that overhead column fixlet at my tree, branch tmp.perf/hpp.
The segfaults happens here:
Program received signal SIGSEGV, Segmentation fault.
0x0000000000498837 in hpp__entry_delta (hpp=0x7fffffffdeb0, he=0xcd6310) at ui/hist.c:200
200 old_percent = 100.0 * he->pair->period / old_total;
Missing separate debuginfos, use: debuginfo-install atk-1.28.0-2.el6.x86_64 bzip2-libs-1.0.5-7.el6_0.x86_64 elfutils-libelf-0.152-1.el6.x86_64 elfutils-libs-0.152-1.el6.x86_64 expat-2.0.1-11.el6_2.x86_64 fontconfig-2.8.0-3.el6.x86_64 freetype-2.3.11-6.el6_2.9.x86_64 glib2-2.22.5-7.el6.x86_64 gtk2-2.18.9-10.el6.x86_64 libX11-1.3-2.el6.x86_64 libXau-1.0.5-1.el6.x86_64 libXcomposite-0.4.1-2.el6.x86_64 libXcursor-1.1.10-2.el6.x86_64 libXdamage-1.1.2-1.el6.x86_64 libXext-1.1-3.el6.x86_64 libXfixes-4.0.4-1.el6.x86_64 libXi-1.3-3.el6.x86_64 libXinerama-1.1-1.el6.x86_64 libXrandr-1.3.0-4.el6.x86_64 libXrender-0.9.5-1.el6.x86_64 libpng-1.2.49-1.el6_2.x86_64 libselinux-2.0.94-5.3.el6.x86_64 libxcb-1.5-1.el6.x86_64 nss-softokn-freebl-3.12.9-11.el6.x86_64 pango-1.28.1-3.el6_0.5.x86_64 perl-libs-5.10.1
-127.el6.x86_64 pixman-0.18.4-1.el6_0.1.x86_64 xz-libs-4.999.9-0.3.beta.20091007git.el6.x86_64 zlib-1.2.3-27.el6.x86_64
(gdb) p he->pair
$1 = (struct hist_entry *) 0x0

Quoted 7 lines

Please try with:
perf record -a usleep 1
perf record -a usleep 1
perf diff
it will use perf.data.old and perf.data and will segfault in that branch.

I see the problem. I overlooked he->pair can be NULL when perf diff is running. So the fix will be adding a NULL check before the line. In case of NULL, it will default to 0, so no problem.

Thanks,
Namhyung

tools/perf/ui/hist.c +1 -1

index 16dc486d02be..3c2701fa6be1 100644
@@ -196,7 +196,7 @@ static int hpp__entry_delta(struct perf_hpp *hpp, struct hist_entry *he)
 	char buf[32] = " ";
 
 	old_total = pair_hists->stats.total_period;
-	if (old_total > 0)
+	if (old_total > 0 && he->pair)
 		old_percent = 100.0 * he->pair->period / old_total;
 
 	new_total = hpp->total_period;

Arnaldo Carvalho de Melo

2012-09-08 15:08 UTC #24 lore raw

Em Sat, Sep 08, 2012 at 11:00:36PM +0900, Namhyung Kim escreveu:

Quoted 7 lines · Arnaldo Carvalho de Melo

Quoted 5 lines
Please try with:
perf record -a usleep 1
perf record -a usleep 1
perf diff

Quoted 5 lines

Quoted 1 line
it will use perf.data.old and perf.data and will segfault in that branch.
I see the problem. I overlooked he->pair can be NULL when perf diff is running. So the fix will be adding a NULL check before the line. In case of NULL, it will default to 0, so no problem.

Ok, now it doesn't segfaults, but the diff results seems bogus:

Withour this patch + your fix:

[root@sandy ~]# perf diff
# Event 'cycles'
#
# Baseline Delta Shared Object Symbol

# ........ ..........  .................  ......................................
#
     0.00%    +39.61%  [kernel.kallsyms]  [k] __alloc_pages_nodemask            
    19.82%     +6.10%  [kernel.kallsyms]  [k] intel_idle                        
     0.00%     +8.61%  [kernel.kallsyms]  [k] generic_exec_single               
     0.00%     +6.39%  [kernel.kallsyms]  [k] list_del                          
     0.00%     +6.15%  [kernel.kallsyms]  [k] nr_iowait_cpu                     
     0.00%     +5.98%  [kernel.kallsyms]  [k] selinux_inode_permission          
     0.00%     +5.23%  libc-2.12.so       [.] _dl_addr                          
     0.00%     +0.76%  [kernel.kallsyms]  [k] group_sched_in                    
     0.10%     +0.32%  [kernel.kallsyms]  [k] native_write_msr_safe             
     0.00%     +0.40%  [kernel.kallsyms]  [k] smp_call_function_single_interrupt
     0.14%     +0.19%  [kernel.kallsyms]  [k] csd_unlock                        
     0.00%     +0.20%  [kernel.kallsyms]  [k] perf_pmu_enable

With your patch:

[root@sandy ~]# perf diff
# Event 'cycles'
#
# Baseline Delta Shared Object Symbol

# ........  .......  .................  ......................................
#
    39.61%  +39.61%  [kernel.kallsyms]  [k] __alloc_pages_nodemask            
    25.92%   +6.10%  [kernel.kallsyms]  [k] intel_idle                        
     8.61%   +8.61%  [kernel.kallsyms]  [k] generic_exec_single               
     6.39%   +6.39%  [kernel.kallsyms]  [k] list_del                          
     6.15%   +6.15%  [kernel.kallsyms]  [k] nr_iowait_cpu                     
     5.98%   +5.98%  [kernel.kallsyms]  [k] selinux_inode_permission          
     5.23%   +5.23%  libc-2.12.so       [.] _dl_addr                          
     0.76%   +0.76%  [kernel.kallsyms]  [k] group_sched_in                    
     0.42%   +0.32%  [kernel.kallsyms]  [k] native_write_msr_safe             
     0.40%   +0.40%  [kernel.kallsyms]  [k] smp_call_function_single_interrupt
     0.33%   +0.19%  [kernel.kallsyms]  [k] csd_unlock                        
     0.20%   +0.20%  [kernel.kallsyms]  [k] perf_pmu_enable                   
[root@sandy ~]#

The deltas are the same, but the baseline completely changed and are bogus.

So please try again and make sure that the results are kept.

- Arnaldo