perf/urgent hists hierarchy fixes

Hi Ingo,

Please consider pulling these 'perf report/top --hierarchy' fixes into
perf/urgent, they don't fix segfaults, just some oddities in scrolling, etc.

If you think it is too late in the 4.9-rc window, then please pull it
instead into perf/core, I did the build/test/container tests for both branches.

Thanks,

- Arnaldo

Test results (for perf/urgent + this) at the end.

The following changes since commit f92b7604149a55cb601fc0b52911b1e11f0f2514:

perf/x86/intel: Honour the CPUID for number of fixed counters in hypervisors (2016-10-28 11:06:25 +0200)

are available in the git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git tags/perf-hists-hierarchy-fixes-for-mingo-20161111

for you to fetch changes up to c72ab446cac1d6c9551fd26c4cfef1b2fc5041fd:

perf hists: Fix column length on --hierarchy (2016-11-09 11:55:29 -0300)

----------------------------------------------------------------
perf/urgent fixes for perf {top,report} --hierarchy

- These are fixes for the --hierarchy view of perf top and report, fixing output oddities, mostly related to scrolling. (Namhyung Kim)

----------------------------------------------------------------
Namhyung Kim (5):
perf hist browser: Fix hierarchy column counts
perf hists browser: Fix indentation of folded sign on --hierarchy perf hists browser: Show folded sign properly on --hierarchy perf hists browser: Fix column indentation on --hierarchy perf hists: Fix column length on --hierarchy

 tools/perf/ui/browsers/hists.c | 48 ++++++++++++++++++++++++++++++++----------
 tools/perf/util/hist.c         | 12 +++++------
 2 files changed, 43 insertions(+), 17 deletions(-)

  # perf test
   1: vmlinux symtab matches kallsyms                          : Ok
   2: detect openat syscall event                              : Ok
   3: detect openat syscall event on all cpus                  : Ok
   4: read samples using the mmap interface                    : Ok
   5: parse events tests                                       : Ok
   6: Validate PERF_RECORD_* events & perf_sample fields       : Ok
   7: Test perf pmu format parsing                             : Ok
   8: Test dso data read                                       : Ok
   9: Test dso data cache                                      : Ok
  10: Test dso data reopen                                     : Ok
  11: roundtrip evsel->name check                              : Ok
  12: Check parsing of sched tracepoints fields                : Ok
  13: Generate and check syscalls:sys_enter_openat event fields: Ok
  14: struct perf_event_attr setup                             : Ok
  15: Test matching and linking multiple hists                 : Ok
  16: Try 'import perf' in python, checking link problems      : Ok
  17: Test breakpoint overflow signal handler                  : Ok
  18: Test breakpoint overflow sampling                        : Ok
  19: Test number of exit event of a simple workload           : Ok
  20: Test software clock events have valid period values      : Ok
  21: Test object code reading                                 : Ok
  22: Test sample parsing                                      : Ok
  23: Test using a dummy software event to keep tracking       : Ok
  24: Test parsing with no sample_id_all bit set               : Ok
  25: Test filtering hist entries                              : Ok
  26: Test mmap thread lookup                                  : Ok
  27: Test thread mg sharing                                   : Ok
  28: Test output sorting of hist entries                      : Ok
  29: Test cumulation of child hist entries                    : Ok
  30: Test tracking with sched_switch                          : Ok
  31: Filter fds with revents mask in a fdarray                : Ok
  32: Add fd to a fdarray, making it autogrow                  : Ok
  33: Test kmod_path__parse function                           : Ok
  34: Test thread map                                          : Ok
  35: Test LLVM searching and compiling                        :
  35.1: Basic BPF llvm compiling test                          : Ok
  35.2: Test kbuild searching                                  : Ok
  35.3: Compile source for BPF prologue generation test        : Ok
  35.4: Compile source for BPF relocation test                 : Ok
  36: Test topology in session                                 : Ok
  37: Test BPF filter                                          :
  37.1: Test basic BPF filtering                               : Ok
  37.2: Test BPF prologue generation                           : Ok
  37.3: Test BPF relocation checker                            : Ok
  38: Test thread map synthesize                               : Ok
  39: Test cpu map synthesize                                  : Ok
  40: Test stat config synthesize                              : Ok
  41: Test stat synthesize                                     : Ok
  42: Test stat round synthesize                               : Ok
  43: Test attr update synthesize                              : Ok
  44: Test events times                                        : Ok
  45: Test backward reading from ring buffer                   : Ok
  46: Test cpu map print                                       : Ok
  47: Test SDT event probing                                   : Ok
  48: Test is_printable_array function                         : Ok
  49: Test bitmap print                                        : Ok
  50: x86 rdpmc test                                           : Ok
  51: Test converting perf time to TSC                         : Ok
  52: Test dwarf unwind                                        : Ok
  53: Test x86 instruction decoder - new instructions          : Ok
  54: Test intel cqm nmi context read                          : Skip
  # 

  # time dm
   1 alpine:3.4: Ok
   2 android-ndk:r12b-arm: Ok
   3 archlinux:latest: Ok
   4 centos:5: Ok
   5 centos:6: Ok
   6 centos:7: Ok
   7 debian:7: Ok
   8 debian:8: Ok
   9 debian:experimental: Ok
  10 fedora:20: Ok
  11 fedora:21: Ok
  12 fedora:22: Ok
  13 fedora:23: Ok
  14 fedora:24: Ok
  15 fedora:24-x-ARC-uClibc: Ok
  16 fedora:rawhide: Ok
  17 mageia:5: Ok
  18 opensuse:13.2: Ok
  19 opensuse:42.1: Ok
  20 opensuse:tumbleweed: Ok
  21 ubuntu:12.04.5: Ok
  22 ubuntu:14.04.4: Ok
  23 ubuntu:14.04.4-x-linaro-arm64: Ok
  24 ubuntu:15.10: Ok
  25 ubuntu:16.04: Ok
  26 ubuntu:16.04-x-arm: Ok
  27 ubuntu:16.04-x-arm64: Ok
  28 ubuntu:16.04-x-powerpc: Ok
  29 ubuntu:16.04-x-powerpc64: Ok
  30 ubuntu:16.04-x-powerpc64el: Ok
  31 ubuntu:16.04-x-s390: Ok
  32 ubuntu:16.10: Ok

real 35m13.612s
user 0m2.028s
sys 0m2.454s
#

  $ make -C tools/perf build-test
  make: Entering directory '/home/acme/git/linux/tools/perf'
  - tarpkg: ./tests/perf-targz-src-pkg .
                  make_no_ui_O: && make NO_NEWT=1 NO_SLANG=1 NO_GTK2=1
            make_no_libaudit_O: && make NO_LIBAUDIT=1
                make_no_newt_O: && make NO_NEWT=1
              make_clean_all_O: && make clean all
         make_install_prefix_O: && make install prefix=/tmp/krava
                   make_help_O: && make help
           make_no_libpython_O: && make NO_LIBPYTHON=1
             make_no_scripts_O: && make NO_LIBPYTHON=1 NO_LIBPERL=1
             make_no_libperl_O: && make NO_LIBPERL=1
           make_no_libunwind_O: && make NO_LIBUNWIND=1
            make_no_demangle_O: && make NO_DEMANGLE=1
                make_minimal_O: && make NO_LIBPERL=1 NO_LIBPYTHON=1 NO_NEWT=1 NO_GTK2=1 NO_DEMANGLE=1 NO_LIBELF=1 NO_LIBUNWIND=1 NO_BACKTRACE=1 NO_LIBNUMA=1 NO_LIBAUDIT=1 NO_LIBBIONIC=1 NO_LIBDW_DWARF_UNWIND=1 NO_AUXTRACE=1 NO_LIBBPF=1 NO_LIBCRYPTO=1 NO_SDT=1
                 make_perf_o_O: && make perf.o
  make_no_libdw_dwarf_unwind_O: && make NO_LIBDW_DWARF_UNWIND=1
   make_install_prefix_slash_O: && make install prefix=/tmp/krava/
              make_no_libelf_O: && make NO_LIBELF=1
            make_install_bin_O: && make install-bin
                    make_doc_O: && make doc
            make_no_auxtrace_O: && make NO_AUXTRACE=1
              make_no_libbpf_O: && make NO_LIBBPF=1
       make_util_pmu_bison_o_O: && make util/pmu-bison.o
                 make_static_O: && make LDFLAGS=-static
                make_install_O: && make install
             make_no_libnuma_O: && make NO_LIBNUMA=1
        make_with_babeltrace_O: && make LIBBABELTRACE=1
                   make_tags_O: && make tags
             make_util_map_o_O: && make util/map.o
           make_no_backtrace_O: && make NO_BACKTRACE=1
                make_no_gtk2_O: && make NO_GTK2=1
                  make_debug_O: && make DEBUG=1
               make_no_slang_O: && make NO_SLANG=1
           make_no_libbionic_O: && make NO_LIBBIONIC=1
                   make_pure_O: && make
  OK
  make: Leaving directory '/home/acme/git/linux/tools/perf'

From: Namhyung Kim <[email protected]>

The perf report/top on TUI supports horizontal scrolling using LEFT and RIGHT keys.

But it calculate the number of columns incorrectly when hierarchy mode is enabled so that keep pressing RIGHT key can make the output disappeared.

In the hierarchy mode, all sort keys are collapsed into a single column, so it needs to be applied when calculating column numbers.

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

index 4ffff7be9299..5adedc1a09d3 100644
@@ -2076,8 +2076,21 @@ 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)
+	if (symbol_conf.report_hierarchy) {
+		struct perf_hpp_list_node *fmt_node;
+
+		/* count overhead columns (in the first node) */
+		fmt_node = list_first_entry(&hists->hpp_formats,
+					    struct perf_hpp_list_node, list);
+		perf_hpp_list__for_each_format(&fmt_node->hpp, fmt)
+			++browser->b.columns;
+
+		/* add a single column for whole hierarchy sort keys*/
 		++browser->b.columns;
+	} else {
+		hists__for_each_format(hists, fmt)
+			++browser->b.columns;
+	}
 
 	hists__reset_column_width(hists);
 }

2.7.4

From: Namhyung Kim <[email protected]>

It should indent 2 spaces for folded sign and a whitespace.

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

index 5adedc1a09d3..225ef2a15a13 100644
@@ -1337,8 +1337,8 @@ static int hist_browser__show_hierarchy_entry(struct hist_browser *browser,
 		}
 
 		if (first) {
-			ui_browser__printf(&browser->b, "%c", folded_sign);
-			width--;
+			ui_browser__printf(&browser->b, "%c ", folded_sign);
+			width -= 2;
 			first = false;
 		} else {
 			ui_browser__printf(&browser->b, "  ");

@@ -1555,7 +1555,7 @@ static int hists_browser__scnprintf_hierarchy_headers(struct hist_browser *brows
 	int indent = hists->nr_hpp_node - 2;
 	bool first_node, first_col;
 
-	ret = scnprintf(buf, size, " ");
+	ret = scnprintf(buf, size, "  ");
 	if (advance_hpp_check(&dummy_hpp, ret))
 		return ret;
 

2.7.4

From: Namhyung Kim <[email protected]>

When horizontal scrolling is used in hierarchy mode, the folded signed disappears at the right most column.

Committer note:

To test it, run 'perf top --hierarchy, see the '+' symbol at the first column, then press the right arrow key, the '+' symbol will disappear, this patch fixes that.

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

index 225ef2a15a13..e767fbd17ad2 100644
@@ -1381,7 +1381,13 @@ static int hist_browser__show_hierarchy_entry(struct hist_browser *browser,
 		}
 
 		perf_hpp_list__for_each_format(entry->hpp_list, fmt) {
-			ui_browser__write_nstring(&browser->b, "", 2);
+			if (first) {
+				ui_browser__printf(&browser->b, "%c ", folded_sign);
+				first = false;
+			} else {
+				ui_browser__write_nstring(&browser->b, "", 2);
+			}
+
 			width -= 2;
 
 			/*

2.7.4

From: Namhyung Kim <[email protected]>

When horizontall scrolling is used in hierarchy mode, the the right most column has unnecessary indentation. Actually it's needed only if some of left (overhead) columns were shown.

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

index e767fbd17ad2..a53fef0c673b 100644
@@ -1361,8 +1361,10 @@ static int hist_browser__show_hierarchy_entry(struct hist_browser *browser,
 		width -= hpp.buf - s;
 	}
 
-	ui_browser__write_nstring(&browser->b, "", hierarchy_indent);
-	width -= hierarchy_indent;
+	if (!first) {
+		ui_browser__write_nstring(&browser->b, "", hierarchy_indent);
+		width -= hierarchy_indent;
+	}
 
 	if (column >= browser->b.horiz_scroll) {
 		char s[2048];

@@ -1565,6 +1567,7 @@ static int hists_browser__scnprintf_hierarchy_headers(struct hist_browser *brows
 	if (advance_hpp_check(&dummy_hpp, ret))
 		return ret;
 
+	first_node = true;
 	/* the first hpp_list_node is for overhead columns */
 	fmt_node = list_first_entry(&hists->hpp_formats,
 				    struct perf_hpp_list_node, list);

@@ -1579,12 +1582,16 @@ static int hists_browser__scnprintf_hierarchy_headers(struct hist_browser *brows
 		ret = scnprintf(dummy_hpp.buf, dummy_hpp.size, "  ");
 		if (advance_hpp_check(&dummy_hpp, ret))
 			break;
+
+		first_node = false;
 	}
 
-	ret = scnprintf(dummy_hpp.buf, dummy_hpp.size, "%*s",
-			indent * HIERARCHY_INDENT, "");
-	if (advance_hpp_check(&dummy_hpp, ret))
-		return ret;
+	if (!first_node) {
+		ret = scnprintf(dummy_hpp.buf, dummy_hpp.size, "%*s",
+				indent * HIERARCHY_INDENT, "");
+		if (advance_hpp_check(&dummy_hpp, ret))
+			return ret;
+	}
 
 	first_node = true;
 	list_for_each_entry_continue(fmt_node, &hists->hpp_formats, list) {

2.7.4

From: Namhyung Kim <[email protected]>

Markus reported that there's a weird behavior on perf top --hierarchy regarding the column length.

Looking at the code, I found a dubious code which affects the symptoms. When --hierarchy option is used, the last column length might be inaccurate since it skips to update the length on leaf entries.

I cannot remember why it did and looks like a leftover from previous version during the development.

Anyway, updating the column length often is not harmful. So let's move the code out.

---
 tools/perf/util/hist.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

index b02992efb513..a69f027368ef 100644
@@ -1600,18 +1600,18 @@ static void hists__hierarchy_output_resort(struct hists *hists,
 		if (prog)
 			ui_progress__update(prog, 1);
 
+		hists->nr_entries++;
+		if (!he->filtered) {
+			hists->nr_non_filtered_entries++;
+			hists__calc_col_len(hists, he);
+		}
+
 		if (!he->leaf) {
 			hists__hierarchy_output_resort(hists, prog,
 						       &he->hroot_in,
 						       &he->hroot_out,
 						       min_callchain_hits,
 						       use_callchain);
-			hists->nr_entries++;
-			if (!he->filtered) {
-				hists->nr_non_filtered_entries++;
-				hists__calc_col_len(hists, he);
-			}
-
 			continue;
 		}
 

2.7.4

Pulled, thanks a lot Arnaldo!

Ingo