Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1496755 > unrolled thread

Scrolling down broken with "perf top --hierarchy"

Back to article view | Back to linux.kernel

Contents

  Scrolling down broken with "perf top --hierarchy" Markus Trippelsdorf <markus@trippelsdorf.de> - 2016-10-06 18:40 +0200
    Re: Scrolling down broken with "perf top --hierarchy" Namhyung Kim <namhyung@kernel.org> - 2016-10-07 03:20 +0200
      Re: Scrolling down broken with "perf top --hierarchy" Markus Trippelsdorf <markus@trippelsdorf.de> - 2016-10-07 06:00 +0200
        Re: Scrolling down broken with "perf top --hierarchy" Namhyung Kim <namhyung@kernel.org> - 2016-10-07 06:30 +0200
          Re: Scrolling down broken with "perf top --hierarchy" Markus Trippelsdorf <markus@trippelsdorf.de> - 2016-10-07 06:40 +0200
            Re: Scrolling down broken with "perf top --hierarchy" Markus Trippelsdorf <markus@trippelsdorf.de> - 2016-10-07 07:00 +0200
              Re: Scrolling down broken with "perf top --hierarchy" Markus Trippelsdorf <markus@trippelsdorf.de> - 2016-10-07 07:10 +0200
                Re: Scrolling down broken with "perf top --hierarchy" Markus Trippelsdorf <markus@trippelsdorf.de> - 2016-10-08 13:30 +0200
                  Re: Scrolling down broken with "perf top --hierarchy" Markus Trippelsdorf <markus@trippelsdorf.de> - 2016-10-10 20:00 +0200
            Re: Scrolling down broken with "perf top --hierarchy" Namhyung Kim <namhyung@kernel.org> - 2016-10-07 07:00 +0200
              Re: Scrolling down broken with "perf top --hierarchy" Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-10-07 16:40 +0200

#1496755 — Scrolling down broken with "perf top --hierarchy"

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).

-- 
Markus

[toc] | [next] | [standalone]

#1496958

Hi Markus,

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.



From 38ef0f20e6b787fcdab265307ac679cfac6dd0ff Mon Sep 17 00:00:00 2001
From: Namhyung Kim <namhyung@kernel.org>
Date: Fri, 7 Oct 2016 10:09:42 +0900
Subject: [PATCH] perf top: Fix refreshing hierarchy entries on TUI

Markus reported that perf top --hierarch cannot scroll down after
refresh.  This was because the number of entries are not updated when
hierarchy is enabled.

Unlike normal report view, hierarchy mode needs to keep its own entry
count since it can have non-leaf entries which can expand/collapse.

Reported-by: Markus Trippelsdorf <markus@trippelsdorf.de>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/ui/browsers/hists.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
index fb8e42c7507a..4ffff7be9299 100644
--- a/tools/perf/ui/browsers/hists.c
+++ b/tools/perf/ui/browsers/hists.c
@@ -601,7 +601,8 @@ int hist_browser__run(struct hist_browser *browser, const char *help)
 			u64 nr_entries;
 			hbt->timer(hbt->arg);
 
-			if (hist_browser__has_filter(browser))
+			if (hist_browser__has_filter(browser) ||
+			    symbol_conf.report_hierarchy)
 				hist_browser__update_nr_entries(browser);
 
 			nr_entries = hist_browser__nr_entries(browser);
-- 
2.9.3

[toc] | [prev] | [next] | [standalone]

#1496979

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.

-- 
Markus

[toc] | [prev] | [next] | [standalone]

#1496992

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?

Thanks,
Namhyung

[toc] | [prev] | [next] | [standalone]

#1496996

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 in the long run you should think of making "perf top --hierarchy"
the default for perf top, because it gives a much better (uncluttered)
overview of what is going on.)

-- 
Markus

[toc] | [prev] | [next] | [standalone]

#1497001

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.

-- 
Markus

[toc] | [prev] | [next] | [standalone]

#1497003

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.

-- 
Markus

[toc] | [prev] | [next] | [standalone]

#1497711

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

-- 
Markus

[toc] | [prev] | [next] | [standalone]

#1498461

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.

Anyway, the following hack "fixes" the issue for me:

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index b02992efb513..7e468fa56980 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -67,7 +67,7 @@ void hists__calc_col_len(struct hists *hists, struct hist_entry *h)
         * +3 accounts for ' y ' symtab origin info
         */
        if (h->ms.sym) {
-               symlen = h->ms.sym->namelen + 4;
+               symlen = 200;
                if (verbose)
                        symlen += BITS_PER_LONG / 4 + 2 + 3;
                hists__new_col_len(hists, HISTC_SYMBOL, symlen);


-- 
Markus

[toc] | [prev] | [next] | [standalone]

#1497002

Cc-ing perf maintainers,

On Fri, Oct 07, 2016 at 06:32:29AM +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.

Ok, I'll send a formal patch with it.

> 
> (And in the long run you should think of making "perf top --hierarchy"
> the default for perf top, because it gives a much better (uncluttered)
> overview of what is going on.)

I think it's a matter of taste.  Some people prefer to see the top
single function or something (i.e. current behavior) while others
prefer to see a higher-level view.

But we can think again about the default at least for perf-top.  I
worried about changing default behavior because last time we did it
for children mode many people complained about it.  But I do think the
hierarchy mode is useful for many people though.

Hmm.. I thought that it already has a config option to enable hierarch
mode by default, but I cannot find it now.

Thanks,
Namhyung

[toc] | [prev] | [next] | [standalone]

#1497185

Em Fri, Oct 07, 2016 at 01:53:57PM +0900, Namhyung Kim escreveu:
> Cc-ing perf maintainers,
> 
> On Fri, Oct 07, 2016 at 06:32:29AM +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.
> 
> Ok, I'll send a formal patch with it.
> 
> > 
> > (And in the long run you should think of making "perf top --hierarchy"
> > the default for perf top, because it gives a much better (uncluttered)
> > overview of what is going on.)
> 
> I think it's a matter of taste.  Some people prefer to see the top
> single function or something (i.e. current behavior) while others
> prefer to see a higher-level view.
> 
> But we can think again about the default at least for perf-top.  I
> worried about changing default behavior because last time we did it
> for children mode many people complained about it.  But I do think the
> hierarchy mode is useful for many people though.

So, I think in such cases we could experiment with asking the user about
switching to the new mode by showing a popup message telling what it is
about, if the user says "yes, I want to try it" switch to it and if
another hotkey is pressed later, write what was chosen (yes, switch to
this new mode, no, I don't like it, don't pester me about it anymore) to
its ~/.perfconfig file so that next time it goes straight to this new
mode, else don't ask the user again and keep using whatever mode was
there already.

What do you think?

- Arnaldo

> Hmm.. I thought that it already has a config option to enable hierarch
> mode by default, but I cannot find it now.
> 
> Thanks,
> Namhyung

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web