* [PATCH 1/5] perf hist browser: Fix hierarchy column counts
2016-11-11 15:52 [GIT PULL 0/5] perf/urgent hists hierarchy fixes Arnaldo Carvalho de Melo
@ 2016-11-11 15:52 ` Arnaldo Carvalho de Melo
2016-11-11 15:52 ` [PATCH 2/5] perf hists browser: Fix indentation of folded sign on --hierarchy Arnaldo Carvalho de Melo
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-11-11 15:52 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Namhyung Kim, Jiri Olsa, Peter Zijlstra,
Arnaldo Carvalho de Melo
From: Namhyung Kim <namhyung@kernel.org>
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.
Reported-and-Tested-by: Markus Trippelsdorf <markus@trippelsdorf.de>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20161024162110.17918-1-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/ui/browsers/hists.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 4ffff7be9299..5adedc1a09d3 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/5] perf hists browser: Fix indentation of folded sign on --hierarchy
2016-11-11 15:52 [GIT PULL 0/5] perf/urgent hists hierarchy fixes Arnaldo Carvalho de Melo
2016-11-11 15:52 ` [PATCH 1/5] perf hist browser: Fix hierarchy column counts Arnaldo Carvalho de Melo
@ 2016-11-11 15:52 ` Arnaldo Carvalho de Melo
2016-11-11 15:52 ` [PATCH 3/5] perf hists browser: Show folded sign properly " Arnaldo Carvalho de Melo
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-11-11 15:52 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Namhyung Kim, Jiri Olsa, Peter Zijlstra,
Arnaldo Carvalho de Melo
From: Namhyung Kim <namhyung@kernel.org>
It should indent 2 spaces for folded sign and a whitespace.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Tested-by: Markus Trippelsdorf <markus@trippelsdorf.de>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20161108130833.9263-2-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/ui/browsers/hists.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 5adedc1a09d3..225ef2a15a13 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/5] perf hists browser: Show folded sign properly on --hierarchy
2016-11-11 15:52 [GIT PULL 0/5] perf/urgent hists hierarchy fixes Arnaldo Carvalho de Melo
2016-11-11 15:52 ` [PATCH 1/5] perf hist browser: Fix hierarchy column counts Arnaldo Carvalho de Melo
2016-11-11 15:52 ` [PATCH 2/5] perf hists browser: Fix indentation of folded sign on --hierarchy Arnaldo Carvalho de Melo
@ 2016-11-11 15:52 ` Arnaldo Carvalho de Melo
2016-11-11 15:53 ` [PATCH 4/5] perf hists browser: Fix column indentation " Arnaldo Carvalho de Melo
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-11-11 15:52 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Namhyung Kim, Jiri Olsa, Peter Zijlstra,
Arnaldo Carvalho de Melo
From: Namhyung Kim <namhyung@kernel.org>
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.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Tested-by: Markus Trippelsdorf <markus@trippelsdorf.de>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20161108130833.9263-3-namhyung@kernel.org
[ Move 'width -= 2' invariant to right after the if/else ]
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/ui/browsers/hists.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index 225ef2a15a13..e767fbd17ad2 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 4/5] perf hists browser: Fix column indentation on --hierarchy
2016-11-11 15:52 [GIT PULL 0/5] perf/urgent hists hierarchy fixes Arnaldo Carvalho de Melo
` (2 preceding siblings ...)
2016-11-11 15:52 ` [PATCH 3/5] perf hists browser: Show folded sign properly " Arnaldo Carvalho de Melo
@ 2016-11-11 15:53 ` Arnaldo Carvalho de Melo
2016-11-11 15:53 ` [PATCH 5/5] perf hists: Fix column length " Arnaldo Carvalho de Melo
2016-11-12 10:49 ` [GIT PULL 0/5] perf/urgent hists hierarchy fixes Ingo Molnar
5 siblings, 0 replies; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-11-11 15:53 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Namhyung Kim, Jiri Olsa, Peter Zijlstra,
Arnaldo Carvalho de Melo
From: Namhyung Kim <namhyung@kernel.org>
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.
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Tested-by: Markus Trippelsdorf <markus@trippelsdorf.de>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/20161108130833.9263-4-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/ui/browsers/hists.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index e767fbd17ad2..a53fef0c673b 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 5/5] perf hists: Fix column length on --hierarchy
2016-11-11 15:52 [GIT PULL 0/5] perf/urgent hists hierarchy fixes Arnaldo Carvalho de Melo
` (3 preceding siblings ...)
2016-11-11 15:53 ` [PATCH 4/5] perf hists browser: Fix column indentation " Arnaldo Carvalho de Melo
@ 2016-11-11 15:53 ` Arnaldo Carvalho de Melo
2016-11-12 10:49 ` [GIT PULL 0/5] perf/urgent hists hierarchy fixes Ingo Molnar
5 siblings, 0 replies; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-11-11 15:53 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Namhyung Kim, Jiri Olsa, Peter Zijlstra,
Arnaldo Carvalho de Melo
From: Namhyung Kim <namhyung@kernel.org>
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.
Reported-and-Tested-by: Markus Trippelsdorf <markus@trippelsdorf.de>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Fixes: 1a3906a7e6b9 ("perf hists: Resort hist entries with hierarchy")
Link: http://lkml.kernel.org/r/20161108130833.9263-5-namhyung@kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/hist.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index b02992efb513..a69f027368ef 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [GIT PULL 0/5] perf/urgent hists hierarchy fixes
2016-11-11 15:52 [GIT PULL 0/5] perf/urgent hists hierarchy fixes Arnaldo Carvalho de Melo
` (4 preceding siblings ...)
2016-11-11 15:53 ` [PATCH 5/5] perf hists: Fix column length " Arnaldo Carvalho de Melo
@ 2016-11-12 10:49 ` Ingo Molnar
5 siblings, 0 replies; 7+ messages in thread
From: Ingo Molnar @ 2016-11-12 10:49 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: linux-kernel, Jiri Olsa, Markus Trippelsdorf, Namhyung Kim,
Peter Zijlstra, Arnaldo Carvalho de Melo
* Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> 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)
>
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> ----------------------------------------------------------------
> 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(-)
Pulled, thanks a lot Arnaldo!
Ingo
^ permalink raw reply [flat|nested] 7+ messages in thread