perf ui/tui: Reset output width for hierarchy

PATCH In review Namhyung Kim 3 participants 7 messages Sep 20, 2016 perf events

Share & tools

Namhyung Kim

author maintainer

2016-09-20 05:30 UTC #0 lore raw

PATCH 1/3 perf ui/tui: Reset output width for hierarchy

When --hierarchy option is used, each entry has its own hpp_list to show the result. But it missed to update width of each column.

Before:

- 46.29% 48.12% netctl-auto
+ 31.44% 29.25% [kernel.vmlinux]
+ 8.52% 11.55% libc-2.22.so
+ 5.19% 6.91% bash
+ 10.75% 11.83% wpa_cli

  + 8.25% 2.23%        swapper
  + 6.45% 5.40%        tr
  + 4.81% 8.09%        awk
  + 4.15% 2.85%        firefox
  + 3.86% 2.53%        sh

After:

- 46.29% 48.12% netctl-auto

      +  31.44%  29.25%        [kernel.vmlinux]
      +   8.52%  11.55%        libc-2.22.so
      +   5.19%   6.91%        bash
  +  10.75%  11.83%        wpa_cli
  +   8.25%   2.23%        swapper
  +   6.45%   5.40%        tr
  +   4.81%   8.09%        awk
  +   4.15%   2.85%        firefox
  +   3.86%   2.53%        sh

Fixes 1b2dbbf41a0f ("perf hists: Use own hpp_list for hierarchy mode")

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

---
 tools/perf/ui/browsers/hists.c | 6 ++++++
 1 file changed, 6 insertions(+)

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

index 35e44b1879e3..49db16334814 100644
@@ -2067,6 +2067,7 @@ void hist_browser__init(struct hist_browser *browser,
 			struct hists *hists)
 {
 	struct perf_hpp_fmt *fmt;
+	struct perf_hpp_list_node *node;
 
 	browser->hists			= hists;
 	browser->b.refresh		= hist_browser__refresh;

@@ -2079,6 +2080,11 @@ void hist_browser__init(struct hist_browser *browser,
 		perf_hpp__reset_width(fmt, hists);
 		++browser->b.columns;
 	}
+	/* hierarchy entries have their own hpp list */
+	list_for_each_entry(node, &hists->hpp_formats, list) {
+		perf_hpp_list__for_each_format(&node->hpp, fmt)
+			perf_hpp__reset_width(fmt, hists);
+	}
 }
 
 struct hist_browser *hist_browser__new(struct hists *hists)

Signature · 1 line

2.9.3

Namhyung Kim

author maintainer

2016-09-20 05:30 UTC #1 lore raw

PATCH 2/3 perf hists: Factor out hists__reset_column_width()

The stdio and tui has same code to reset hpp format column width. Factor it out as a new function.

Suggested-by Jiri Olsa <[email protected]>

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

---
 tools/perf/ui/browsers/hists.c | 12 +++---------
 tools/perf/ui/hist.c           | 15 +++++++++++++++
 tools/perf/ui/stdio/hist.c     | 10 +---------
 tools/perf/util/hist.h         |  1 +
 4 files changed, 20 insertions(+), 18 deletions(-)

tools/perf/ui/browsers/hists.c +3 -9

index 49db16334814..a6d5d248b8fb 100644
@@ -2067,7 +2067,6 @@ void hist_browser__init(struct hist_browser *browser,
 			struct hists *hists)
 {
 	struct perf_hpp_fmt *fmt;
-	struct perf_hpp_list_node *node;
 
 	browser->hists			= hists;
 	browser->b.refresh		= hist_browser__refresh;

@@ -2076,15 +2075,10 @@ void hist_browser__init(struct hist_browser *browser,
 	browser->b.use_navkeypressed	= true;
 	browser->show_headers		= symbol_conf.show_hist_headers;
 
-	hists__for_each_format(hists, fmt) {
-		perf_hpp__reset_width(fmt, hists);
+	hists__for_each_format(hists, fmt)
 		++browser->b.columns;
-	}
-	/* hierarchy entries have their own hpp list */
-	list_for_each_entry(node, &hists->hpp_formats, list) {
-		perf_hpp_list__for_each_format(&node->hpp, fmt)
-			perf_hpp__reset_width(fmt, hists);
-	}
+
+	hists__reset_column_width(hists);
 }
 
 struct hist_browser *hist_browser__new(struct hists *hists)

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

index b47fafc8ee2a..60c4a4d08374 100644
@@ -699,6 +699,21 @@ void perf_hpp__reset_width(struct perf_hpp_fmt *fmt, struct hists *hists)
 	}
 }
 
+void hists__reset_column_width(struct hists *hists)
+{
+	struct perf_hpp_fmt *fmt;
+	struct perf_hpp_list_node *node;
+
+	hists__for_each_format(hists, fmt)
+		perf_hpp__reset_width(fmt, hists);
+
+	/* hierarchy entries have their own hpp list */
+	list_for_each_entry(node, &hists->hpp_formats, list) {
+		perf_hpp_list__for_each_format(&node->hpp, fmt)
+			perf_hpp__reset_width(fmt, hists);
+	}
+}
+
 void perf_hpp__set_user_width(const char *width_list_str)
 {
 	struct perf_hpp_fmt *fmt;

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

index a57131e61fe3..8e1840bff29d 100644
@@ -717,8 +717,6 @@ size_t hists__fprintf(struct hists *hists, bool show_header, int max_rows,
 		      int max_cols, float min_pcnt, FILE *fp,
 		      bool use_callchain)
 {
-	struct perf_hpp_fmt *fmt;
-	struct perf_hpp_list_node *node;
 	struct rb_node *nd;
 	size_t ret = 0;
 	const char *sep = symbol_conf.field_sep;

@@ -729,13 +727,7 @@ size_t hists__fprintf(struct hists *hists, bool show_header, int max_rows,
 
 	init_rem_hits();
 
-	hists__for_each_format(hists, fmt)
-		perf_hpp__reset_width(fmt, hists);
-	/* hierarchy entries have their own hpp list */
-	list_for_each_entry(node, &hists->hpp_formats, list) {
-		perf_hpp_list__for_each_format(&node->hpp, fmt)
-			perf_hpp__reset_width(fmt, hists);
-	}
+	hists__reset_column_width(hists);
 
 	if (symbol_conf.col_width_list_str)
 		perf_hpp__set_user_width(symbol_conf.col_width_list_str);

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

index a002c93fe422..defa957f27df 100644
@@ -368,6 +368,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);
 void perf_hpp__set_user_width(const char *width_list_str);
+void hists__reset_column_width(struct hists *hists);
 
 typedef u64 (*hpp_field_fn)(struct hist_entry *he);
 typedef int (*hpp_callback_fn)(struct perf_hpp *hpp, bool front);

Signature · 1 line

2.9.3

Jiri Olsa

Acked-by on v1 2/3

2016-09-20 07:07 UTC #2 lore raw

Re: [PATCH 2/3] perf hists: Factor out hists__reset_column_width()

Quoted 5 lines · Namhyung Kim

The stdio and tui has same code to reset hpp format column width. Factor it out as a new function.
Suggested-by Jiri Olsa <[email protected]>
Signed-off-by Namhyung Kim <[email protected]>

Acked-by Jiri Olsa <[email protected]>

thanks,
jirka

tip-bot for Namhyung Kim

merge bot

2016-09-20 21:46 UTC #3 lore raw

[tip:perf/core] perf hists: Factor out hists__reset_column_width()

In reply to Namhyung Kim

perf hists: Factor out hists__reset_column_width()

Commit-ID:  e3b60bc93d81e0542ac433df226b8de8b963533e
Gitweb:     http://git.kernel.org/tip/e3b60bc93d81e0542ac433df226b8de8b963533e
Author:     Namhyung Kim <[email protected]>
AuthorDate: Tue, 20 Sep 2016 14:30:24 +0900
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 20 Sep 2016 16:13:37 -0300

perf hists: Factor out hists__reset_column_width()

The stdio and tui has same code to reset hpp format column width. Factor it out as a new function.

Suggested-and-Acked-by Jiri Olsa <[email protected]>

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

Cc Andi Kleen <[email protected]>

Cc Peter Zijlstra <[email protected]>

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

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

---
 tools/perf/ui/browsers/hists.c | 12 +++---------
 tools/perf/ui/hist.c           | 15 +++++++++++++++
 tools/perf/ui/stdio/hist.c     | 10 +---------
 tools/perf/util/hist.h         |  1 +
 4 files changed, 20 insertions(+), 18 deletions(-)

tools/perf/ui/browsers/hists.c +3 -9

index 49db163..a6d5d24 100644
@@ -2067,7 +2067,6 @@ void hist_browser__init(struct hist_browser *browser,
 			struct hists *hists)
 {
 	struct perf_hpp_fmt *fmt;
-	struct perf_hpp_list_node *node;
 
 	browser->hists			= hists;
 	browser->b.refresh		= hist_browser__refresh;

@@ -2076,15 +2075,10 @@ void hist_browser__init(struct hist_browser *browser,
 	browser->b.use_navkeypressed	= true;
 	browser->show_headers		= symbol_conf.show_hist_headers;
 
-	hists__for_each_format(hists, fmt) {
-		perf_hpp__reset_width(fmt, hists);
+	hists__for_each_format(hists, fmt)
 		++browser->b.columns;
-	}
-	/* hierarchy entries have their own hpp list */
-	list_for_each_entry(node, &hists->hpp_formats, list) {
-		perf_hpp_list__for_each_format(&node->hpp, fmt)
-			perf_hpp__reset_width(fmt, hists);
-	}
+
+	hists__reset_column_width(hists);
 }
 
 struct hist_browser *hist_browser__new(struct hists *hists)

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

index b47fafc..60c4a4d 100644
@@ -699,6 +699,21 @@ void perf_hpp__reset_width(struct perf_hpp_fmt *fmt, struct hists *hists)
 	}
 }
 
+void hists__reset_column_width(struct hists *hists)
+{
+	struct perf_hpp_fmt *fmt;
+	struct perf_hpp_list_node *node;
+
+	hists__for_each_format(hists, fmt)
+		perf_hpp__reset_width(fmt, hists);
+
+	/* hierarchy entries have their own hpp list */
+	list_for_each_entry(node, &hists->hpp_formats, list) {
+		perf_hpp_list__for_each_format(&node->hpp, fmt)
+			perf_hpp__reset_width(fmt, hists);
+	}
+}
+
 void perf_hpp__set_user_width(const char *width_list_str)
 {
 	struct perf_hpp_fmt *fmt;

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

index a57131e..8e1840b 100644
@@ -717,8 +717,6 @@ size_t hists__fprintf(struct hists *hists, bool show_header, int max_rows,
 		      int max_cols, float min_pcnt, FILE *fp,
 		      bool use_callchain)
 {
-	struct perf_hpp_fmt *fmt;
-	struct perf_hpp_list_node *node;
 	struct rb_node *nd;
 	size_t ret = 0;
 	const char *sep = symbol_conf.field_sep;

@@ -729,13 +727,7 @@ size_t hists__fprintf(struct hists *hists, bool show_header, int max_rows,
 
 	init_rem_hits();
 
-	hists__for_each_format(hists, fmt)
-		perf_hpp__reset_width(fmt, hists);
-	/* hierarchy entries have their own hpp list */
-	list_for_each_entry(node, &hists->hpp_formats, list) {
-		perf_hpp_list__for_each_format(&node->hpp, fmt)
-			perf_hpp__reset_width(fmt, hists);
-	}
+	hists__reset_column_width(hists);
 
 	if (symbol_conf.col_width_list_str)
 		perf_hpp__set_user_width(symbol_conf.col_width_list_str);

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

index a002c93..defa957 100644
@@ -368,6 +368,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);
 void perf_hpp__set_user_width(const char *width_list_str);
+void hists__reset_column_width(struct hists *hists);
 
 typedef u64 (*hpp_field_fn)(struct hist_entry *he);
 typedef int (*hpp_callback_fn)(struct perf_hpp *hpp, bool front);

Namhyung Kim

author maintainer

2016-09-20 05:30 UTC #4 lore raw

PATCH 3/3 perf report: Fix output of 'pid' sort key

In reply to Namhyung Kim

The thread->comm can be changed during the lifetime due to prctl() or exec(). For this reason each hist entry has a pointer to a comm at the time as well as a pointer to the thread. So it should use the he->comm instead of thread__comm(he->thread) which always returns the latest comm. This can be seen using following example:

  $ perf report --hierarchy -s comm,pid

Before:

    3.86%   2.53%     sh
       1.05%   0.45%         776:sh
       0.67%   0.49%        8190:sh
       0.65%   0.37%        8194:sh
       0.28%   0.21%        8192:awk
       0.24%   0.22%        8191:acpi
       0.24%   0.21%        8196:awk
       0.22%   0.15%        8193:tr
       0.21%   0.14%        8195:netctl-auto
       0.10%   0.30%        8319:date
       0.10%   0.00%        8320:xsetroot
       0.09%   0.00%        8321:sleep

After:

    3.86%   2.53%     sh
       1.05%   0.45%         776:sh
       0.67%   0.49%        8190:sh
       0.65%   0.37%        8194:sh
       0.28%   0.21%        8192:sh
       0.24%   0.22%        8191:sh
       0.24%   0.21%        8196:sh
       0.22%   0.15%        8193:sh
       0.21%   0.14%        8195:sh
       0.10%   0.30%        8319:sh
       0.10%   0.00%        8320:sh
       0.09%   0.00%        8321:sh

Cc Frederic Weisbecker <[email protected]>

Fixes 4dfced359fbc ("perf tools: Get current comm instead of last one")

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

---
 tools/perf/util/sort.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

tools/perf/util/sort.c +1 -1

index 1884d7f9b9d2..549dfbdbe9c3 100644
@@ -77,7 +77,7 @@ sort__thread_cmp(struct hist_entry *left, struct hist_entry *right)
 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);
+	const char *comm = comm__str(he->comm);
 
 	width = max(7U, width) - 8;
 	return repsep_snprintf(bf, size, "%7d:%-*.*s", he->thread->tid,

Signature · 1 line

2.9.3

Jiri Olsa

Acked-by on v1 1/3

2016-09-20 07:07 UTC #5 lore raw

In reply to Namhyung Kim

Quoted 29 lines · Namhyung Kim

When --hierarchy option is used, each entry has its own hpp_list to show the result. But it missed to update width of each column.
Before:
- 46.29% 48.12% netctl-auto
+ 31.44% 29.25% [kernel.vmlinux]
+ 8.52% 11.55% libc-2.22.so
+ 5.19% 6.91% bash
+ 10.75% 11.83% wpa_cli
  + 8.25% 2.23%        swapper
  + 6.45% 5.40%        tr
  + 4.81% 8.09%        awk
  + 4.15% 2.85%        firefox
  + 3.86% 2.53%        sh
After:
- 46.29% 48.12% netctl-auto
      +  31.44%  29.25%        [kernel.vmlinux]
      +   8.52%  11.55%        libc-2.22.so
      +   5.19%   6.91%        bash
  +  10.75%  11.83%        wpa_cli
  +   8.25%   2.23%        swapper
  +   6.45%   5.40%        tr
  +   4.81%   8.09%        awk
  +   4.15%   2.85%        firefox
  +   3.86%   2.53%        sh

Quoted 2 lines

Fixes 1b2dbbf41a0f ("perf hists: Use own hpp_list for hierarchy mode")
Signed-off-by Namhyung Kim <[email protected]>

Acked-by Jiri Olsa <[email protected]>

thanks,
jirka

Quoted 31 lines

---
 tools/perf/ui/browsers/hists.c | 6 ++++++
 1 file changed, 6 insertions(+)

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

index 35e44b1879e3..49db16334814 100644
@@ -2067,6 +2067,7 @@ void hist_browser__init(struct hist_browser *browser,
 			struct hists *hists)
 {
 	struct perf_hpp_fmt *fmt;
+	struct perf_hpp_list_node *node;
 
 	browser->hists			= hists;
 	browser->b.refresh		= hist_browser__refresh;

@@ -2079,6 +2080,11 @@ void hist_browser__init(struct hist_browser *browser,
 		perf_hpp__reset_width(fmt, hists);
 		++browser->b.columns;
 	}
+	/* hierarchy entries have their own hpp list */
+	list_for_each_entry(node, &hists->hpp_formats, list) {
+		perf_hpp_list__for_each_format(&node->hpp, fmt)
+			perf_hpp__reset_width(fmt, hists);
+	}
 }
 
 struct hist_browser *hist_browser__new(struct hists *hists)

Signature · 1 line

2.9.3

tip-bot for Namhyung Kim

merge bot

2016-09-20 21:45 UTC #6 lore raw

In reply to Namhyung Kim

perf ui/tui: Reset output width for hierarchy

Commit-ID:  5ff3e7a224d40f9dd73625b91377787034a8b35e
Gitweb:     http://git.kernel.org/tip/5ff3e7a224d40f9dd73625b91377787034a8b35e
Author:     Namhyung Kim <[email protected]>
AuthorDate: Tue, 20 Sep 2016 14:30:23 +0900
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Tue, 20 Sep 2016 16:08:30 -0300

perf ui/tui: Reset output width for hierarchy

When --hierarchy option is used, each entry has its own hpp_list to show the result. But it missed to update width of each column.

Before:

- 46.29% 48.12% netctl-auto
+ 31.44% 29.25% [kernel.vmlinux]
+ 8.52% 11.55% libc-2.22.so
+ 5.19% 6.91% bash
+ 10.75% 11.83% wpa_cli

  + 8.25% 2.23%        swapper
  + 6.45% 5.40%        tr
  + 4.81% 8.09%        awk
  + 4.15% 2.85%        firefox
  + 3.86% 2.53%        sh

After:

- 46.29% 48.12% netctl-auto

      +  31.44%  29.25%        [kernel.vmlinux]
      +   8.52%  11.55%        libc-2.22.so
      +   5.19%   6.91%        bash
  +  10.75%  11.83%        wpa_cli
  +   8.25%   2.23%        swapper
  +   6.45%   5.40%        tr
  +   4.81%   8.09%        awk
  +   4.15%   2.85%        firefox
  +   3.86%   2.53%        sh

Committer note:

Full testing instructions:

1) Record with an event group:

  $ perf record -e '{cycles,instructions}' make -j4

2) Use report in hierarchy mode, to get a few expanded trees on the same screen, use --percent-limit:

  $ perf report --hierarchy --percent-limit 0.5

Samples: 103K of event 'anon group { cycles:u, instructions:u }', Event count (approx.): 57317631725

         Overhead        Command / Shared Object / Symbol        ◆
-  58.89%  55.12%        cc1                                     ▒
   -  50.26%  48.10%        cc1                                  ▒
          3.61%   5.13%        [.] _cpp_lex_token                ▒
          2.58%   0.78%        [.] ht_lookup_with_hash           ▒
          1.31%   1.30%        [.] ggc_internal_alloc            ▒
          1.08%   2.25%        [.] get_combined_adhoc_loc        ▒
          1.01%   1.95%        [.] ira_init                      ▒
          0.96%   1.78%        [.] linemap_position_for_column   ▒
          0.65%   1.01%        [.] cpp_get_token_with_location   ▒
   -   7.52%   6.58%        libc-2.23.so                         ▒
          1.70%   1.78%        [.] _int_malloc                   ▒
          0.69%   0.75%        [.] _int_free                     ▒
          0.67%   0.42%        [.] malloc_consolidate            ▒
   -   0.58%   0.42%        ld-2.23.so                           ▒
                               no entry >= 0.50%                 ▒
   -   0.52%   0.03%        [kernel.vmlinux]                     ▒
                               no entry >= 0.50%                 ▒

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

Acked-by Jiri Olsa <[email protected]>

Cc Andi Kleen <[email protected]>

Cc Peter Zijlstra <[email protected]>

Fixes 1b2dbbf41a0f ("perf hists: Use own hpp_list for hierarchy mode")

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

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

---
 tools/perf/ui/browsers/hists.c | 6 ++++++
 1 file changed, 6 insertions(+)

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

index 35e44b1..49db163 100644
@@ -2067,6 +2067,7 @@ void hist_browser__init(struct hist_browser *browser,
 			struct hists *hists)
 {
 	struct perf_hpp_fmt *fmt;
+	struct perf_hpp_list_node *node;
 
 	browser->hists			= hists;
 	browser->b.refresh		= hist_browser__refresh;

@@ -2079,6 +2080,11 @@ void hist_browser__init(struct hist_browser *browser,
 		perf_hpp__reset_width(fmt, hists);
 		++browser->b.columns;
 	}
+	/* hierarchy entries have their own hpp list */
+	list_for_each_entry(node, &hists->hpp_formats, list) {
+		perf_hpp_list__for_each_format(&node->hpp, fmt)
+			perf_hpp__reset_width(fmt, hists);
+	}
 }
 
 struct hist_browser *hist_browser__new(struct hists *hists)