Re: Scrolling down broken with "perf top --hierarchy"

[Namhyung Kim <namhyung@kernel.org>]

Hi,

Sorry for late reply.

On Mon, Oct 10, 2016 at 07:54:27PM +0200, Markus Trippelsdorf wrote:
> On 2016.10.08 at 13:21 +0200, Markus Trippelsdorf wrote:
> > On 2016.10.07 at 07:09 +0200, Markus Trippelsdorf wrote:
> > > On 2016.10.07 at 06:56 +0200, Markus Trippelsdorf wrote:
> > > > On 2016.10.07 at 06:32 +0200, Markus Trippelsdorf wrote:
> > > > > On 2016.10.07 at 13:22 +0900, Namhyung Kim wrote:
> > > > > > On Fri, Oct 07, 2016 at 05:51:18AM +0200, Markus Trippelsdorf wrote:
> > > > > > > On 2016.10.07 at 10:17 +0900, Namhyung Kim wrote:
> > > > > > > > On Thu, Oct 06, 2016 at 06:33:33PM +0200, Markus Trippelsdorf wrote:
> > > > > > > > > Scrolling down is broken when using "perf top --hierarchy".
> > > > > > > > > When it starts up everything is OK and one can scroll up and down to all
> > > > > > > > > entries. But as further and further new entries get added to the list,
> > > > > > > > > scrolling down is blocked (at the position of the last entry that was
> > > > > > > > > shown directly after startup).
> > > > > > > > 
> > > > > > > > I think below patch will fix the problem.  Please check.
> > > > > > > 
> > > > > > > Yes. It works fine now. Many thanks.
> > > > > > 
> > > > > > Good.  Can I add your Tested-by then?
> > > > > 
> > > > > Sure. 
> > > > 
> > > > And BTW symbols are currently always cut off at 60 characters in
> > > > expanded entries.
> > > 
> > > Hmm, no. Sometimes they are cut off, sometimes they are not. I haven't
> > > figured out what triggered this strange behavior.
> > 
> > Here is an example:
> > 
> >  % echo $COLUMNS
> >  179
> >  % perf top --hierarchy
> > +  34.81%     [kernel]
> > -  20.89%     chrome
> >        0.51%     [.] v8::internal::IncrementalMarking:
> >        0.43%     [.] tc_malloc
> >        0.29%     [.] sqlite3BtreeMovetoUnpacked
> >        0.28%     [.] tc_free
> >        0.24%     [.] v8::internal::BodyDescriptorBase:
> >        0.24%     [.] sqlite3VdbeExec
> >        0.22%     [.] v8::internal::MarkCompactCollecto
> >        0.19%     [.] blink::SelectorChecker::checkOne
> >        0.19%     [.] SkBlitRow::Color32
> >        0.18%     [.] SkBlitLCD16OpaqueRow_SSE2
> >        0.17%     [.] btreeInitPage.part.366
> >        0.16%     [.] blink::SelectorChecker::matchSele
> >        0.15%     [.] blink::ElementRuleCollector::coll
> >        0.15%     [.] blink::CSSTokenizer::consumeName
> >        0.14%     [.] sqlite3GetVarint
> >        0.13%     [.] operator new[]
> >        0.12%     [.] FPDFAPI_inflate_fast
> >        0.11%     [.] v8::internal::HeapObject::SizeFro
> >        0.09%     [.] tracked_objects::ThreadData::Tall
> > ...
> 
> To continue this monologue, perf doesn't even look at these entries. So some
> hists__calc_col_len() calls seem to be missing for the "perf top --hierarchy"
> case or hists__reset_col_len() is called too early or too often.

Right, it missed to call the function for leaf entries.  Could you
please check below patch fixes the problem?

Thanks,
Namhyung


>From 83383fa1412ddb4a1b7113aea799df63887bb4f8 Mon Sep 17 00:00:00 2001
From: Namhyung Kim <namhyung@kernel.org>
Date: Mon, 24 Oct 2016 13:32:23 +0900
Subject: [PATCH] perf hists: Fix column length on --hierarchy

Markus reported that there's a weird behavior on perf top --hierarch
regarding the column length.  Looking at the code, I found a debious
code which affects the symtoms.  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-by: Markus Trippelsdorf <markus@trippelsdorf.de>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 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.10.0