[PATCHSET 0/6] perf tools: Honor column width setting

[PATCHSET 0/6] perf tools: Honor column width setting

[Namhyung Kim]

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]   


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

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


Thanks,
Namhyung


Namhyung Kim (6):
  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

 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           |  31 ++--
 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                   |  51 +++---
 11 files changed, 274 insertions(+), 150 deletions(-)

-- 
2.0.0

[PATCH 1/6] perf tools: Left-align output contents

[Namhyung Kim]

Now perf left-aligns column headers but the contents does not.  It
should have same alignment.

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

diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index 14e5a039bc45..9a531743c8c2 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -70,7 +70,7 @@ 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,
+	return repsep_snprintf(bf, size, "%-*s:%5d", width - 6,
 			       comm ?: "", he->thread->tid);
 }
 
@@ -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

[PATCH 2/6] perf tools: Make __hpp__fmt() receive an additional len argument

[Namhyung Kim]

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

[PATCH 3/6] perf tools: Save column length in perf_hpp_fmt

[Namhyung Kim]

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 9a531743c8c2..396b0094943d 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

[PATCH 4/6] perf report: Honor column width setting

[Namhyung Kim]

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         | 44 +++++++++++++---------
 6 files changed, 116 insertions(+), 63 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 90122abd3721..59da427c11bb 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 396b0094943d..f89714329c0f 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -70,7 +70,9 @@ 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,
+
+	width = max(7U, width) - 6;
+	return repsep_snprintf(bf, size, "%-*.*s:%5d", width, width,
 			       comm ?: "", he->thread->tid);
 }
 
@@ -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

[PATCH 5/6] perf top: Add -w option for setting column width

[Namhyung Kim]

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

[PATCH 6/6] perf tools: Add name field into perf_hpp_fmt

[Namhyung Kim]

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 f89714329c0f..7a1944f8d001 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

Re: [PATCHSET 0/6] perf tools: Honor column width setting

[Jiri Olsa]

On Wed, Jul 09, 2014 at 02:28:08PM +0900, Namhyung Kim wrote:
> 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.

hi,
I've got broken TUI output for "perf report --group" 

Samples: 17  of event 'anon group { cycles, instructions }', Event count (approx.): 9145256
 56.44% 31.59%  ls  libc-2.17.so       [.] __strcoll_l
 39.94% 0.00%  ls  ld-2.17.so         [.] _dl_new_object
 3.48% 0.00%  ls  [kernel.kallsyms]  [k] setup_arg_pages
 0.14% 0.33%  ls  [kernel.kallsyms]  [k] native_write_msr_safe
 0.00% 30.17%  ls  [kernel.kallsyms]  [k] security_inode_permission
 0.00% 29.78%  ls  ls                 [.] indent
 0.00% 8.12%  ls  [kernel.kallsyms]  [k] __slab_alloc

I have 'show-headers' set to false in ~/.perfconfig and the output
got fixed after displaying headers by pressing 'H'

jirka

Re: [PATCHSET 0/6] perf tools: Honor column width setting

[Namhyung Kim]

Hi Jiri,

On Mon, 21 Jul 2014 11:07:55 +0200, Jiri Olsa wrote:
> On Wed, Jul 09, 2014 at 02:28:08PM +0900, Namhyung Kim wrote:
>> 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.
>
> hi,
> I've got broken TUI output for "perf report --group" 
>
> Samples: 17  of event 'anon group { cycles, instructions }', Event count (approx.): 9145256
>  56.44% 31.59%  ls  libc-2.17.so       [.] __strcoll_l
>  39.94% 0.00%  ls  ld-2.17.so         [.] _dl_new_object
>  3.48% 0.00%  ls  [kernel.kallsyms]  [k] setup_arg_pages
>  0.14% 0.33%  ls  [kernel.kallsyms]  [k] native_write_msr_safe
>  0.00% 30.17%  ls  [kernel.kallsyms]  [k] security_inode_permission
>  0.00% 29.78%  ls  ls                 [.] indent
>  0.00% 8.12%  ls  [kernel.kallsyms]  [k] __slab_alloc
>
> I have 'show-headers' set to false in ~/.perfconfig and the output
> got fixed after displaying headers by pressing 'H'

Argh, you're right.  It should be fixed with this patch:

Thanks,
Namhyung



>From a8fffc7802402d5ddd03e02956304199fa20fae9 Mon Sep 17 00:00:00 2001
From: Namhyung Kim <namhyung@kernel.org>
Date: Wed, 23 Jul 2014 16:20:45 +0900
Subject: [PATCH] perf tools: Fix column alignment when headers aren't shown on
 TUI

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

Re: [PATCH 3/6] perf tools: Save column length in perf_hpp_fmt

[Jiri Olsa]

On Wed, Jul 09, 2014 at 02:28:11PM +0900, Namhyung Kim wrote:
> 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.

SNIP

> +
> +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;

just curious.. why is it 9 for guest %, while we use 8 for host?
I understand that was the current state

jirka

Re: [PATCH 4/6] perf report: Honor column width setting

[Jiri Olsa]

On Wed, Jul 09, 2014 at 02:28:12PM +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.

SNIP

>  
>  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 396b0094943d..f89714329c0f 100644
> --- a/tools/perf/util/sort.c
> +++ b/tools/perf/util/sort.c
> @@ -70,7 +70,9 @@ 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,
> +
> +	width = max(7U, width) - 6;
> +	return repsep_snprintf(bf, size, "%-*.*s:%5d", width, width,
>  			       comm ?: "", he->thread->tid);

I couldn't found what your format string:
  "%-*.*s:%5d", width, width

is supposed to do... it gives me the same result as for:
  "%-*s:%5d", width

you use it on more places so I guess it's not a typo ;-)

thanks,
jirka

Re: [PATCH 4/6] perf report: Honor column width setting

[Jiri Olsa]

On Wed, Jul 09, 2014 at 02:28:12PM +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.

SNIP

> --- a/tools/perf/util/sort.c
> +++ b/tools/perf/util/sort.c
> @@ -70,7 +70,9 @@ 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,
> +
> +	width = max(7U, width) - 6;
> +	return repsep_snprintf(bf, size, "%-*.*s:%5d", width, width,
>  			       comm ?: "", he->thread->tid);
>  }

also we now print out all sort headers left alligned, which is wrong for
sort_thread header: "Command:  Pid' that mirrors value strings with ':'
being aligned

maybe something like in attached patch..?

thanks,
jirka


---
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index f89714329c0f..fbbe86b7f63c 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -78,6 +78,7 @@ static int hist_entry__thread_snprintf(struct hist_entry *he, char *bf,
 
 struct sort_entry sort_thread = {
 	.se_header	= "Command:  Pid",
+	.se_header_right= true,
 	.se_cmp		= sort__thread_cmp,
 	.se_snprintf	= hist_entry__thread_snprintf,
 	.se_width_idx	= HISTC_THREAD,
@@ -1212,13 +1213,17 @@ static int __sort__hpp_header(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
 {
 	struct hpp_sort_entry *hse;
 	size_t len = fmt->user_len;
+	const char *fmt_str = "%-*.*s";
 
 	hse = container_of(fmt, struct hpp_sort_entry, 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);
+	if (hse->se->se_header_right)
+		fmt_str = "%*.*s";
+
+	return scnprintf(hpp->buf, hpp->size, fmt_str, len, len, hse->se->se_header);
 }
 
 static int __sort__hpp_width(struct perf_hpp_fmt *fmt,
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index 041f0c9cea2b..16a07b9b16c2 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -197,6 +197,7 @@ struct sort_entry {
 	struct list_head list;
 
 	const char *se_header;
+	bool se_header_right;
 
 	int64_t (*se_cmp)(struct hist_entry *, struct hist_entry *);
 	int64_t (*se_collapse)(struct hist_entry *, struct hist_entry *);

Re: [PATCHSET 0/6] perf tools: Honor column width setting

[Jiri Olsa]

On Wed, Jul 23, 2014 at 04:40:36PM +0900, Namhyung Kim wrote:
> Hi Jiri,
> 
> On Mon, 21 Jul 2014 11:07:55 +0200, Jiri Olsa wrote:
> > On Wed, Jul 09, 2014 at 02:28:08PM +0900, Namhyung Kim wrote:
> >> 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.
> >
> > hi,
> > I've got broken TUI output for "perf report --group" 
> >
> > Samples: 17  of event 'anon group { cycles, instructions }', Event count (approx.): 9145256
> >  56.44% 31.59%  ls  libc-2.17.so       [.] __strcoll_l
> >  39.94% 0.00%  ls  ld-2.17.so         [.] _dl_new_object
> >  3.48% 0.00%  ls  [kernel.kallsyms]  [k] setup_arg_pages
> >  0.14% 0.33%  ls  [kernel.kallsyms]  [k] native_write_msr_safe
> >  0.00% 30.17%  ls  [kernel.kallsyms]  [k] security_inode_permission
> >  0.00% 29.78%  ls  ls                 [.] indent
> >  0.00% 8.12%  ls  [kernel.kallsyms]  [k] __slab_alloc
> >
> > I have 'show-headers' set to false in ~/.perfconfig and the output
> > got fixed after displaying headers by pressing 'H'
> 
> Argh, you're right.  It should be fixed with this patch:

yep, that fixed that for me

thanks,
jirka

Re: [PATCH 4/6] perf report: Honor column width setting

[Arnaldo Carvalho de Melo]

Em Thu, Jul 24, 2014 at 02:57:51PM +0200, Jiri Olsa escreveu:
> On Wed, Jul 09, 2014 at 02:28:12PM +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.
> 
> SNIP
> 
> > --- a/tools/perf/util/sort.c
> > +++ b/tools/perf/util/sort.c
> > @@ -70,7 +70,9 @@ 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,
> > +
> > +	width = max(7U, width) - 6;
> > +	return repsep_snprintf(bf, size, "%-*.*s:%5d", width, width,
> >  			       comm ?: "", he->thread->tid);
> >  }
> 
> also we now print out all sort headers left alligned, which is wrong for
> sort_thread header: "Command:  Pid' that mirrors value strings with ':'
> being aligned
> 
> maybe something like in attached patch..?
> 
> thanks,
> jirka
> 
> 
> ---
> diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
> index f89714329c0f..fbbe86b7f63c 100644
> --- a/tools/perf/util/sort.c
> +++ b/tools/perf/util/sort.c
> @@ -78,6 +78,7 @@ static int hist_entry__thread_snprintf(struct hist_entry *he, char *bf,
>  
>  struct sort_entry sort_thread = {
>  	.se_header	= "Command:  Pid",
> +	.se_header_right= true,

Is this just for the header? Perhaps its more clearly named .se_right_justify?

>  	.se_cmp		= sort__thread_cmp,
>  	.se_snprintf	= hist_entry__thread_snprintf,
>  	.se_width_idx	= HISTC_THREAD,
> @@ -1212,13 +1213,17 @@ static int __sort__hpp_header(struct perf_hpp_fmt *fmt, struct perf_hpp *hpp,
>  {
>  	struct hpp_sort_entry *hse;
>  	size_t len = fmt->user_len;
> +	const char *fmt_str = "%-*.*s";
>  
>  	hse = container_of(fmt, struct hpp_sort_entry, 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);
> +	if (hse->se->se_header_right)
> +		fmt_str = "%*.*s";
> +
> +	return scnprintf(hpp->buf, hpp->size, fmt_str, len, len, hse->se->se_header);
>  }
>  
>  static int __sort__hpp_width(struct perf_hpp_fmt *fmt,
> diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
> index 041f0c9cea2b..16a07b9b16c2 100644
> --- a/tools/perf/util/sort.h
> +++ b/tools/perf/util/sort.h
> @@ -197,6 +197,7 @@ struct sort_entry {
>  	struct list_head list;
>  
>  	const char *se_header;
> +	bool se_header_right;
>  
>  	int64_t (*se_cmp)(struct hist_entry *, struct hist_entry *);
>  	int64_t (*se_collapse)(struct hist_entry *, struct hist_entry *);

Re: [PATCHSET 0/6] perf tools: Honor column width setting

[Arnaldo Carvalho de Melo]

Em Wed, Jul 09, 2014 at 02:28:08PM +0900, Namhyung Kim escreveu:
> 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.

Due to the issues pointed out, will wait for a v2 patchset, ok?

- Arnaldo

Re: [PATCH 4/6] perf report: Honor column width setting

[Jiri Olsa]

On Thu, Jul 24, 2014 at 10:57:47AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Thu, Jul 24, 2014 at 02:57:51PM +0200, Jiri Olsa escreveu:
> > On Wed, Jul 09, 2014 at 02:28:12PM +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.
> > 
> > SNIP
> > 
> > > --- a/tools/perf/util/sort.c
> > > +++ b/tools/perf/util/sort.c
> > > @@ -70,7 +70,9 @@ 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,
> > > +
> > > +	width = max(7U, width) - 6;
> > > +	return repsep_snprintf(bf, size, "%-*.*s:%5d", width, width,
> > >  			       comm ?: "", he->thread->tid);
> > >  }
> > 
> > also we now print out all sort headers left alligned, which is wrong for
> > sort_thread header: "Command:  Pid' that mirrors value strings with ':'
> > being aligned
> > 
> > maybe something like in attached patch..?
> > 
> > thanks,
> > jirka
> > 
> > 
> > ---
> > diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
> > index f89714329c0f..fbbe86b7f63c 100644
> > --- a/tools/perf/util/sort.c
> > +++ b/tools/perf/util/sort.c
> > @@ -78,6 +78,7 @@ static int hist_entry__thread_snprintf(struct hist_entry *he, char *bf,
> >  
> >  struct sort_entry sort_thread = {
> >  	.se_header	= "Command:  Pid",
> > +	.se_header_right= true,
> 
> Is this just for the header? Perhaps its more clearly named .se_right_justify?

yep, it's just for header.. let's see Namhyung might have better
idea how to handle this problem

jirka

Re: [PATCH 4/6] perf report: Honor column width setting

[Namhyung Kim]

Hi guys,

On Thu, Jul 24, 2014 at 11:49 PM, Jiri Olsa <jolsa@redhat.com> wrote:
> On Thu, Jul 24, 2014 at 10:57:47AM -0300, Arnaldo Carvalho de Melo wrote:
>> Em Thu, Jul 24, 2014 at 02:57:51PM +0200, Jiri Olsa escreveu:
>> > On Wed, Jul 09, 2014 at 02:28:12PM +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.
>> >
>> > SNIP
>> >
>> > > --- a/tools/perf/util/sort.c
>> > > +++ b/tools/perf/util/sort.c
>> > > @@ -70,7 +70,9 @@ 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,
>> > > +
>> > > + width = max(7U, width) - 6;
>> > > + return repsep_snprintf(bf, size, "%-*.*s:%5d", width, width,
>> > >                          comm ?: "", he->thread->tid);
>> > >  }
>> >
>> > also we now print out all sort headers left alligned, which is wrong for
>> > sort_thread header: "Command:  Pid' that mirrors value strings with ':'
>> > being aligned
>> >
>> > maybe something like in attached patch..?
>> >
>> > thanks,
>> > jirka
>> >
>> >
>> > ---
>> > diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
>> > index f89714329c0f..fbbe86b7f63c 100644
>> > --- a/tools/perf/util/sort.c
>> > +++ b/tools/perf/util/sort.c
>> > @@ -78,6 +78,7 @@ static int hist_entry__thread_snprintf(struct hist_entry *he, char *bf,
>> >
>> >  struct sort_entry sort_thread = {
>> >     .se_header      = "Command:  Pid",
>> > +   .se_header_right= true,
>>
>> Is this just for the header? Perhaps its more clearly named .se_right_justify?
>
> yep, it's just for header.. let's see Namhyung might have better
> idea how to handle this problem

I think it still needs to be left-aligned.  So I'd rather change the
order - i.e. "Pid: Command".  Is it okay for you?

Thanks,
Namhyung

Re: [PATCHSET 0/6] perf tools: Honor column width setting

[Namhyung Kim]

On Thu, Jul 24, 2014 at 10:59 PM, Arnaldo Carvalho de Melo
<acme@kernel.org> wrote:
> Em Wed, Jul 09, 2014 at 02:28:08PM +0900, Namhyung Kim escreveu:
>> 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.
>
> Due to the issues pointed out, will wait for a v2 patchset, ok?

Okay.  Will send v2 tomorrow.

Thanks,
Namhyung

Re: [PATCH 3/6] perf tools: Save column length in perf_hpp_fmt

[Namhyung Kim]

On Thu, Jul 24, 2014 at 9:10 PM, Jiri Olsa <jolsa@redhat.com> wrote:
> On Wed, Jul 09, 2014 at 02:28:11PM +0900, Namhyung Kim wrote:
>> 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.
>
> SNIP
>
>> +
>> +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;
>
> just curious.. why is it 9 for guest %, while we use 8 for host?
> I understand that was the current state

It's the strlen of header - "guest sys". :)

Thanks,
Namhyung

Re: [PATCH 4/6] perf report: Honor column width setting

[Namhyung Kim]

On Thu, Jul 24, 2014 at 9:47 PM, Jiri Olsa <jolsa@redhat.com> wrote:
> On Wed, Jul 09, 2014 at 02:28:12PM +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.
>
> SNIP
>
>>
>>  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 396b0094943d..f89714329c0f 100644
>> --- a/tools/perf/util/sort.c
>> +++ b/tools/perf/util/sort.c
>> @@ -70,7 +70,9 @@ 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,
>> +
>> +     width = max(7U, width) - 6;
>> +     return repsep_snprintf(bf, size, "%-*.*s:%5d", width, width,
>>                              comm ?: "", he->thread->tid);
>
> I couldn't found what your format string:
>   "%-*.*s:%5d", width, width
>
> is supposed to do... it gives me the same result as for:
>   "%-*s:%5d", width
>
> you use it on more places so I guess it's not a typo ;-)

Yep, it's NOT a typo.  The second * is necessary for limiting maximum
length of printed string - without it we cannot control column
alignment of long symbols.

Thanks,
Namhyung

Re: [PATCH 3/6] perf tools: Save column length in perf_hpp_fmt

[Jiri Olsa]

On Fri, Jul 25, 2014 at 12:46:25AM +0900, Namhyung Kim wrote:
> On Thu, Jul 24, 2014 at 9:10 PM, Jiri Olsa <jolsa@redhat.com> wrote:
> > On Wed, Jul 09, 2014 at 02:28:11PM +0900, Namhyung Kim wrote:
> >> 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.
> >
> > SNIP
> >
> >> +
> >> +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;
> >
> > just curious.. why is it 9 for guest %, while we use 8 for host?
> > I understand that was the current state
> 
> It's the strlen of header - "guest sys". :)

argh.. I did not think of header, just value ;-)

thanks,
jirka

Re: [PATCH 4/6] perf report: Honor column width setting

[Jiri Olsa]

On Fri, Jul 25, 2014 at 12:41:15AM +0900, Namhyung Kim wrote:
> Hi guys,
> 
> On Thu, Jul 24, 2014 at 11:49 PM, Jiri Olsa <jolsa@redhat.com> wrote:
> > On Thu, Jul 24, 2014 at 10:57:47AM -0300, Arnaldo Carvalho de Melo wrote:
> >> Em Thu, Jul 24, 2014 at 02:57:51PM +0200, Jiri Olsa escreveu:
> >> > On Wed, Jul 09, 2014 at 02:28:12PM +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.
> >> >
> >> > SNIP
> >> >
> >> > > --- a/tools/perf/util/sort.c
> >> > > +++ b/tools/perf/util/sort.c
> >> > > @@ -70,7 +70,9 @@ 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,
> >> > > +
> >> > > + width = max(7U, width) - 6;
> >> > > + return repsep_snprintf(bf, size, "%-*.*s:%5d", width, width,
> >> > >                          comm ?: "", he->thread->tid);
> >> > >  }
> >> >
> >> > also we now print out all sort headers left alligned, which is wrong for
> >> > sort_thread header: "Command:  Pid' that mirrors value strings with ':'
> >> > being aligned
> >> >
> >> > maybe something like in attached patch..?
> >> >
> >> > thanks,
> >> > jirka
> >> >
> >> >
> >> > ---
> >> > diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
> >> > index f89714329c0f..fbbe86b7f63c 100644
> >> > --- a/tools/perf/util/sort.c
> >> > +++ b/tools/perf/util/sort.c
> >> > @@ -78,6 +78,7 @@ static int hist_entry__thread_snprintf(struct hist_entry *he, char *bf,
> >> >
> >> >  struct sort_entry sort_thread = {
> >> >     .se_header      = "Command:  Pid",
> >> > +   .se_header_right= true,
> >>
> >> Is this just for the header? Perhaps its more clearly named .se_right_justify?
> >
> > yep, it's just for header.. let's see Namhyung might have better
> > idea how to handle this problem
> 
> I think it still needs to be left-aligned.  So I'd rather change the
> order - i.e. "Pid: Command".  Is it okay for you?

yes, jirka

Re: [PATCH 4/6] perf report: Honor column width setting

[Arnaldo Carvalho de Melo]

Em Thu, Jul 24, 2014 at 05:58:44PM +0200, Jiri Olsa escreveu:
> On Fri, Jul 25, 2014 at 12:41:15AM +0900, Namhyung Kim wrote:
> > On Thu, Jul 24, 2014 at 11:49 PM, Jiri Olsa <jolsa@redhat.com> wrote:
> > > On Thu, Jul 24, 2014 at 10:57:47AM -0300, Arnaldo Carvalho de Melo wrote:
> > >> Em Thu, Jul 24, 2014 at 02:57:51PM +0200, Jiri Olsa escreveu:
> > >> >  struct sort_entry sort_thread = {
> > >> >     .se_header      = "Command:  Pid",
> > >> > +   .se_header_right= true,

> > >> Is this just for the header? Perhaps its more clearly named .se_right_justify?

> > > yep, it's just for header.. let's see Namhyung might have better
> > > idea how to handle this problem

> > I think it still needs to be left-aligned.  So I'd rather change the

Good, I like simpler fixes :)

> > order - i.e. "Pid: Command".  Is it okay for you?
 
> yes, jirka

Yes, its is more usual to have pid+command anyway, just run 'ps' :-)

- Arnaldo