mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCHSET 0/7] perf tools: Honor column width setting (v2)
@ 2014-07-25  1:18 Namhyung Kim
  2014-07-25  1:18 ` [PATCH 1/7] perf tools: Left-align output contents Namhyung Kim
                   ` (6 more replies)
  0 siblings, 7 replies; 13+ messages in thread
From: Namhyung Kim @ 2014-07-25  1:18 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Ingo Molnar, Paul Mackerras, Namhyung Kim,
	Namhyung Kim, LKML, Jiri Olsa

Hello,

This patchset is to control perf report/top output column width by
-w/--column-widths option so that it can fit into the terminal size.
The -w option is there for perf report but it ignored by recent output
field changed due to some reason.  This patchset fixes it and supports
perf top also.

This is sometimes useful if your terminal is small and there's some
C++ applications which have amazingly long symbol names.  Without this
patchset user might not see those symbols on TUI, since it maps
left/right arrow keys to other functions.

The -w option sets column width starting from the first column
(overhead or optional overhead_children column unless -F option is
given).  It doesn't make sense to limit those overhead columns so it's
not a hard-limit for them.  But it *is* a hard-limit for other columns
such as comm, dso, symbol, and so on.  One can use 0 not to
limit/force a width for those columns.


For example:

  $ perf report --stdio -s comm,dso
  ...
  # Overhead  Command          Shared Object                                        
  # ........  ...............  .....................................................
  #
      71.08%  gnome-shell      perf-1497.map                                        
       9.23%  gnome-shell      swrast_dri.so                                        
       3.99%  Xorg             [unknown]                                            
       3.18%  Xorg             [kernel.kallsyms]                                    
       2.93%  gnome-shell      [kernel.kallsyms]                                    
       2.41%  swapper          [kernel.kallsyms]                                    
       1.55%  synergys         libpthread-2.15.so                                   
       1.08%  synergys         synergys                                             
       0.68%  systemd-journal  [kernel.kallsyms]                                    
       0.59%  synergys         libstdc++.so.6.0.17                                  
       0.58%  gnome-shell      libc-2.15.so                                         
       0.54%  kworker/0:2      [kernel.kallsyms]                                    
       0.20%  systemd-journal  [unknown]                                            
       0.19%  gnome-shell      libclutter-1.0.so.0.1000.8.#prelink#.1kh1we (deleted)
       0.18%  firefox          libxul.so (deleted)                                  
       0.17%  gnome-shell      libcogl.so.9.1.1.#prelink#.ZL3fn1 (deleted)          
       0.14%  firefox          [kernel.kallsyms]                                    
       0.14%  gnome-shell      libgobject-2.0.so.0.3200.4                           
       0.13%  gnome-shell      libpthread-2.15.so                                   
       0.11%  synergys         [kernel.kallsyms]                                    
       0.10%  perf             [kernel.kallsyms]                                    


  $ perf report --stdio -s comm,dso -w 0,10,20   # 0 means no limit
  ...
  # Overhead  Command     Shared Object       
  # ........  ..........  ....................
  #
      71.08%  gnome-shel  perf-1497.map       
       9.23%  gnome-shel  swrast_dri.so       
       3.99%  Xorg        [unknown]           
       3.18%  Xorg        [kernel.kallsyms]   
       2.93%  gnome-shel  [kernel.kallsyms]   
       2.41%  swapper     [kernel.kallsyms]   
       1.55%  synergys    libpthread-2.15.so  
       1.08%  synergys    synergys            
       0.68%  systemd-jo  [kernel.kallsyms]   
       0.59%  synergys    libstdc++.so.6.0.17 
       0.58%  gnome-shel  libc-2.15.so        
       0.54%  kworker/0:  [kernel.kallsyms]   
       0.20%  systemd-jo  [unknown]           
       0.19%  gnome-shel  libclutter-1.0.so.0.
       0.18%  firefox     libxul.so (deleted) 
       0.17%  gnome-shel  libcogl.so.9.1.1.#pr
       0.14%  firefox     [kernel.kallsyms]   
       0.14%  gnome-shel  libgobject-2.0.so.0.
       0.13%  gnome-shel  libpthread-2.15.so  
       0.11%  synergys    [kernel.kallsyms]   
       0.10%  perf        [kernel.kallsyms]   


 * changes in v2:
  - fix TUI alignment when no header is shown  (Jiri)
  - change printing order of pid sort key  (Jiri)


You can also get this from 'perf/width-v2' branch on my tree

  git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git


Thanks,
Namhyung


Namhyung Kim (7):
  perf tools: Left-align output contents
  perf tools: Make __hpp__fmt() receive an additional len argument
  perf tools: Save column length in perf_hpp_fmt
  perf report: Honor column width setting
  perf top: Add -w option for setting column width
  perf tools: Add name field into perf_hpp_fmt
  perf tools: Fix column alignment when headers aren't shown on TUI

 tools/perf/Documentation/perf-report.txt |   2 +-
 tools/perf/Documentation/perf-top.txt    |   6 +
 tools/perf/builtin-top.c                 |   3 +
 tools/perf/ui/browsers/hists.c           |  36 ++--
 tools/perf/ui/gtk/hists.c                |  18 +-
 tools/perf/ui/hist.c                     | 275 +++++++++++++++++++------------
 tools/perf/ui/stdio/hist.c               |   4 +-
 tools/perf/util/color.c                  |  16 ++
 tools/perf/util/color.h                  |   1 +
 tools/perf/util/hist.h                   |  17 +-
 tools/perf/util/sort.c                   |  55 ++++---
 11 files changed, 279 insertions(+), 154 deletions(-)

-- 
2.0.0


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/7] perf tools: Left-align output contents
  2014-07-25  1:18 [PATCHSET 0/7] perf tools: Honor column width setting (v2) Namhyung Kim
@ 2014-07-25  1:18 ` Namhyung Kim
  2014-07-25  1:18 ` [PATCH 2/7] perf tools: Make __hpp__fmt() receive an additional len argument Namhyung Kim
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Namhyung Kim @ 2014-07-25  1:18 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Ingo Molnar, Paul Mackerras, Namhyung Kim,
	Namhyung Kim, LKML, Jiri Olsa

Now perf left-aligns column headers but the contents does not.  It
should have same alignment.  This requires a change in pid sort key -
it consists of two part (pid and comm).  As length of comm can be vary
it'd be better to change the order of them.

Thanks to Jiri Olsa for pointing this out.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/util/sort.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 14e5a039bc45..eda9ee836cee 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -70,12 +70,12 @@ static int hist_entry__thread_snprintf(struct hist_entry *he, char *bf,
 				       size_t size, unsigned int width)
 {
 	const char *comm = thread__comm_str(he->thread);
-	return repsep_snprintf(bf, size, "%*s:%5d", width - 6,
-			       comm ?: "", he->thread->tid);
+	return repsep_snprintf(bf, size, "%5d:%-*s", he->thread->tid,
+			       width - 6, comm ?: "");
 }
 
 struct sort_entry sort_thread = {
-	.se_header	= "Command:  Pid",
+	.se_header	= "  Pid:Command",
 	.se_cmp		= sort__thread_cmp,
 	.se_snprintf	= hist_entry__thread_snprintf,
 	.se_width_idx	= HISTC_THREAD,
@@ -106,7 +106,7 @@ sort__comm_sort(struct hist_entry *left, struct hist_entry *right)
 static int hist_entry__comm_snprintf(struct hist_entry *he, char *bf,
 				     size_t size, unsigned int width)
 {
-	return repsep_snprintf(bf, size, "%*s", width, comm__str(he->comm));
+	return repsep_snprintf(bf, size, "%-*s", width, comm__str(he->comm));
 }
 
 struct sort_entry sort_comm = {
@@ -305,7 +305,7 @@ static int hist_entry__srcline_snprintf(struct hist_entry *he, char *bf,
 					size_t size,
 					unsigned int width __maybe_unused)
 {
-	return repsep_snprintf(bf, size, "%s", he->srcline);
+	return repsep_snprintf(bf, size, "%-s", he->srcline);
 }
 
 struct sort_entry sort_srcline = {
-- 
2.0.0


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 2/7] perf tools: Make __hpp__fmt() receive an additional len argument
  2014-07-25  1:18 [PATCHSET 0/7] perf tools: Honor column width setting (v2) Namhyung Kim
  2014-07-25  1:18 ` [PATCH 1/7] perf tools: Left-align output contents Namhyung Kim
@ 2014-07-25  1:18 ` Namhyung Kim
  2014-07-25  1:18 ` [PATCH 3/7] perf tools: Save column length in perf_hpp_fmt Namhyung Kim
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Namhyung Kim @ 2014-07-25  1:18 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Ingo Molnar, Paul Mackerras, Namhyung Kim,
	Namhyung Kim, LKML, Jiri Olsa

So that it can properly handle alignment requirements later.  To do
that, add percent_color_len_snprintf() fucntion to help coloring of
overhead columns.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/ui/browsers/hists.c | 14 ++++++++------
 tools/perf/ui/gtk/hists.c      |  8 +++++---
 tools/perf/ui/hist.c           | 43 +++++++++++++++++++++---------------------
 tools/perf/util/color.c        | 16 ++++++++++++++++
 tools/perf/util/color.h        |  1 +
 tools/perf/util/hist.h         |  4 ++--
 6 files changed, 54 insertions(+), 32 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index a94b11fc5e00..02507ba944e3 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -653,17 +653,18 @@ struct hpp_arg {
 static int __hpp__slsmg_color_printf(struct perf_hpp *hpp, const char *fmt, ...)
 {
 	struct hpp_arg *arg = hpp->ptr;
-	int ret;
+	int ret, len;
 	va_list args;
 	double percent;
 
 	va_start(args, fmt);
+	len = va_arg(args, int);
 	percent = va_arg(args, double);
 	va_end(args);
 
 	ui_browser__set_percent_color(arg->b, percent, arg->current_entry);
 
-	ret = scnprintf(hpp->buf, hpp->size, fmt, percent);
+	ret = scnprintf(hpp->buf, hpp->size, fmt, len, percent);
 	slsmg_printf("%s", hpp->buf);
 
 	advance_hpp(hpp, ret);
@@ -681,7 +682,7 @@ hist_browser__hpp_color_##_type(struct perf_hpp_fmt *fmt __maybe_unused,\
 				struct perf_hpp *hpp,			\
 				struct hist_entry *he)			\
 {									\
-	return __hpp__fmt(hpp, he, __hpp_get_##_field, " %6.2f%%",	\
+	return __hpp__fmt(hpp, he, __hpp_get_##_field, " %*.2f%%", 6,	\
 			  __hpp__slsmg_color_printf, true);		\
 }
 
@@ -697,13 +698,14 @@ hist_browser__hpp_color_##_type(struct perf_hpp_fmt *fmt __maybe_unused,\
 				struct hist_entry *he)			\
 {									\
 	if (!symbol_conf.cumulate_callchain) {				\
-		int ret = scnprintf(hpp->buf, hpp->size, "%8s", "N/A");	\
+		int ret = scnprintf(hpp->buf, hpp->size,		\
+				    "%*s", 8, "N/A");			\
 		slsmg_printf("%s", hpp->buf);				\
 									\
 		return ret;						\
 	}								\
-	return __hpp__fmt(hpp, he, __hpp_get_acc_##_field, " %6.2f%%",	\
-			  __hpp__slsmg_color_printf, true);		\
+	return __hpp__fmt(hpp, he, __hpp_get_acc_##_field, " %*.2f%%",	\
+			  6, __hpp__slsmg_color_printf, true);	\
 }
 
 __HPP_COLOR_PERCENT_FN(overhead, period)
diff --git a/tools/perf/ui/gtk/hists.c b/tools/perf/ui/gtk/hists.c
index 6ca60e482cdc..91f6cd7d2312 100644
--- a/tools/perf/ui/gtk/hists.c
+++ b/tools/perf/ui/gtk/hists.c
@@ -11,6 +11,7 @@
 static int __percent_color_snprintf(struct perf_hpp *hpp, const char *fmt, ...)
 {
 	int ret = 0;
+	int len;
 	va_list args;
 	double percent;
 	const char *markup;
@@ -18,6 +19,7 @@ static int __percent_color_snprintf(struct perf_hpp *hpp, const char *fmt, ...)
 	size_t size = hpp->size;
 
 	va_start(args, fmt);
+	len = va_arg(args, int);
 	percent = va_arg(args, double);
 	va_end(args);
 
@@ -25,7 +27,7 @@ static int __percent_color_snprintf(struct perf_hpp *hpp, const char *fmt, ...)
 	if (markup)
 		ret += scnprintf(buf, size, markup);
 
-	ret += scnprintf(buf + ret, size - ret, fmt, percent);
+	ret += scnprintf(buf + ret, size - ret, fmt, len, percent);
 
 	if (markup)
 		ret += scnprintf(buf + ret, size - ret, "</span>");
@@ -43,7 +45,7 @@ static int perf_gtk__hpp_color_##_type(struct perf_hpp_fmt *fmt __maybe_unused,
 				       struct perf_hpp *hpp,			\
 				       struct hist_entry *he)			\
 {										\
-	return __hpp__fmt(hpp, he, he_get_##_field, " %6.2f%%",			\
+	return __hpp__fmt(hpp, he, he_get_##_field, " %*.2f%%", 6,		\
 			  __percent_color_snprintf, true);			\
 }
 
@@ -57,7 +59,7 @@ static int perf_gtk__hpp_color_##_type(struct perf_hpp_fmt *fmt __maybe_unused,
 				       struct perf_hpp *hpp,			\
 				       struct hist_entry *he)			\
 {										\
-	return __hpp__fmt_acc(hpp, he, he_get_acc_##_field, " %6.2f%%",		\
+	return __hpp__fmt_acc(hpp, he, he_get_acc_##_field, " %*.2f%%", 6, 	\
 			      __percent_color_snprintf, true);			\
 }
 
diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index 498adb23c02e..c6cffbd0b0e1 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -16,7 +16,7 @@
 })
 
 int __hpp__fmt(struct perf_hpp *hpp, struct hist_entry *he,
-	       hpp_field_fn get_field, const char *fmt,
+	       hpp_field_fn get_field, const char *fmt, int len,
 	       hpp_snprint_fn print_fn, bool fmt_percent)
 {
 	int ret;
@@ -32,9 +32,9 @@ int __hpp__fmt(struct perf_hpp *hpp, struct hist_entry *he,
 		if (total)
 			percent = 100.0 * get_field(he) / total;
 
-		ret = hpp__call_print_fn(hpp, print_fn, fmt, percent);
+		ret = hpp__call_print_fn(hpp, print_fn, fmt, len, percent);
 	} else
-		ret = hpp__call_print_fn(hpp, print_fn, fmt, get_field(he));
+		ret = hpp__call_print_fn(hpp, print_fn, fmt, len, get_field(he));
 
 	if (perf_evsel__is_group_event(evsel)) {
 		int prev_idx, idx_delta;
@@ -60,19 +60,19 @@ int __hpp__fmt(struct perf_hpp *hpp, struct hist_entry *he,
 				 */
 				if (fmt_percent) {
 					ret += hpp__call_print_fn(hpp, print_fn,
-								  fmt, 0.0);
+								  fmt, len, 0.0);
 				} else {
 					ret += hpp__call_print_fn(hpp, print_fn,
-								  fmt, 0ULL);
+								  fmt, len, 0ULL);
 				}
 			}
 
 			if (fmt_percent) {
-				ret += hpp__call_print_fn(hpp, print_fn, fmt,
+				ret += hpp__call_print_fn(hpp, print_fn, fmt, len,
 							  100.0 * period / total);
 			} else {
 				ret += hpp__call_print_fn(hpp, print_fn, fmt,
-							  period);
+							  len, period);
 			}
 
 			prev_idx = perf_evsel__group_idx(evsel);
@@ -86,10 +86,10 @@ int __hpp__fmt(struct perf_hpp *hpp, struct hist_entry *he,
 			 */
 			if (fmt_percent) {
 				ret += hpp__call_print_fn(hpp, print_fn,
-							  fmt, 0.0);
+							  fmt, len, 0.0);
 			} else {
 				ret += hpp__call_print_fn(hpp, print_fn,
-							  fmt, 0ULL);
+							  fmt, len, 0ULL);
 			}
 		}
 	}
@@ -105,7 +105,7 @@ int __hpp__fmt(struct perf_hpp *hpp, struct hist_entry *he,
 }
 
 int __hpp__fmt_acc(struct perf_hpp *hpp, struct hist_entry *he,
-		   hpp_field_fn get_field, const char *fmt,
+		   hpp_field_fn get_field, const char *fmt, int len,
 		   hpp_snprint_fn print_fn, bool fmt_percent)
 {
 	if (!symbol_conf.cumulate_callchain) {
@@ -113,7 +113,7 @@ int __hpp__fmt_acc(struct perf_hpp *hpp, struct hist_entry *he,
 				fmt_percent ? 8 : 12, "N/A");
 	}
 
-	return __hpp__fmt(hpp, he, get_field, fmt, print_fn, fmt_percent);
+	return __hpp__fmt(hpp, he, get_field, fmt, len, print_fn, fmt_percent);
 }
 
 static int field_cmp(u64 field_a, u64 field_b)
@@ -221,11 +221,12 @@ static int hpp_color_scnprintf(struct perf_hpp *hpp, const char *fmt, ...)
 	va_list args;
 	ssize_t ssize = hpp->size;
 	double percent;
-	int ret;
+	int ret, len;
 
 	va_start(args, fmt);
+	len = va_arg(args, int);
 	percent = va_arg(args, double);
-	ret = value_color_snprintf(hpp->buf, hpp->size, fmt, percent);
+	ret = percent_color_len_snprintf(hpp->buf, hpp->size, fmt, len, percent);
 	va_end(args);
 
 	return (ret >= ssize) ? (ssize - 1) : ret;
@@ -253,7 +254,7 @@ static u64 he_get_##_field(struct hist_entry *he)				\
 static int hpp__color_##_type(struct perf_hpp_fmt *fmt __maybe_unused,		\
 			      struct perf_hpp *hpp, struct hist_entry *he) 	\
 {										\
-	return __hpp__fmt(hpp, he, he_get_##_field, " %6.2f%%",			\
+	return __hpp__fmt(hpp, he, he_get_##_field, " %*.2f%%",	6,		\
 			  hpp_color_scnprintf, true);				\
 }
 
@@ -261,8 +262,8 @@ static int hpp__color_##_type(struct perf_hpp_fmt *fmt __maybe_unused,		\
 static int hpp__entry_##_type(struct perf_hpp_fmt *_fmt __maybe_unused,		\
 			      struct perf_hpp *hpp, struct hist_entry *he) 	\
 {										\
-	const char *fmt = symbol_conf.field_sep ? " %.2f" : " %6.2f%%";		\
-	return __hpp__fmt(hpp, he, he_get_##_field, fmt,			\
+	int len = symbol_conf.field_sep ? 1 : 6;				\
+	return __hpp__fmt(hpp, he, he_get_##_field, " %*.2f%%",	len,		\
 			  hpp_entry_scnprintf, true);				\
 }
 
@@ -281,7 +282,7 @@ static u64 he_get_acc_##_field(struct hist_entry *he)				\
 static int hpp__color_##_type(struct perf_hpp_fmt *fmt __maybe_unused,		\
 			      struct perf_hpp *hpp, struct hist_entry *he) 	\
 {										\
-	return __hpp__fmt_acc(hpp, he, he_get_acc_##_field, " %6.2f%%",		\
+	return __hpp__fmt_acc(hpp, he, he_get_acc_##_field, " %*.2f%%",	6, 	\
 			      hpp_color_scnprintf, true);			\
 }
 
@@ -289,8 +290,8 @@ static int hpp__color_##_type(struct perf_hpp_fmt *fmt __maybe_unused,		\
 static int hpp__entry_##_type(struct perf_hpp_fmt *_fmt __maybe_unused,		\
 			      struct perf_hpp *hpp, struct hist_entry *he) 	\
 {										\
-	const char *fmt = symbol_conf.field_sep ? " %.2f" : " %6.2f%%";		\
-	return __hpp__fmt_acc(hpp, he, he_get_acc_##_field, fmt,		\
+	int len = symbol_conf.field_sep ? 1 : 6;				\
+	return __hpp__fmt_acc(hpp, he, he_get_##_field, " %*.2f%%", len,	\
 			      hpp_entry_scnprintf, true);			\
 }
 
@@ -309,8 +310,8 @@ static u64 he_get_raw_##_field(struct hist_entry *he)				\
 static int hpp__entry_##_type(struct perf_hpp_fmt *_fmt __maybe_unused,		\
 			      struct perf_hpp *hpp, struct hist_entry *he) 	\
 {										\
-	const char *fmt = symbol_conf.field_sep ? " %"PRIu64 : " %11"PRIu64;	\
-	return __hpp__fmt(hpp, he, he_get_raw_##_field, fmt,			\
+	int len = symbol_conf.field_sep ? 1 : 11;				\
+	return __hpp__fmt(hpp, he, he_get_raw_##_field, " %*"PRIu64, len, 	\
 			  hpp_entry_scnprintf, false);				\
 }
 
diff --git a/tools/perf/util/color.c b/tools/perf/util/color.c
index 87b8672eb413..f4654183d391 100644
--- a/tools/perf/util/color.c
+++ b/tools/perf/util/color.c
@@ -335,3 +335,19 @@ int percent_color_snprintf(char *bf, size_t size, const char *fmt, ...)
 	va_end(args);
 	return value_color_snprintf(bf, size, fmt, percent);
 }
+
+int percent_color_len_snprintf(char *bf, size_t size, const char *fmt, ...)
+{
+	va_list args;
+	int len;
+	double percent;
+	const char *color;
+
+	va_start(args, fmt);
+	len = va_arg(args, int);
+	percent = va_arg(args, double);
+	va_end(args);
+
+	color = get_percent_color(percent);
+	return color_snprintf(bf, size, color, fmt, len, percent);
+}
diff --git a/tools/perf/util/color.h b/tools/perf/util/color.h
index 7ff30a62a132..0a594b8a0c26 100644
--- a/tools/perf/util/color.h
+++ b/tools/perf/util/color.h
@@ -41,6 +41,7 @@ int color_fprintf_ln(FILE *fp, const char *color, const char *fmt, ...);
 int color_fwrite_lines(FILE *fp, const char *color, size_t count, const char *buf);
 int value_color_snprintf(char *bf, size_t size, const char *fmt, double value);
 int percent_color_snprintf(char *bf, size_t size, const char *fmt, ...);
+int percent_color_len_snprintf(char *bf, size_t size, const char *fmt, ...);
 int percent_color_fprintf(FILE *fp, const char *fmt, double percent);
 const char *get_percent_color(double percent);
 
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 742f49a85725..13d074db9ef1 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -267,10 +267,10 @@ typedef int (*hpp_callback_fn)(struct perf_hpp *hpp, bool front);
 typedef int (*hpp_snprint_fn)(struct perf_hpp *hpp, const char *fmt, ...);
 
 int __hpp__fmt(struct perf_hpp *hpp, struct hist_entry *he,
-	       hpp_field_fn get_field, const char *fmt,
+	       hpp_field_fn get_field, const char *fmt, int len,
 	       hpp_snprint_fn print_fn, bool fmt_percent);
 int __hpp__fmt_acc(struct perf_hpp *hpp, struct hist_entry *he,
-		   hpp_field_fn get_field, const char *fmt,
+		   hpp_field_fn get_field, const char *fmt, int len,
 		   hpp_snprint_fn print_fn, bool fmt_percent);
 
 static inline void advance_hpp(struct perf_hpp *hpp, int inc)
-- 
2.0.0


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 3/7] perf tools: Save column length in perf_hpp_fmt
  2014-07-25  1:18 [PATCHSET 0/7] perf tools: Honor column width setting (v2) Namhyung Kim
  2014-07-25  1:18 ` [PATCH 1/7] perf tools: Left-align output contents Namhyung Kim
  2014-07-25  1:18 ` [PATCH 2/7] perf tools: Make __hpp__fmt() receive an additional len argument Namhyung Kim
@ 2014-07-25  1:18 ` Namhyung Kim
  2014-07-25  1:18 ` [PATCH 4/7] perf report: Honor column width setting Namhyung Kim
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Namhyung Kim @ 2014-07-25  1:18 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Ingo Molnar, Paul Mackerras, Namhyung Kim,
	Namhyung Kim, LKML, Jiri Olsa

Save column length in the hpp format and pass it to print functions.
This is a preparation for users to control column width in the output.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/ui/browsers/hists.c |   2 +-
 tools/perf/ui/hist.c           | 135 +++++++++++++++++++++++++++--------------
 tools/perf/util/hist.h         |   2 +
 tools/perf/util/sort.c         |   3 +-
 4 files changed, 94 insertions(+), 48 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 02507ba944e3..c1d8d3925fe1 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -849,7 +849,7 @@ static int hists__scnprintf_headers(char *buf, size_t size, struct hists *hists)
 		if (perf_hpp__should_skip(fmt))
 			continue;
 
-		/* We need to add the length of the columns header. */
+		/* We need to ensure length of the columns header. */
 		perf_hpp__reset_width(fmt, hists);
 
 		ret = fmt->header(fmt, &dummy_hpp, hists_to_evsel(hists));
diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index c6cffbd0b0e1..e28ca972d046 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -110,7 +110,7 @@ int __hpp__fmt_acc(struct perf_hpp *hpp, struct hist_entry *he,
 {
 	if (!symbol_conf.cumulate_callchain) {
 		return snprintf(hpp->buf, hpp->size, "%*s",
-				fmt_percent ? 8 : 12, "N/A");
+				fmt_percent ? len + 2 : len + 1, "N/A");
 	}
 
 	return __hpp__fmt(hpp, he, get_field, fmt, len, print_fn, fmt_percent);
@@ -190,32 +190,31 @@ static int __hpp__sort_acc(struct hist_entry *a, struct hist_entry *b,
 	return ret;
 }
 
-#define __HPP_HEADER_FN(_type, _str, _min_width, _unit_width) 		\
-static int hpp__header_##_type(struct perf_hpp_fmt *fmt __maybe_unused,	\
-			       struct perf_hpp *hpp,			\
-			       struct perf_evsel *evsel)		\
-{									\
-	int len = _min_width;						\
-									\
-	if (symbol_conf.event_group)					\
-		len = max(len, evsel->nr_members * _unit_width);	\
-									\
-	return scnprintf(hpp->buf, hpp->size, "%*s", len, _str);	\
-}
-
-#define __HPP_WIDTH_FN(_type, _min_width, _unit_width) 			\
-static int hpp__width_##_type(struct perf_hpp_fmt *fmt __maybe_unused,	\
+#define __HPP_WIDTH_FN(_type, _str)					\
+static int hpp__width_##_type(struct perf_hpp_fmt *fmt,			\
 			      struct perf_hpp *hpp __maybe_unused,	\
 			      struct perf_evsel *evsel)			\
 {									\
-	int len = _min_width;						\
+	int len = fmt->len;						\
 									\
 	if (symbol_conf.event_group)					\
-		len = max(len, evsel->nr_members * _unit_width);	\
+		len = max(len, evsel->nr_members * len);		\
+									\
+	if (len < (int)strlen(_str))					\
+		len = strlen(_str);					\
 									\
 	return len;							\
 }
 
+#define __HPP_HEADER_FN(_type, _str) 					\
+static int hpp__header_##_type(struct perf_hpp_fmt *fmt,		\
+			       struct perf_hpp *hpp,			\
+			       struct perf_evsel *evsel)		\
+{									\
+	int len = hpp__width_##_type(fmt, hpp, evsel);			\
+	return scnprintf(hpp->buf, hpp->size, "%*s", len, _str);	\
+}
+
 static int hpp_color_scnprintf(struct perf_hpp *hpp, const char *fmt, ...)
 {
 	va_list args;
@@ -251,18 +250,19 @@ static u64 he_get_##_field(struct hist_entry *he)				\
 	return he->stat._field;							\
 }										\
 										\
-static int hpp__color_##_type(struct perf_hpp_fmt *fmt __maybe_unused,		\
+static int hpp__color_##_type(struct perf_hpp_fmt *fmt,				\
 			      struct perf_hpp *hpp, struct hist_entry *he) 	\
 {										\
-	return __hpp__fmt(hpp, he, he_get_##_field, " %*.2f%%",	6,		\
+	int len = fmt->len - 2;	/* 2 for a space and a % sign */		\
+	return __hpp__fmt(hpp, he, he_get_##_field, " %*.2f%%",	len,		\
 			  hpp_color_scnprintf, true);				\
 }
 
 #define __HPP_ENTRY_PERCENT_FN(_type, _field)					\
-static int hpp__entry_##_type(struct perf_hpp_fmt *_fmt __maybe_unused,		\
+static int hpp__entry_##_type(struct perf_hpp_fmt *fmt,				\
 			      struct perf_hpp *hpp, struct hist_entry *he) 	\
 {										\
-	int len = symbol_conf.field_sep ? 1 : 6;				\
+	int len = symbol_conf.field_sep ? 1 : fmt->len - 2;			\
 	return __hpp__fmt(hpp, he, he_get_##_field, " %*.2f%%",	len,		\
 			  hpp_entry_scnprintf, true);				\
 }
@@ -279,18 +279,19 @@ static u64 he_get_acc_##_field(struct hist_entry *he)				\
 	return he->stat_acc->_field;						\
 }										\
 										\
-static int hpp__color_##_type(struct perf_hpp_fmt *fmt __maybe_unused,		\
+static int hpp__color_##_type(struct perf_hpp_fmt *fmt,				\
 			      struct perf_hpp *hpp, struct hist_entry *he) 	\
 {										\
-	return __hpp__fmt_acc(hpp, he, he_get_acc_##_field, " %*.2f%%",	6, 	\
+	int len = fmt->len - 2;	/* 2 for a space and a % sign */		\
+	return __hpp__fmt_acc(hpp, he, he_get_acc_##_field, " %*.2f%%",	len, 	\
 			      hpp_color_scnprintf, true);			\
 }
 
 #define __HPP_ENTRY_ACC_PERCENT_FN(_type, _field)				\
-static int hpp__entry_##_type(struct perf_hpp_fmt *_fmt __maybe_unused,		\
+static int hpp__entry_##_type(struct perf_hpp_fmt *fmt,				\
 			      struct perf_hpp *hpp, struct hist_entry *he) 	\
 {										\
-	int len = symbol_conf.field_sep ? 1 : 6;				\
+	int len = symbol_conf.field_sep ? 1 : fmt->len - 2;			\
 	return __hpp__fmt_acc(hpp, he, he_get_##_field, " %*.2f%%", len,	\
 			      hpp_entry_scnprintf, true);			\
 }
@@ -307,10 +308,10 @@ static u64 he_get_raw_##_field(struct hist_entry *he)				\
 	return he->stat._field;							\
 }										\
 										\
-static int hpp__entry_##_type(struct perf_hpp_fmt *_fmt __maybe_unused,		\
+static int hpp__entry_##_type(struct perf_hpp_fmt *fmt,				\
 			      struct perf_hpp *hpp, struct hist_entry *he) 	\
 {										\
-	int len = symbol_conf.field_sep ? 1 : 11;				\
+	int len = symbol_conf.field_sep ? 1 : fmt->len - 1;			\
 	return __hpp__fmt(hpp, he, he_get_raw_##_field, " %*"PRIu64, len, 	\
 			  hpp_entry_scnprintf, false);				\
 }
@@ -322,37 +323,38 @@ static int64_t hpp__sort_##_type(struct hist_entry *a, struct hist_entry *b)	\
 }
 
 
-#define HPP_PERCENT_FNS(_type, _str, _field, _min_width, _unit_width)	\
-__HPP_HEADER_FN(_type, _str, _min_width, _unit_width)			\
-__HPP_WIDTH_FN(_type, _min_width, _unit_width)				\
+#define HPP_PERCENT_FNS(_type, _str, _field)				\
+__HPP_WIDTH_FN(_type, _str)						\
+__HPP_HEADER_FN(_type, _str)						\
 __HPP_COLOR_PERCENT_FN(_type, _field)					\
 __HPP_ENTRY_PERCENT_FN(_type, _field)					\
 __HPP_SORT_FN(_type, _field)
 
-#define HPP_PERCENT_ACC_FNS(_type, _str, _field, _min_width, _unit_width)\
-__HPP_HEADER_FN(_type, _str, _min_width, _unit_width)			\
-__HPP_WIDTH_FN(_type, _min_width, _unit_width)				\
+#define HPP_PERCENT_ACC_FNS(_type, _str, _field)			\
+__HPP_WIDTH_FN(_type, _str)						\
+__HPP_HEADER_FN(_type, _str)						\
 __HPP_COLOR_ACC_PERCENT_FN(_type, _field)				\
 __HPP_ENTRY_ACC_PERCENT_FN(_type, _field)				\
 __HPP_SORT_ACC_FN(_type, _field)
 
-#define HPP_RAW_FNS(_type, _str, _field, _min_width, _unit_width)	\
-__HPP_HEADER_FN(_type, _str, _min_width, _unit_width)			\
-__HPP_WIDTH_FN(_type, _min_width, _unit_width)				\
+#define HPP_RAW_FNS(_type, _str, _field)				\
+__HPP_WIDTH_FN(_type, _str)						\
+__HPP_HEADER_FN(_type, _str)						\
 __HPP_ENTRY_RAW_FN(_type, _field)					\
 __HPP_SORT_RAW_FN(_type, _field)
 
-__HPP_HEADER_FN(overhead_self, "Self", 8, 8)
+__HPP_WIDTH_FN(overhead_self, "Self")
+__HPP_HEADER_FN(overhead_self, "Self")
 
-HPP_PERCENT_FNS(overhead, "Overhead", period, 8, 8)
-HPP_PERCENT_FNS(overhead_sys, "sys", period_sys, 8, 8)
-HPP_PERCENT_FNS(overhead_us, "usr", period_us, 8, 8)
-HPP_PERCENT_FNS(overhead_guest_sys, "guest sys", period_guest_sys, 9, 8)
-HPP_PERCENT_FNS(overhead_guest_us, "guest usr", period_guest_us, 9, 8)
-HPP_PERCENT_ACC_FNS(overhead_acc, "Children", period, 8, 8)
+HPP_PERCENT_FNS(overhead, "Overhead", period)
+HPP_PERCENT_FNS(overhead_sys, "sys", period_sys)
+HPP_PERCENT_FNS(overhead_us, "usr", period_us)
+HPP_PERCENT_FNS(overhead_guest_sys, "guest sys", period_guest_sys)
+HPP_PERCENT_FNS(overhead_guest_us, "guest usr", period_guest_us)
+HPP_PERCENT_ACC_FNS(overhead_acc, "Children", period)
 
-HPP_RAW_FNS(samples, "Samples", nr_events, 12, 12)
-HPP_RAW_FNS(period, "Period", period, 12, 12)
+HPP_RAW_FNS(samples, "Samples", nr_events)
+HPP_RAW_FNS(period, "Period", period)
 
 static int64_t hpp__nop_cmp(struct hist_entry *a __maybe_unused,
 			    struct hist_entry *b __maybe_unused)
@@ -453,6 +455,8 @@ void perf_hpp__init(void)
 
 		perf_hpp__format[PERF_HPP__OVERHEAD].header =
 						hpp__header_overhead_self;
+		perf_hpp__format[PERF_HPP__OVERHEAD].width =
+						hpp__width_overhead_self;
 	}
 
 	perf_hpp__column_enable(PERF_HPP__OVERHEAD);
@@ -519,6 +523,7 @@ void perf_hpp__cancel_cumulate(void)
 
 	perf_hpp__column_disable(PERF_HPP__OVERHEAD_ACC);
 	perf_hpp__format[PERF_HPP__OVERHEAD].header = hpp__header_overhead;
+	perf_hpp__format[PERF_HPP__OVERHEAD].width = hpp__width_overhead;
 }
 
 void perf_hpp__setup_output_field(void)
@@ -623,3 +628,41 @@ unsigned int hists__sort_list_width(struct hists *hists)
 
 	return ret;
 }
+
+void perf_hpp__reset_width(struct perf_hpp_fmt *fmt, struct hists *hists)
+{
+	int idx;
+
+	if (perf_hpp__is_sort_entry(fmt))
+		return perf_hpp__reset_sort_width(fmt, hists);
+
+	for (idx = 0; idx < PERF_HPP__MAX_INDEX; idx++) {
+		if (fmt == &perf_hpp__format[idx])
+			break;
+	}
+
+	if (idx == PERF_HPP__MAX_INDEX)
+		return;
+
+	switch (idx) {
+	case PERF_HPP__OVERHEAD:
+	case PERF_HPP__OVERHEAD_SYS:
+	case PERF_HPP__OVERHEAD_US:
+	case PERF_HPP__OVERHEAD_ACC:
+		fmt->len = 8;
+		break;
+
+	case PERF_HPP__OVERHEAD_GUEST_SYS:
+	case PERF_HPP__OVERHEAD_GUEST_US:
+		fmt->len = 9;
+		break;
+
+	case PERF_HPP__SAMPLES:
+	case PERF_HPP__PERIOD:
+		fmt->len = 12;
+		break;
+
+	default:
+		break;
+	}
+}
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 13d074db9ef1..a7ae890f4c71 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -207,6 +207,7 @@ struct perf_hpp_fmt {
 	struct list_head list;
 	struct list_head sort_list;
 	bool elide;
+	int len;
 };
 
 extern struct list_head perf_hpp__list;
@@ -261,6 +262,7 @@ static inline bool perf_hpp__should_skip(struct perf_hpp_fmt *format)
 }
 
 void perf_hpp__reset_width(struct perf_hpp_fmt *fmt, struct hists *hists);
+void perf_hpp__reset_sort_width(struct perf_hpp_fmt *fmt, struct hists *hists);
 
 typedef u64 (*hpp_field_fn)(struct hist_entry *he);
 typedef int (*hpp_callback_fn)(struct perf_hpp *hpp, bool front);
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index eda9ee836cee..a7f8a7b0eced 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -1194,7 +1194,7 @@ bool perf_hpp__same_sort_entry(struct perf_hpp_fmt *a, struct perf_hpp_fmt *b)
 	return hse_a->se == hse_b->se;
 }
 
-void perf_hpp__reset_width(struct perf_hpp_fmt *fmt, struct hists *hists)
+void perf_hpp__reset_sort_width(struct perf_hpp_fmt *fmt, struct hists *hists)
 {
 	struct hpp_sort_entry *hse;
 
@@ -1265,6 +1265,7 @@ __sort_dimension__alloc_hpp(struct sort_dimension *sd)
 	INIT_LIST_HEAD(&hse->hpp.list);
 	INIT_LIST_HEAD(&hse->hpp.sort_list);
 	hse->hpp.elide = false;
+	hse->hpp.len = 0;
 
 	return hse;
 }
-- 
2.0.0


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 4/7] perf report: Honor column width setting
  2014-07-25  1:18 [PATCHSET 0/7] perf tools: Honor column width setting (v2) Namhyung Kim
                   ` (2 preceding siblings ...)
  2014-07-25  1:18 ` [PATCH 3/7] perf tools: Save column length in perf_hpp_fmt Namhyung Kim
@ 2014-07-25  1:18 ` Namhyung Kim
  2014-07-25 11:13   ` Jiri Olsa
  2014-07-25  1:18 ` [PATCH 5/7] perf top: Add -w option for setting column width Namhyung Kim
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 13+ messages in thread
From: Namhyung Kim @ 2014-07-25  1:18 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Ingo Molnar, Paul Mackerras, Namhyung Kim,
	Namhyung Kim, LKML, Jiri Olsa

Set column width and do not change it if user gives -w/--column-widths
option.  It'll truncate longer symbols than the width if exists.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/ui/browsers/hists.c | 23 ++++++++----
 tools/perf/ui/gtk/hists.c      | 10 ++---
 tools/perf/ui/hist.c           | 84 +++++++++++++++++++++++++++++-------------
 tools/perf/ui/stdio/hist.c     |  4 +-
 tools/perf/util/hist.h         | 14 ++++---
 tools/perf/util/sort.c         | 46 ++++++++++++++---------
 6 files changed, 117 insertions(+), 64 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index c1d8d3925fe1..f0ef031558dd 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -678,12 +678,12 @@ static u64 __hpp_get_##_field(struct hist_entry *he)			\
 }									\
 									\
 static int								\
-hist_browser__hpp_color_##_type(struct perf_hpp_fmt *fmt __maybe_unused,\
+hist_browser__hpp_color_##_type(struct perf_hpp_fmt *fmt,		\
 				struct perf_hpp *hpp,			\
 				struct hist_entry *he)			\
 {									\
-	return __hpp__fmt(hpp, he, __hpp_get_##_field, " %*.2f%%", 6,	\
-			  __hpp__slsmg_color_printf, true);		\
+	return hpp__fmt(fmt, hpp, he, __hpp_get_##_field, " %*.2f%%",	\
+			__hpp__slsmg_color_printf, true);		\
 }
 
 #define __HPP_COLOR_ACC_PERCENT_FN(_type, _field)			\
@@ -693,19 +693,20 @@ static u64 __hpp_get_acc_##_field(struct hist_entry *he)		\
 }									\
 									\
 static int								\
-hist_browser__hpp_color_##_type(struct perf_hpp_fmt *fmt __maybe_unused,\
+hist_browser__hpp_color_##_type(struct perf_hpp_fmt *fmt,		\
 				struct perf_hpp *hpp,			\
 				struct hist_entry *he)			\
 {									\
 	if (!symbol_conf.cumulate_callchain) {				\
+		int len = fmt->user_len ?: fmt->len;			\
 		int ret = scnprintf(hpp->buf, hpp->size,		\
-				    "%*s", 8, "N/A");			\
+				    "%*s", len, "N/A");			\
 		slsmg_printf("%s", hpp->buf);				\
 									\
 		return ret;						\
 	}								\
-	return __hpp__fmt(hpp, he, __hpp_get_acc_##_field, " %*.2f%%",	\
-			  6, __hpp__slsmg_color_printf, true);	\
+	return hpp__fmt(fmt, hpp, he, __hpp_get_acc_##_field,		\
+			" %*.2f%%", __hpp__slsmg_color_printf, true);	\
 }
 
 __HPP_COLOR_PERCENT_FN(overhead, period)
@@ -797,8 +798,11 @@ static int hist_browser__show_entry(struct hist_browser *browser,
 			if (fmt->color) {
 				width -= fmt->color(fmt, &hpp, entry);
 			} else {
-				width -= fmt->entry(fmt, &hpp, entry);
+				int ret = fmt->entry(fmt, &hpp, entry);
+				s[ret] = '\0';
 				slsmg_printf("%s", s);
+
+				width -= ret;
 			}
 		}
 
@@ -1549,6 +1553,9 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 
 	memset(options, 0, sizeof(options));
 
+	if (symbol_conf.col_width_list_str)
+		perf_hpp__set_user_width(symbol_conf.col_width_list_str);
+
 	while (1) {
 		const struct thread *thread = NULL;
 		const struct dso *dso = NULL;
diff --git a/tools/perf/ui/gtk/hists.c b/tools/perf/ui/gtk/hists.c
index 91f6cd7d2312..897b2e140428 100644
--- a/tools/perf/ui/gtk/hists.c
+++ b/tools/perf/ui/gtk/hists.c
@@ -41,12 +41,12 @@ static u64 he_get_##_field(struct hist_entry *he)				\
 	return he->stat._field;							\
 }										\
 										\
-static int perf_gtk__hpp_color_##_type(struct perf_hpp_fmt *fmt __maybe_unused,	\
+static int perf_gtk__hpp_color_##_type(struct perf_hpp_fmt *fmt,		\
 				       struct perf_hpp *hpp,			\
 				       struct hist_entry *he)			\
 {										\
-	return __hpp__fmt(hpp, he, he_get_##_field, " %*.2f%%", 6,		\
-			  __percent_color_snprintf, true);			\
+	return hpp__fmt(fmt, hpp, he, he_get_##_field, " %*.2f%%",		\
+			__percent_color_snprintf, true);			\
 }
 
 #define __HPP_COLOR_ACC_PERCENT_FN(_type, _field)				\
@@ -59,8 +59,8 @@ static int perf_gtk__hpp_color_##_type(struct perf_hpp_fmt *fmt __maybe_unused,
 				       struct perf_hpp *hpp,			\
 				       struct hist_entry *he)			\
 {										\
-	return __hpp__fmt_acc(hpp, he, he_get_acc_##_field, " %*.2f%%", 6, 	\
-			      __percent_color_snprintf, true);			\
+	return hpp__fmt_acc(fmt, hpp, he, he_get_acc_##_field, " %*.2f%%", 	\
+			    __percent_color_snprintf, true);			\
 }
 
 __HPP_COLOR_PERCENT_FN(overhead, period)
diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index e28ca972d046..b2d60a95f01d 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -15,9 +15,9 @@
 	__ret;							\
 })
 
-int __hpp__fmt(struct perf_hpp *hpp, struct hist_entry *he,
-	       hpp_field_fn get_field, const char *fmt, int len,
-	       hpp_snprint_fn print_fn, bool fmt_percent)
+static int __hpp__fmt(struct perf_hpp *hpp, struct hist_entry *he,
+		      hpp_field_fn get_field, const char *fmt, int len,
+		      hpp_snprint_fn print_fn, bool fmt_percent)
 {
 	int ret;
 	struct hists *hists = he->hists;
@@ -104,16 +104,35 @@ int __hpp__fmt(struct perf_hpp *hpp, struct hist_entry *he,
 	return ret;
 }
 
-int __hpp__fmt_acc(struct perf_hpp *hpp, struct hist_entry *he,
-		   hpp_field_fn get_field, const char *fmt, int len,
-		   hpp_snprint_fn print_fn, bool fmt_percent)
+int hpp__fmt(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
+	     struct hist_entry *he, hpp_field_fn get_field,
+	     const char *fmtstr, hpp_snprint_fn print_fn, bool fmt_percent)
+{
+	int len = fmt->user_len ?: fmt->len;
+
+	if (symbol_conf.field_sep) {
+		return __hpp__fmt(hpp, he, get_field, fmtstr, 1,
+				  print_fn, fmt_percent);
+	}
+
+	if (fmt_percent)
+		len -= 2; /* 2 for a space and a % sign */
+	else
+		len -= 1;
+
+	return  __hpp__fmt(hpp, he, get_field, fmtstr, len, print_fn, fmt_percent);
+}
+
+int hpp__fmt_acc(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
+		 struct hist_entry *he, hpp_field_fn get_field,
+		 const char *fmtstr, hpp_snprint_fn print_fn, bool fmt_percent)
 {
 	if (!symbol_conf.cumulate_callchain) {
-		return snprintf(hpp->buf, hpp->size, "%*s",
-				fmt_percent ? len + 2 : len + 1, "N/A");
+		int len = fmt->user_len ?: fmt->len;
+		return snprintf(hpp->buf, hpp->size, " %*s", len - 1, "N/A");
 	}
 
-	return __hpp__fmt(hpp, he, get_field, fmt, len, print_fn, fmt_percent);
+	return hpp__fmt(fmt, hpp, he, get_field, fmtstr, print_fn, fmt_percent);
 }
 
 static int field_cmp(u64 field_a, u64 field_b)
@@ -195,10 +214,10 @@ static int hpp__width_##_type(struct perf_hpp_fmt *fmt,			\
 			      struct perf_hpp *hpp __maybe_unused,	\
 			      struct perf_evsel *evsel)			\
 {									\
-	int len = fmt->len;						\
+	int len = fmt->user_len ?: fmt->len;				\
 									\
 	if (symbol_conf.event_group)					\
-		len = max(len, evsel->nr_members * len);		\
+		len = max(len, evsel->nr_members * fmt->len);		\
 									\
 	if (len < (int)strlen(_str))					\
 		len = strlen(_str);					\
@@ -253,18 +272,16 @@ static u64 he_get_##_field(struct hist_entry *he)				\
 static int hpp__color_##_type(struct perf_hpp_fmt *fmt,				\
 			      struct perf_hpp *hpp, struct hist_entry *he) 	\
 {										\
-	int len = fmt->len - 2;	/* 2 for a space and a % sign */		\
-	return __hpp__fmt(hpp, he, he_get_##_field, " %*.2f%%",	len,		\
-			  hpp_color_scnprintf, true);				\
+	return hpp__fmt(fmt, hpp, he, he_get_##_field, " %*.2f%%",		\
+			hpp_color_scnprintf, true);				\
 }
 
 #define __HPP_ENTRY_PERCENT_FN(_type, _field)					\
 static int hpp__entry_##_type(struct perf_hpp_fmt *fmt,				\
 			      struct perf_hpp *hpp, struct hist_entry *he) 	\
 {										\
-	int len = symbol_conf.field_sep ? 1 : fmt->len - 2;			\
-	return __hpp__fmt(hpp, he, he_get_##_field, " %*.2f%%",	len,		\
-			  hpp_entry_scnprintf, true);				\
+	return hpp__fmt(fmt, hpp, he, he_get_##_field, " %*.2f%%",		\
+			hpp_entry_scnprintf, true);				\
 }
 
 #define __HPP_SORT_FN(_type, _field)						\
@@ -282,18 +299,16 @@ static u64 he_get_acc_##_field(struct hist_entry *he)				\
 static int hpp__color_##_type(struct perf_hpp_fmt *fmt,				\
 			      struct perf_hpp *hpp, struct hist_entry *he) 	\
 {										\
-	int len = fmt->len - 2;	/* 2 for a space and a % sign */		\
-	return __hpp__fmt_acc(hpp, he, he_get_acc_##_field, " %*.2f%%",	len, 	\
-			      hpp_color_scnprintf, true);			\
+	return hpp__fmt_acc(fmt, hpp, he, he_get_acc_##_field, " %*.2f%%", 	\
+			    hpp_color_scnprintf, true);				\
 }
 
 #define __HPP_ENTRY_ACC_PERCENT_FN(_type, _field)				\
 static int hpp__entry_##_type(struct perf_hpp_fmt *fmt,				\
 			      struct perf_hpp *hpp, struct hist_entry *he) 	\
 {										\
-	int len = symbol_conf.field_sep ? 1 : fmt->len - 2;			\
-	return __hpp__fmt_acc(hpp, he, he_get_##_field, " %*.2f%%", len,	\
-			      hpp_entry_scnprintf, true);			\
+	return hpp__fmt_acc(fmt, hpp, he, he_get_##_field, " %*.2f%%",		\
+			    hpp_entry_scnprintf, true);				\
 }
 
 #define __HPP_SORT_ACC_FN(_type, _field)					\
@@ -311,9 +326,8 @@ static u64 he_get_raw_##_field(struct hist_entry *he)				\
 static int hpp__entry_##_type(struct perf_hpp_fmt *fmt,				\
 			      struct perf_hpp *hpp, struct hist_entry *he) 	\
 {										\
-	int len = symbol_conf.field_sep ? 1 : fmt->len - 1;			\
-	return __hpp__fmt(hpp, he, he_get_raw_##_field, " %*"PRIu64, len, 	\
-			  hpp_entry_scnprintf, false);				\
+	return hpp__fmt(fmt, hpp, he, he_get_raw_##_field, " %*"PRIu64, 	\
+			hpp_entry_scnprintf, false);				\
 }
 
 #define __HPP_SORT_RAW_FN(_type, _field)					\
@@ -666,3 +680,21 @@ void perf_hpp__reset_width(struct perf_hpp_fmt *fmt, struct hists *hists)
 		break;
 	}
 }
+
+void perf_hpp__set_user_width(const char *width_list_str)
+{
+	struct perf_hpp_fmt *fmt;
+	const char *ptr = width_list_str;
+
+	perf_hpp__for_each_format(fmt) {
+		char *p;
+
+		int len = strtol(ptr, &p, 10);
+		fmt->user_len = len;
+
+		if (*p == ',')
+			ptr = p + 1;
+		else
+			break;
+	}
+}
diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c
index 40af0acb4fe9..15b451acbde6 100644
--- a/tools/perf/ui/stdio/hist.c
+++ b/tools/perf/ui/stdio/hist.c
@@ -395,10 +395,12 @@ size_t hists__fprintf(struct hists *hists, bool show_header, int max_rows,
 
 	init_rem_hits();
 
-
 	perf_hpp__for_each_format(fmt)
 		perf_hpp__reset_width(fmt, hists);
 
+	if (symbol_conf.col_width_list_str)
+		perf_hpp__set_user_width(symbol_conf.col_width_list_str);
+
 	if (!show_header)
 		goto print_entries;
 
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index a7ae890f4c71..2e70003fcd00 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -208,6 +208,7 @@ struct perf_hpp_fmt {
 	struct list_head sort_list;
 	bool elide;
 	int len;
+	int user_len;
 };
 
 extern struct list_head perf_hpp__list;
@@ -263,17 +264,18 @@ static inline bool perf_hpp__should_skip(struct perf_hpp_fmt *format)
 
 void perf_hpp__reset_width(struct perf_hpp_fmt *fmt, struct hists *hists);
 void perf_hpp__reset_sort_width(struct perf_hpp_fmt *fmt, struct hists *hists);
+void perf_hpp__set_user_width(const char *width_list_str);
 
 typedef u64 (*hpp_field_fn)(struct hist_entry *he);
 typedef int (*hpp_callback_fn)(struct perf_hpp *hpp, bool front);
 typedef int (*hpp_snprint_fn)(struct perf_hpp *hpp, const char *fmt, ...);
 
-int __hpp__fmt(struct perf_hpp *hpp, struct hist_entry *he,
-	       hpp_field_fn get_field, const char *fmt, int len,
-	       hpp_snprint_fn print_fn, bool fmt_percent);
-int __hpp__fmt_acc(struct perf_hpp *hpp, struct hist_entry *he,
-		   hpp_field_fn get_field, const char *fmt, int len,
-		   hpp_snprint_fn print_fn, bool fmt_percent);
+int hpp__fmt(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
+	     struct hist_entry *he, hpp_field_fn get_field,
+	     const char *fmtstr, hpp_snprint_fn print_fn, bool fmt_percent);
+int hpp__fmt_acc(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
+		 struct hist_entry *he, hpp_field_fn get_field,
+		 const char *fmtstr, hpp_snprint_fn print_fn, bool fmt_percent);
 
 static inline void advance_hpp(struct perf_hpp *hpp, int inc)
 {
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index a7f8a7b0eced..c75d95023d4c 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -70,8 +70,10 @@ static int hist_entry__thread_snprintf(struct hist_entry *he, char *bf,
 				       size_t size, unsigned int width)
 {
 	const char *comm = thread__comm_str(he->thread);
-	return repsep_snprintf(bf, size, "%5d:%-*s", he->thread->tid,
-			       width - 6, comm ?: "");
+
+	width = max(7U, width) - 6;
+	return repsep_snprintf(bf, size, "%5d:%-*.*s", he->thread->tid,
+			       width, width, comm ?: "");
 }
 
 struct sort_entry sort_thread = {
@@ -106,7 +108,7 @@ sort__comm_sort(struct hist_entry *left, struct hist_entry *right)
 static int hist_entry__comm_snprintf(struct hist_entry *he, char *bf,
 				     size_t size, unsigned int width)
 {
-	return repsep_snprintf(bf, size, "%-*s", width, comm__str(he->comm));
+	return repsep_snprintf(bf, size, "%-*.*s", width, width, comm__str(he->comm));
 }
 
 struct sort_entry sort_comm = {
@@ -152,10 +154,10 @@ static int _hist_entry__dso_snprintf(struct map *map, char *bf,
 	if (map && map->dso) {
 		const char *dso_name = !verbose ? map->dso->short_name :
 			map->dso->long_name;
-		return repsep_snprintf(bf, size, "%-*s", width, dso_name);
+		return repsep_snprintf(bf, size, "%-*.*s", width, width, dso_name);
 	}
 
-	return repsep_snprintf(bf, size, "%-*s", width, "[unknown]");
+	return repsep_snprintf(bf, size, "%-*.*s", width, width, "[unknown]");
 }
 
 static int hist_entry__dso_snprintf(struct hist_entry *he, char *bf,
@@ -257,7 +259,7 @@ static int _hist_entry__sym_snprintf(struct map *map, struct symbol *sym,
 				       width - ret, "");
 	}
 
-	return ret;
+	return width;
 }
 
 static int hist_entry__sym_snprintf(struct hist_entry *he, char *bf,
@@ -302,10 +304,9 @@ sort__srcline_cmp(struct hist_entry *left, struct hist_entry *right)
 }
 
 static int hist_entry__srcline_snprintf(struct hist_entry *he, char *bf,
-					size_t size,
-					unsigned int width __maybe_unused)
+					size_t size, unsigned int width)
 {
-	return repsep_snprintf(bf, size, "%-s", he->srcline);
+	return repsep_snprintf(bf, size, "%*.*-s", width, width, he->srcline);
 }
 
 struct sort_entry sort_srcline = {
@@ -332,7 +333,7 @@ sort__parent_cmp(struct hist_entry *left, struct hist_entry *right)
 static int hist_entry__parent_snprintf(struct hist_entry *he, char *bf,
 				       size_t size, unsigned int width)
 {
-	return repsep_snprintf(bf, size, "%-*s", width,
+	return repsep_snprintf(bf, size, "%-*.*s", width, width,
 			      he->parent ? he->parent->name : "[other]");
 }
 
@@ -354,7 +355,7 @@ sort__cpu_cmp(struct hist_entry *left, struct hist_entry *right)
 static int hist_entry__cpu_snprintf(struct hist_entry *he, char *bf,
 				    size_t size, unsigned int width)
 {
-	return repsep_snprintf(bf, size, "%*d", width, he->cpu);
+	return repsep_snprintf(bf, size, "%*.*d", width, width, he->cpu);
 }
 
 struct sort_entry sort_cpu = {
@@ -484,7 +485,7 @@ static int hist_entry__mispredict_snprintf(struct hist_entry *he, char *bf,
 	else if (he->branch_info->flags.mispred)
 		out = "Y";
 
-	return repsep_snprintf(bf, size, "%-*s", width, out);
+	return repsep_snprintf(bf, size, "%-*.*s", width, width, out);
 }
 
 /* --sort daddr_sym */
@@ -1210,12 +1211,14 @@ static int __sort__hpp_header(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
 			      struct perf_evsel *evsel)
 {
 	struct hpp_sort_entry *hse;
-	size_t len;
+	size_t len = fmt->user_len;
 
 	hse = container_of(fmt, struct hpp_sort_entry, hpp);
-	len = hists__col_len(&evsel->hists, hse->se->se_width_idx);
 
-	return scnprintf(hpp->buf, hpp->size, "%-*s", len, hse->se->se_header);
+	if (!len)
+		len = hists__col_len(&evsel->hists, hse->se->se_width_idx);
+
+	return scnprintf(hpp->buf, hpp->size, "%-*.*s", len, len, hse->se->se_header);
 }
 
 static int __sort__hpp_width(struct perf_hpp_fmt *fmt,
@@ -1223,20 +1226,26 @@ static int __sort__hpp_width(struct perf_hpp_fmt *fmt,
 			     struct perf_evsel *evsel)
 {
 	struct hpp_sort_entry *hse;
+	size_t len = fmt->user_len;
 
 	hse = container_of(fmt, struct hpp_sort_entry, hpp);
 
-	return hists__col_len(&evsel->hists, hse->se->se_width_idx);
+	if (!len)
+		len = hists__col_len(&evsel->hists, hse->se->se_width_idx);
+
+	return len;
 }
 
 static int __sort__hpp_entry(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
 			     struct hist_entry *he)
 {
 	struct hpp_sort_entry *hse;
-	size_t len;
+	size_t len = fmt->user_len;
 
 	hse = container_of(fmt, struct hpp_sort_entry, hpp);
-	len = hists__col_len(he->hists, hse->se->se_width_idx);
+
+	if (!len)
+		len = hists__col_len(he->hists, hse->se->se_width_idx);
 
 	return hse->se->se_snprintf(he, hpp->buf, hpp->size, len);
 }
@@ -1266,6 +1275,7 @@ __sort_dimension__alloc_hpp(struct sort_dimension *sd)
 	INIT_LIST_HEAD(&hse->hpp.sort_list);
 	hse->hpp.elide = false;
 	hse->hpp.len = 0;
+	hse->hpp.user_len = 0;
 
 	return hse;
 }
-- 
2.0.0


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 5/7] perf top: Add -w option for setting column width
  2014-07-25  1:18 [PATCHSET 0/7] perf tools: Honor column width setting (v2) Namhyung Kim
                   ` (3 preceding siblings ...)
  2014-07-25  1:18 ` [PATCH 4/7] perf report: Honor column width setting Namhyung Kim
@ 2014-07-25  1:18 ` Namhyung Kim
  2014-07-25  1:18 ` [PATCH 6/7] perf tools: Add name field into perf_hpp_fmt Namhyung Kim
  2014-07-25  1:18 ` [PATCH 7/7] perf tools: Fix column alignment when headers aren't shown on TUI Namhyung Kim
  6 siblings, 0 replies; 13+ messages in thread
From: Namhyung Kim @ 2014-07-25  1:18 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Ingo Molnar, Paul Mackerras, Namhyung Kim,
	Namhyung Kim, LKML, Jiri Olsa

Add -w/--column-widths option like perf report does so that users are
able to see symbols even with some very long C++ library/functions.
It can be a list separated by comma for each column.

  $ perf top -w 0,20,30

The value of 0 means there's no limit.

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/Documentation/perf-report.txt | 2 +-
 tools/perf/Documentation/perf-top.txt    | 6 ++++++
 tools/perf/builtin-top.c                 | 3 +++
 3 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/tools/perf/Documentation/perf-report.txt b/tools/perf/Documentation/perf-report.txt
index d2b59af62bc0..d561e0214f52 100644
--- a/tools/perf/Documentation/perf-report.txt
+++ b/tools/perf/Documentation/perf-report.txt
@@ -147,7 +147,7 @@ OPTIONS
 -w::
 --column-widths=<width[,width...]>::
 	Force each column width to the provided list, for large terminal
-	readability.
+	readability.  0 means no limit (default behavior).
 
 -t::
 --field-separator=::
diff --git a/tools/perf/Documentation/perf-top.txt b/tools/perf/Documentation/perf-top.txt
index 180ae02137a5..28fdee394880 100644
--- a/tools/perf/Documentation/perf-top.txt
+++ b/tools/perf/Documentation/perf-top.txt
@@ -193,6 +193,12 @@ Default is to monitor all CPUS.
 	sum of shown entries will be always 100%. "absolute" means it retains
 	the original value before and after the filter is applied.
 
+-w::
+--column-widths=<width[,width...]>::
+	Force each column width to the provided list, for large terminal
+	readability.  0 means no limit (default behavior).
+
+
 INTERACTIVE PROMPTING KEYS
 --------------------------
 
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 377971dc89a3..bde216b2071c 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -1131,6 +1131,9 @@ int cmd_top(int argc, const char **argv, const char *prefix __maybe_unused)
 		     "Don't show entries under that percent", parse_percent_limit),
 	OPT_CALLBACK(0, "percentage", NULL, "relative|absolute",
 		     "How to display percentage of filtered entries", parse_filter_percentage),
+	OPT_STRING('w', "column-widths", &symbol_conf.col_width_list_str,
+		   "width[,width...]",
+		   "don't try to adjust column width, use these fixed values"),
 	OPT_END()
 	};
 	const char * const top_usage[] = {
-- 
2.0.0


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 6/7] perf tools: Add name field into perf_hpp_fmt
  2014-07-25  1:18 [PATCHSET 0/7] perf tools: Honor column width setting (v2) Namhyung Kim
                   ` (4 preceding siblings ...)
  2014-07-25  1:18 ` [PATCH 5/7] perf top: Add -w option for setting column width Namhyung Kim
@ 2014-07-25  1:18 ` Namhyung Kim
  2014-07-25 11:12   ` Jiri Olsa
  2014-07-25  1:18 ` [PATCH 7/7] perf tools: Fix column alignment when headers aren't shown on TUI Namhyung Kim
  6 siblings, 1 reply; 13+ messages in thread
From: Namhyung Kim @ 2014-07-25  1:18 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Ingo Molnar, Paul Mackerras, Namhyung Kim,
	Namhyung Kim, LKML, Jiri Olsa

It makes the code a bit simpler and easier to debug IMHO.

I guess it can also remove similar code in perf diff, but let's keep
it for a future work. :)

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/ui/gtk/hists.c |   4 +-
 tools/perf/ui/hist.c      | 109 ++++++++++++++++++++++------------------------
 tools/perf/util/hist.h    |   1 +
 tools/perf/util/sort.c    |   6 +--
 4 files changed, 57 insertions(+), 63 deletions(-)

diff --git a/tools/perf/ui/gtk/hists.c b/tools/perf/ui/gtk/hists.c
index 897b2e140428..f3fa4258b256 100644
--- a/tools/perf/ui/gtk/hists.c
+++ b/tools/perf/ui/gtk/hists.c
@@ -207,10 +207,8 @@ static void perf_gtk__show_hists(GtkWidget *window, struct hists *hists,
 		if (perf_hpp__is_sort_entry(fmt))
 			sym_col = col_idx;
 
-		fmt->header(fmt, &hpp, hists_to_evsel(hists));
-
 		gtk_tree_view_insert_column_with_attributes(GTK_TREE_VIEW(view),
-							    -1, ltrim(s),
+							    -1, fmt->name,
 							    renderer, "markup",
 							    col_idx++, NULL);
 	}
diff --git a/tools/perf/ui/hist.c b/tools/perf/ui/hist.c
index b2d60a95f01d..93285df06164 100644
--- a/tools/perf/ui/hist.c
+++ b/tools/perf/ui/hist.c
@@ -209,7 +209,7 @@ static int __hpp__sort_acc(struct hist_entry *a, struct hist_entry *b,
 	return ret;
 }
 
-#define __HPP_WIDTH_FN(_type, _str)					\
+#define __HPP_WIDTH_FN(_type)						\
 static int hpp__width_##_type(struct perf_hpp_fmt *fmt,			\
 			      struct perf_hpp *hpp __maybe_unused,	\
 			      struct perf_evsel *evsel)			\
@@ -219,19 +219,19 @@ static int hpp__width_##_type(struct perf_hpp_fmt *fmt,			\
 	if (symbol_conf.event_group)					\
 		len = max(len, evsel->nr_members * fmt->len);		\
 									\
-	if (len < (int)strlen(_str))					\
-		len = strlen(_str);					\
+	if (len < (int)strlen(fmt->name))				\
+		len = strlen(fmt->name);				\
 									\
 	return len;							\
 }
 
-#define __HPP_HEADER_FN(_type, _str) 					\
+#define __HPP_HEADER_FN(_type) 						\
 static int hpp__header_##_type(struct perf_hpp_fmt *fmt,		\
 			       struct perf_hpp *hpp,			\
 			       struct perf_evsel *evsel)		\
 {									\
 	int len = hpp__width_##_type(fmt, hpp, evsel);			\
-	return scnprintf(hpp->buf, hpp->size, "%*s", len, _str);	\
+	return scnprintf(hpp->buf, hpp->size, "%*s", len, fmt->name);	\
 }
 
 static int hpp_color_scnprintf(struct perf_hpp *hpp, const char *fmt, ...)
@@ -337,38 +337,35 @@ static int64_t hpp__sort_##_type(struct hist_entry *a, struct hist_entry *b)	\
 }
 
 
-#define HPP_PERCENT_FNS(_type, _str, _field)				\
-__HPP_WIDTH_FN(_type, _str)						\
-__HPP_HEADER_FN(_type, _str)						\
+#define HPP_PERCENT_FNS(_type, _field)					\
+__HPP_WIDTH_FN(_type)							\
+__HPP_HEADER_FN(_type)							\
 __HPP_COLOR_PERCENT_FN(_type, _field)					\
 __HPP_ENTRY_PERCENT_FN(_type, _field)					\
 __HPP_SORT_FN(_type, _field)
 
-#define HPP_PERCENT_ACC_FNS(_type, _str, _field)			\
-__HPP_WIDTH_FN(_type, _str)						\
-__HPP_HEADER_FN(_type, _str)						\
+#define HPP_PERCENT_ACC_FNS(_type, _field)				\
+__HPP_WIDTH_FN(_type)							\
+__HPP_HEADER_FN(_type)							\
 __HPP_COLOR_ACC_PERCENT_FN(_type, _field)				\
 __HPP_ENTRY_ACC_PERCENT_FN(_type, _field)				\
 __HPP_SORT_ACC_FN(_type, _field)
 
-#define HPP_RAW_FNS(_type, _str, _field)				\
-__HPP_WIDTH_FN(_type, _str)						\
-__HPP_HEADER_FN(_type, _str)						\
+#define HPP_RAW_FNS(_type, _field)					\
+__HPP_WIDTH_FN(_type)							\
+__HPP_HEADER_FN(_type)							\
 __HPP_ENTRY_RAW_FN(_type, _field)					\
 __HPP_SORT_RAW_FN(_type, _field)
 
-__HPP_WIDTH_FN(overhead_self, "Self")
-__HPP_HEADER_FN(overhead_self, "Self")
+HPP_PERCENT_FNS(overhead, period)
+HPP_PERCENT_FNS(overhead_sys, period_sys)
+HPP_PERCENT_FNS(overhead_us, period_us)
+HPP_PERCENT_FNS(overhead_guest_sys, period_guest_sys)
+HPP_PERCENT_FNS(overhead_guest_us, period_guest_us)
+HPP_PERCENT_ACC_FNS(overhead_acc, period)
 
-HPP_PERCENT_FNS(overhead, "Overhead", period)
-HPP_PERCENT_FNS(overhead_sys, "sys", period_sys)
-HPP_PERCENT_FNS(overhead_us, "usr", period_us)
-HPP_PERCENT_FNS(overhead_guest_sys, "guest sys", period_guest_sys)
-HPP_PERCENT_FNS(overhead_guest_us, "guest usr", period_guest_us)
-HPP_PERCENT_ACC_FNS(overhead_acc, "Children", period)
-
-HPP_RAW_FNS(samples, "Samples", nr_events)
-HPP_RAW_FNS(period, "Period", period)
+HPP_RAW_FNS(samples, nr_events)
+HPP_RAW_FNS(period, period)
 
 static int64_t hpp__nop_cmp(struct hist_entry *a __maybe_unused,
 			    struct hist_entry *b __maybe_unused)
@@ -376,47 +373,50 @@ static int64_t hpp__nop_cmp(struct hist_entry *a __maybe_unused,
 	return 0;
 }
 
-#define HPP__COLOR_PRINT_FNS(_name)			\
+#define HPP__COLOR_PRINT_FNS(_name, _fn)		\
 	{						\
-		.header	= hpp__header_ ## _name,	\
-		.width	= hpp__width_ ## _name,		\
-		.color	= hpp__color_ ## _name,		\
-		.entry	= hpp__entry_ ## _name,		\
+		.name   = _name,			\
+		.header	= hpp__header_ ## _fn,		\
+		.width	= hpp__width_ ## _fn,		\
+		.color	= hpp__color_ ## _fn,		\
+		.entry	= hpp__entry_ ## _fn,		\
 		.cmp	= hpp__nop_cmp,			\
 		.collapse = hpp__nop_cmp,		\
-		.sort	= hpp__sort_ ## _name,		\
+		.sort	= hpp__sort_ ## _fn,		\
 	}
 
-#define HPP__COLOR_ACC_PRINT_FNS(_name)			\
+#define HPP__COLOR_ACC_PRINT_FNS(_name, _fn)		\
 	{						\
-		.header	= hpp__header_ ## _name,	\
-		.width	= hpp__width_ ## _name,		\
-		.color	= hpp__color_ ## _name,		\
-		.entry	= hpp__entry_ ## _name,		\
+		.name   = _name,			\
+		.header	= hpp__header_ ## _fn,		\
+		.width	= hpp__width_ ## _fn,		\
+		.color	= hpp__color_ ## _fn,		\
+		.entry	= hpp__entry_ ## _fn,		\
 		.cmp	= hpp__nop_cmp,			\
 		.collapse = hpp__nop_cmp,		\
-		.sort	= hpp__sort_ ## _name,		\
+		.sort	= hpp__sort_ ## _fn,		\
 	}
 
-#define HPP__PRINT_FNS(_name)				\
+#define HPP__PRINT_FNS(_name, _fn)			\
 	{						\
-		.header	= hpp__header_ ## _name,	\
-		.width	= hpp__width_ ## _name,		\
-		.entry	= hpp__entry_ ## _name,		\
+		.name   = _name,			\
+		.header	= hpp__header_ ## _fn,	\
+		.width	= hpp__width_ ## _fn,		\
+		.entry	= hpp__entry_ ## _fn,		\
 		.cmp	= hpp__nop_cmp,			\
 		.collapse = hpp__nop_cmp,		\
-		.sort	= hpp__sort_ ## _name,		\
+		.sort	= hpp__sort_ ## _fn,		\
 	}
 
 struct perf_hpp_fmt perf_hpp__format[] = {
-	HPP__COLOR_PRINT_FNS(overhead),
-	HPP__COLOR_PRINT_FNS(overhead_sys),
-	HPP__COLOR_PRINT_FNS(overhead_us),
-	HPP__COLOR_PRINT_FNS(overhead_guest_sys),
-	HPP__COLOR_PRINT_FNS(overhead_guest_us),
-	HPP__COLOR_ACC_PRINT_FNS(overhead_acc),
-	HPP__PRINT_FNS(samples),
-	HPP__PRINT_FNS(period)
+	HPP__COLOR_PRINT_FNS("Overhead", overhead),
+	HPP__COLOR_PRINT_FNS("sys", overhead_sys),
+	HPP__COLOR_PRINT_FNS("usr", overhead_us),
+	HPP__COLOR_PRINT_FNS("guest sys", overhead_guest_sys),
+	HPP__COLOR_PRINT_FNS("guest usr", overhead_guest_us),
+	HPP__COLOR_ACC_PRINT_FNS("Children", overhead_acc),
+	HPP__PRINT_FNS("Samples", samples),
+	HPP__PRINT_FNS("Period", period)
 };
 
 LIST_HEAD(perf_hpp__list);
@@ -466,11 +466,7 @@ void perf_hpp__init(void)
 
 	if (symbol_conf.cumulate_callchain) {
 		perf_hpp__column_enable(PERF_HPP__OVERHEAD_ACC);
-
-		perf_hpp__format[PERF_HPP__OVERHEAD].header =
-						hpp__header_overhead_self;
-		perf_hpp__format[PERF_HPP__OVERHEAD].width =
-						hpp__width_overhead_self;
+		perf_hpp__format[PERF_HPP__OVERHEAD].name = "Self";
 	}
 
 	perf_hpp__column_enable(PERF_HPP__OVERHEAD);
@@ -536,8 +532,7 @@ void perf_hpp__cancel_cumulate(void)
 		return;
 
 	perf_hpp__column_disable(PERF_HPP__OVERHEAD_ACC);
-	perf_hpp__format[PERF_HPP__OVERHEAD].header = hpp__header_overhead;
-	perf_hpp__format[PERF_HPP__OVERHEAD].width = hpp__width_overhead;
+	perf_hpp__format[PERF_HPP__OVERHEAD].name = "Overhead";
 }
 
 void perf_hpp__setup_output_field(void)
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 2e70003fcd00..95405a8fbd95 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -192,6 +192,7 @@ struct perf_hpp {
 };
 
 struct perf_hpp_fmt {
+	const char *name;
 	int (*header)(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
 		      struct perf_evsel *evsel);
 	int (*width)(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index c75d95023d4c..0be2d2bc922a 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -1203,8 +1203,7 @@ void perf_hpp__reset_sort_width(struct perf_hpp_fmt *fmt, struct hists *hists)
 		return;
 
 	hse = container_of(fmt, struct hpp_sort_entry, hpp);
-	hists__new_col_len(hists, hse->se->se_width_idx,
-			   strlen(hse->se->se_header));
+	hists__new_col_len(hists, hse->se->se_width_idx, strlen(fmt->name));
 }
 
 static int __sort__hpp_header(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
@@ -1218,7 +1217,7 @@ static int __sort__hpp_header(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
 	if (!len)
 		len = hists__col_len(&evsel->hists, hse->se->se_width_idx);
 
-	return scnprintf(hpp->buf, hpp->size, "%-*.*s", len, len, hse->se->se_header);
+	return scnprintf(hpp->buf, hpp->size, "%-*.*s", len, len, fmt->name);
 }
 
 static int __sort__hpp_width(struct perf_hpp_fmt *fmt,
@@ -1262,6 +1261,7 @@ __sort_dimension__alloc_hpp(struct sort_dimension *sd)
 	}
 
 	hse->se = sd->entry;
+	hse->hpp.name = sd->entry->se_header;
 	hse->hpp.header = __sort__hpp_header;
 	hse->hpp.width = __sort__hpp_width;
 	hse->hpp.entry = __sort__hpp_entry;
-- 
2.0.0


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 7/7] perf tools: Fix column alignment when headers aren't shown on TUI
  2014-07-25  1:18 [PATCHSET 0/7] perf tools: Honor column width setting (v2) Namhyung Kim
                   ` (5 preceding siblings ...)
  2014-07-25  1:18 ` [PATCH 6/7] perf tools: Add name field into perf_hpp_fmt Namhyung Kim
@ 2014-07-25  1:18 ` Namhyung Kim
  6 siblings, 0 replies; 13+ messages in thread
From: Namhyung Kim @ 2014-07-25  1:18 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Ingo Molnar, Paul Mackerras, Namhyung Kim,
	Namhyung Kim, LKML, Jiri Olsa

If user sets ui.show-headers config option to false, it didn't
calculate default column width so it broke the alignment.  This is
because it does the calculation just before showing headers.

Move it to the beginning of the hist browser so that it can be called
regardless of the config option.

Reported-by: Jiri Olsa <jolsa@redhat.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/ui/browsers/hists.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index f0ef031558dd..ab9ca221fc2c 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -853,9 +853,6 @@ static int hists__scnprintf_headers(char *buf, size_t size, struct hists *hists)
 		if (perf_hpp__should_skip(fmt))
 			continue;
 
-		/* We need to ensure length of the columns header. */
-		perf_hpp__reset_width(fmt, hists);
-
 		ret = fmt->header(fmt, &dummy_hpp, hists_to_evsel(hists));
 		if (advance_hpp_check(&dummy_hpp, ret))
 			break;
@@ -1504,6 +1501,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 	char buf[64];
 	char script_opt[64];
 	int delay_secs = hbt ? hbt->refresh : 0;
+	struct perf_hpp_fmt *fmt;
 
 #define HIST_BROWSER_HELP_COMMON					\
 	"h/?/F1        Show this window\n"				\
@@ -1553,6 +1551,9 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 
 	memset(options, 0, sizeof(options));
 
+	perf_hpp__for_each_format(fmt)
+		perf_hpp__reset_width(fmt, hists);
+
 	if (symbol_conf.col_width_list_str)
 		perf_hpp__set_user_width(symbol_conf.col_width_list_str);
 
-- 
2.0.0


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 6/7] perf tools: Add name field into perf_hpp_fmt
  2014-07-25  1:18 ` [PATCH 6/7] perf tools: Add name field into perf_hpp_fmt Namhyung Kim
@ 2014-07-25 11:12   ` Jiri Olsa
  2014-07-28  0:35     ` Namhyung Kim
  0 siblings, 1 reply; 13+ messages in thread
From: Jiri Olsa @ 2014-07-25 11:12 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
	Paul Mackerras, Namhyung Kim, LKML

On Fri, Jul 25, 2014 at 10:18:57AM +0900, Namhyung Kim wrote:

SNIP

>  }
>  
> -#define __HPP_WIDTH_FN(_type, _str)					\
> +#define __HPP_WIDTH_FN(_type)						\
>  static int hpp__width_##_type(struct perf_hpp_fmt *fmt,			\
>  			      struct perf_hpp *hpp __maybe_unused,	\
>  			      struct perf_evsel *evsel)			\
> @@ -219,19 +219,19 @@ static int hpp__width_##_type(struct perf_hpp_fmt *fmt,			\
>  	if (symbol_conf.event_group)					\
>  		len = max(len, evsel->nr_members * fmt->len);		\
>  									\
> -	if (len < (int)strlen(_str))					\
> -		len = strlen(_str);					\
> +	if (len < (int)strlen(fmt->name))				\
> +		len = strlen(fmt->name);				\

hum, so with the '_str' string argument missing we dont need this
function to be generated by macro, right? could be one single function
called hpp__width_entry or something like that

>  									\
>  	return len;							\
>  }
>  
> -#define __HPP_HEADER_FN(_type, _str) 					\
> +#define __HPP_HEADER_FN(_type) 						\
>  static int hpp__header_##_type(struct perf_hpp_fmt *fmt,		\
>  			       struct perf_hpp *hpp,			\
>  			       struct perf_evsel *evsel)		\
>  {									\
>  	int len = hpp__width_##_type(fmt, hpp, evsel);			\
> -	return scnprintf(hpp->buf, hpp->size, "%*s", len, _str);	\
> +	return scnprintf(hpp->buf, hpp->size, "%*s", len, fmt->name);	\
>  }
>  

same here ^^^ for hpp__header_##_type


jirka

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 4/7] perf report: Honor column width setting
  2014-07-25  1:18 ` [PATCH 4/7] perf report: Honor column width setting Namhyung Kim
@ 2014-07-25 11:13   ` Jiri Olsa
  2014-07-28  0:43     ` Namhyung Kim
  0 siblings, 1 reply; 13+ messages in thread
From: Jiri Olsa @ 2014-07-25 11:13 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
	Paul Mackerras, Namhyung Kim, LKML

On Fri, Jul 25, 2014 at 10:18:55AM +0900, Namhyung Kim wrote:
> Set column width and do not change it if user gives -w/--column-widths
> option.  It'll truncate longer symbols than the width if exists.
> 
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---

SNIP

>  
>  __HPP_COLOR_PERCENT_FN(overhead, period)
> @@ -797,8 +798,11 @@ static int hist_browser__show_entry(struct hist_browser *browser,
>  			if (fmt->color) {
>  				width -= fmt->color(fmt, &hpp, entry);
>  			} else {
> -				width -= fmt->entry(fmt, &hpp, entry);
> +				int ret = fmt->entry(fmt, &hpp, entry);
> +				s[ret] = '\0';
>  				slsmg_printf("%s", s);

from quick check it seems like all entry callback functions
use some sort of snprint which should already handle the string
length properly.. if not, we should fix that instead, no?

jirka

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 6/7] perf tools: Add name field into perf_hpp_fmt
  2014-07-25 11:12   ` Jiri Olsa
@ 2014-07-28  0:35     ` Namhyung Kim
  0 siblings, 0 replies; 13+ messages in thread
From: Namhyung Kim @ 2014-07-28  0:35 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
	Paul Mackerras, Namhyung Kim, LKML

Hi Jiri,

On Fri, 25 Jul 2014 13:12:30 +0200, Jiri Olsa wrote:
> On Fri, Jul 25, 2014 at 10:18:57AM +0900, Namhyung Kim wrote:
>
> SNIP
>
>>  }
>>  
>> -#define __HPP_WIDTH_FN(_type, _str)					\
>> +#define __HPP_WIDTH_FN(_type)						\
>>  static int hpp__width_##_type(struct perf_hpp_fmt *fmt,			\
>>  			      struct perf_hpp *hpp __maybe_unused,	\
>>  			      struct perf_evsel *evsel)			\
>> @@ -219,19 +219,19 @@ static int hpp__width_##_type(struct perf_hpp_fmt *fmt,			\
>>  	if (symbol_conf.event_group)					\
>>  		len = max(len, evsel->nr_members * fmt->len);		\
>>  									\
>> -	if (len < (int)strlen(_str))					\
>> -		len = strlen(_str);					\
>> +	if (len < (int)strlen(fmt->name))				\
>> +		len = strlen(fmt->name);				\
>
> hum, so with the '_str' string argument missing we dont need this
> function to be generated by macro, right? could be one single function
> called hpp__width_entry or something like that

You're right.  Will change both!

Thanks,
Namhyung


>
>>  									\
>>  	return len;							\
>>  }
>>  
>> -#define __HPP_HEADER_FN(_type, _str) 					\
>> +#define __HPP_HEADER_FN(_type) 						\
>>  static int hpp__header_##_type(struct perf_hpp_fmt *fmt,		\
>>  			       struct perf_hpp *hpp,			\
>>  			       struct perf_evsel *evsel)		\
>>  {									\
>>  	int len = hpp__width_##_type(fmt, hpp, evsel);			\
>> -	return scnprintf(hpp->buf, hpp->size, "%*s", len, _str);	\
>> +	return scnprintf(hpp->buf, hpp->size, "%*s", len, fmt->name);	\
>>  }
>>  
>
> same here ^^^ for hpp__header_##_type
>
>
> jirka

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 4/7] perf report: Honor column width setting
  2014-07-25 11:13   ` Jiri Olsa
@ 2014-07-28  0:43     ` Namhyung Kim
  2014-07-28 11:17       ` Jiri Olsa
  0 siblings, 1 reply; 13+ messages in thread
From: Namhyung Kim @ 2014-07-28  0:43 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
	Paul Mackerras, Namhyung Kim, LKML

On Fri, 25 Jul 2014 13:13:02 +0200, Jiri Olsa wrote:
> On Fri, Jul 25, 2014 at 10:18:55AM +0900, Namhyung Kim wrote:
>> Set column width and do not change it if user gives -w/--column-widths
>> option.  It'll truncate longer symbols than the width if exists.
>> 
>> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
>> ---
>
> SNIP
>
>>  
>>  __HPP_COLOR_PERCENT_FN(overhead, period)
>> @@ -797,8 +798,11 @@ static int hist_browser__show_entry(struct hist_browser *browser,
>>  			if (fmt->color) {
>>  				width -= fmt->color(fmt, &hpp, entry);
>>  			} else {
>> -				width -= fmt->entry(fmt, &hpp, entry);
>> +				int ret = fmt->entry(fmt, &hpp, entry);
>> +				s[ret] = '\0';
>>  				slsmg_printf("%s", s);
>
> from quick check it seems like all entry callback functions
> use some sort of snprint which should already handle the string
> length properly.. if not, we should fix that instead, no?

It was 'symbol' sort entry (_hist_entry__sym_snprintf) which has several
snprint calls in it.  So I changed here rather than adding complexity
the print function.  I can change it if you want. ;)

Thanks,
Namhyung

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 4/7] perf report: Honor column width setting
  2014-07-28  0:43     ` Namhyung Kim
@ 2014-07-28 11:17       ` Jiri Olsa
  0 siblings, 0 replies; 13+ messages in thread
From: Jiri Olsa @ 2014-07-28 11:17 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
	Paul Mackerras, Namhyung Kim, LKML

On Mon, Jul 28, 2014 at 09:43:37AM +0900, Namhyung Kim wrote:
> On Fri, 25 Jul 2014 13:13:02 +0200, Jiri Olsa wrote:
> > On Fri, Jul 25, 2014 at 10:18:55AM +0900, Namhyung Kim wrote:
> >> Set column width and do not change it if user gives -w/--column-widths
> >> option.  It'll truncate longer symbols than the width if exists.
> >> 
> >> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> >> ---
> >
> > SNIP
> >
> >>  
> >>  __HPP_COLOR_PERCENT_FN(overhead, period)
> >> @@ -797,8 +798,11 @@ static int hist_browser__show_entry(struct hist_browser *browser,
> >>  			if (fmt->color) {
> >>  				width -= fmt->color(fmt, &hpp, entry);
> >>  			} else {
> >> -				width -= fmt->entry(fmt, &hpp, entry);
> >> +				int ret = fmt->entry(fmt, &hpp, entry);
> >> +				s[ret] = '\0';
> >>  				slsmg_printf("%s", s);
> >
> > from quick check it seems like all entry callback functions
> > use some sort of snprint which should already handle the string
> > length properly.. if not, we should fix that instead, no?
> 
> It was 'symbol' sort entry (_hist_entry__sym_snprintf) which has several
> snprint calls in it.  So I changed here rather than adding complexity
> the print function.  I can change it if you want. ;)

it'd seem more consistent to me.. Arnaldo? ;-)

jirka

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2014-07-28 11:17 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-25  1:18 [PATCHSET 0/7] perf tools: Honor column width setting (v2) Namhyung Kim
2014-07-25  1:18 ` [PATCH 1/7] perf tools: Left-align output contents Namhyung Kim
2014-07-25  1:18 ` [PATCH 2/7] perf tools: Make __hpp__fmt() receive an additional len argument Namhyung Kim
2014-07-25  1:18 ` [PATCH 3/7] perf tools: Save column length in perf_hpp_fmt Namhyung Kim
2014-07-25  1:18 ` [PATCH 4/7] perf report: Honor column width setting Namhyung Kim
2014-07-25 11:13   ` Jiri Olsa
2014-07-28  0:43     ` Namhyung Kim
2014-07-28 11:17       ` Jiri Olsa
2014-07-25  1:18 ` [PATCH 5/7] perf top: Add -w option for setting column width Namhyung Kim
2014-07-25  1:18 ` [PATCH 6/7] perf tools: Add name field into perf_hpp_fmt Namhyung Kim
2014-07-25 11:12   ` Jiri Olsa
2014-07-28  0:35     ` Namhyung Kim
2014-07-25  1:18 ` [PATCH 7/7] perf tools: Fix column alignment when headers aren't shown on TUI Namhyung Kim

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome