linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Scrolling down broken with "perf top --hierarchy"
@ 2016-10-06 16:33 Markus Trippelsdorf
  2016-10-07  1:17 ` Namhyung Kim
  0 siblings, 1 reply; 20+ messages in thread
From: Markus Trippelsdorf @ 2016-10-06 16:33 UTC (permalink / raw)
  To: Namhyung Kim; +Cc: linux-kernel

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Scrolling down broken with "perf top --hierarchy"
  2016-10-06 16:33 Scrolling down broken with "perf top --hierarchy" Markus Trippelsdorf
@ 2016-10-07  1:17 ` Namhyung Kim
  2016-10-07  3:51   ` Markus Trippelsdorf
  0 siblings, 1 reply; 20+ messages in thread
From: Namhyung Kim @ 2016-10-07  1:17 UTC (permalink / raw)
  To: Markus Trippelsdorf; +Cc: linux-kernel

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: Scrolling down broken with "perf top --hierarchy"
  2016-10-07  1:17 ` Namhyung Kim
@ 2016-10-07  3:51   ` Markus Trippelsdorf
  2016-10-07  4:22     ` Namhyung Kim
  0 siblings, 1 reply; 20+ messages in thread
From: Markus Trippelsdorf @ 2016-10-07  3:51 UTC (permalink / raw)
  To: Namhyung Kim; +Cc: linux-kernel

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Scrolling down broken with "perf top --hierarchy"
  2016-10-07  3:51   ` Markus Trippelsdorf
@ 2016-10-07  4:22     ` Namhyung Kim
  2016-10-07  4:32       ` Markus Trippelsdorf
  0 siblings, 1 reply; 20+ messages in thread
From: Namhyung Kim @ 2016-10-07  4:22 UTC (permalink / raw)
  To: Markus Trippelsdorf; +Cc: linux-kernel

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Scrolling down broken with "perf top --hierarchy"
  2016-10-07  4:22     ` Namhyung Kim
@ 2016-10-07  4:32       ` Markus Trippelsdorf
  2016-10-07  4:53         ` Namhyung Kim
  2016-10-07  4:56         ` Markus Trippelsdorf
  0 siblings, 2 replies; 20+ messages in thread
From: Markus Trippelsdorf @ 2016-10-07  4:32 UTC (permalink / raw)
  To: Namhyung Kim; +Cc: linux-kernel

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Scrolling down broken with "perf top --hierarchy"
  2016-10-07  4:32       ` Markus Trippelsdorf
@ 2016-10-07  4:53         ` Namhyung Kim
  2016-10-07 14:35           ` Arnaldo Carvalho de Melo
  2016-10-07  4:56         ` Markus Trippelsdorf
  1 sibling, 1 reply; 20+ messages in thread
From: Namhyung Kim @ 2016-10-07  4:53 UTC (permalink / raw)
  To: Markus Trippelsdorf
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Ingo Molnar,
	Peter Zijlstra, Jiri Olsa

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Scrolling down broken with "perf top --hierarchy"
  2016-10-07  4:32       ` Markus Trippelsdorf
  2016-10-07  4:53         ` Namhyung Kim
@ 2016-10-07  4:56         ` Markus Trippelsdorf
  2016-10-07  5:09           ` Markus Trippelsdorf
  1 sibling, 1 reply; 20+ messages in thread
From: Markus Trippelsdorf @ 2016-10-07  4:56 UTC (permalink / raw)
  To: Namhyung Kim; +Cc: linux-kernel

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Scrolling down broken with "perf top --hierarchy"
  2016-10-07  4:56         ` Markus Trippelsdorf
@ 2016-10-07  5:09           ` Markus Trippelsdorf
  2016-10-08 11:21             ` Markus Trippelsdorf
  0 siblings, 1 reply; 20+ messages in thread
From: Markus Trippelsdorf @ 2016-10-07  5:09 UTC (permalink / raw)
  To: Namhyung Kim; +Cc: linux-kernel

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Scrolling down broken with "perf top --hierarchy"
  2016-10-07  4:53         ` Namhyung Kim
@ 2016-10-07 14:35           ` Arnaldo Carvalho de Melo
  2016-10-24  5:11             ` Namhyung Kim
  0 siblings, 1 reply; 20+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-10-07 14:35 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Markus Trippelsdorf, linux-kernel, Ingo Molnar, Peter Zijlstra,
	Jiri Olsa

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Scrolling down broken with "perf top --hierarchy"
  2016-10-07  5:09           ` Markus Trippelsdorf
@ 2016-10-08 11:21             ` Markus Trippelsdorf
  2016-10-10 17:54               ` Markus Trippelsdorf
  0 siblings, 1 reply; 20+ messages in thread
From: Markus Trippelsdorf @ 2016-10-08 11:21 UTC (permalink / raw)
  To: Namhyung Kim; +Cc: linux-kernel

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Scrolling down broken with "perf top --hierarchy"
  2016-10-08 11:21             ` Markus Trippelsdorf
@ 2016-10-10 17:54               ` Markus Trippelsdorf
  2016-10-24  4:55                 ` Namhyung Kim
  0 siblings, 1 reply; 20+ messages in thread
From: Markus Trippelsdorf @ 2016-10-10 17:54 UTC (permalink / raw)
  To: Namhyung Kim; +Cc: linux-kernel

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: Scrolling down broken with "perf top --hierarchy"
  2016-10-10 17:54               ` Markus Trippelsdorf
@ 2016-10-24  4:55                 ` Namhyung Kim
  2016-10-24  5:53                   ` Markus Trippelsdorf
  0 siblings, 1 reply; 20+ messages in thread
From: Namhyung Kim @ 2016-10-24  4:55 UTC (permalink / raw)
  To: Markus Trippelsdorf; +Cc: linux-kernel, jolsa, acme

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: Scrolling down broken with "perf top --hierarchy"
  2016-10-07 14:35           ` Arnaldo Carvalho de Melo
@ 2016-10-24  5:11             ` Namhyung Kim
  2016-10-24 10:10               ` Taeung Song
  0 siblings, 1 reply; 20+ messages in thread
From: Namhyung Kim @ 2016-10-24  5:11 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Markus Trippelsdorf, linux-kernel, Ingo Molnar, Peter Zijlstra,
	Jiri Olsa, Taeung Song

Hi Arnaldo,

Sorry for late reply.

On Fri, Oct 07, 2016 at 11:35:45AM -0300, Arnaldo Carvalho de Melo wrote:
> 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?

I think it's a flexible way to set the default behavior while it seems
like a little bit complicated for implementation.  Also I think it's
better to popup another dialog at the end and asks for comfirmation
(but it might not be appropriate for --stdio).

And to do that, we need to have a (programmable) way of dealing with
the configs.

Taeung, is there an update on your config patchset (especially for
write support)?

Thanks,
Namhyung

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Scrolling down broken with "perf top --hierarchy"
  2016-10-24  4:55                 ` Namhyung Kim
@ 2016-10-24  5:53                   ` Markus Trippelsdorf
  2016-10-24  6:02                     ` Namhyung Kim
  0 siblings, 1 reply; 20+ messages in thread
From: Markus Trippelsdorf @ 2016-10-24  5:53 UTC (permalink / raw)
  To: Namhyung Kim; +Cc: linux-kernel, jolsa, acme

On 2016.10.24 at 13:55 +0900, Namhyung Kim wrote:
> 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?

Yes, it does. Thank you.

Another issue: all entries vanish if one scrolls to the left two times.

-- 
Markus

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Scrolling down broken with "perf top --hierarchy"
  2016-10-24  5:53                   ` Markus Trippelsdorf
@ 2016-10-24  6:02                     ` Namhyung Kim
  2016-10-24  6:04                       ` Markus Trippelsdorf
  0 siblings, 1 reply; 20+ messages in thread
From: Namhyung Kim @ 2016-10-24  6:02 UTC (permalink / raw)
  To: Markus Trippelsdorf; +Cc: linux-kernel, jolsa, acme

On Mon, Oct 24, 2016 at 07:53:12AM +0200, Markus Trippelsdorf wrote:
> On 2016.10.24 at 13:55 +0900, Namhyung Kim wrote:
> > 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:
> > > > > > 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?
> 
> Yes, it does. Thank you.
> 
> Another issue: all entries vanish if one scrolls to the left two times.

Hmm.. Did you mean pressing RIGHT key two times?

Thanks,
Namhyung

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Scrolling down broken with "perf top --hierarchy"
  2016-10-24  6:02                     ` Namhyung Kim
@ 2016-10-24  6:04                       ` Markus Trippelsdorf
  2016-10-24  6:14                         ` Namhyung Kim
  0 siblings, 1 reply; 20+ messages in thread
From: Markus Trippelsdorf @ 2016-10-24  6:04 UTC (permalink / raw)
  To: Namhyung Kim; +Cc: linux-kernel, jolsa, acme

On 2016.10.24 at 15:02 +0900, Namhyung Kim wrote:
> On Mon, Oct 24, 2016 at 07:53:12AM +0200, Markus Trippelsdorf wrote:
> > On 2016.10.24 at 13:55 +0900, Namhyung Kim wrote:
> > > 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:
> > > > > > > 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?
> > 
> > Yes, it does. Thank you.
> > 
> > Another issue: all entries vanish if one scrolls to the left two times.
> 
> Hmm.. Did you mean pressing RIGHT key two times?

Ah, yes, sorry.

-- 
Markus

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Scrolling down broken with "perf top --hierarchy"
  2016-10-24  6:04                       ` Markus Trippelsdorf
@ 2016-10-24  6:14                         ` Namhyung Kim
  0 siblings, 0 replies; 20+ messages in thread
From: Namhyung Kim @ 2016-10-24  6:14 UTC (permalink / raw)
  To: Markus Trippelsdorf; +Cc: linux-kernel, jolsa, acme

On Mon, Oct 24, 2016 at 08:04:31AM +0200, Markus Trippelsdorf wrote:
> On 2016.10.24 at 15:02 +0900, Namhyung Kim wrote:
> > On Mon, Oct 24, 2016 at 07:53:12AM +0200, Markus Trippelsdorf wrote:
> > > Another issue: all entries vanish if one scrolls to the left two times.
> > 
> > Hmm.. Did you mean pressing RIGHT key two times?
> 
> Ah, yes, sorry.

Ok, will fix.  Thanks for the report.

Thanks,
Namhyung

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Scrolling down broken with "perf top --hierarchy"
  2016-10-24  5:11             ` Namhyung Kim
@ 2016-10-24 10:10               ` Taeung Song
  2016-10-24 16:37                 ` Namhyung Kim
  0 siblings, 1 reply; 20+ messages in thread
From: Taeung Song @ 2016-10-24 10:10 UTC (permalink / raw)
  To: Namhyung Kim, Arnaldo Carvalho de Melo
  Cc: Markus Trippelsdorf, linux-kernel, Ingo Molnar, Peter Zijlstra,
	Jiri Olsa

Hi, Namhyung and Arnaldo :)

On 10/24/2016 02:11 PM, Namhyung Kim wrote:
> Hi Arnaldo,
>
> Sorry for late reply.
>
> On Fri, Oct 07, 2016 at 11:35:45AM -0300, Arnaldo Carvalho de Melo wrote:
>> 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?
>
> I think it's a flexible way to set the default behavior while it seems
> like a little bit complicated for implementation.  Also I think it's
> better to popup another dialog at the end and asks for comfirmation
> (but it might not be appropriate for --stdio).
>
> And to do that, we need to have a (programmable) way of dealing with
> the configs.
>
> Taeung, is there an update on your config patchset (especially for
> write support)?
>

Is related this link with what you said ? 
https://lkml.org/lkml/2016/1/11/495

Yes, the config patchset would be need to be updated.
Because the config patchset which has 'write' feature
don't use a recent 'struct perf_config_set' so I should change it
to use 'perf_config_set' like show_config() of builtin-config.c:36.

Do you need write support of perf-config command ?
If this feature is more necessary than a recent patchset about default 
config array https://lkml.org/lkml/2016/9/5/17,
I'd remake config patchset for getting and setting features first. :)


Thanks,
Taeung

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Scrolling down broken with "perf top --hierarchy"
  2016-10-24 10:10               ` Taeung Song
@ 2016-10-24 16:37                 ` Namhyung Kim
  2016-10-27  0:45                   ` Taeung Song
  0 siblings, 1 reply; 20+ messages in thread
From: Namhyung Kim @ 2016-10-24 16:37 UTC (permalink / raw)
  To: Taeung Song
  Cc: Arnaldo Carvalho de Melo, Markus Trippelsdorf, linux-kernel,
	Ingo Molnar, Peter Zijlstra, Jiri Olsa

Hi Taeung,

On Mon, Oct 24, 2016 at 07:10:24PM +0900, Taeung Song wrote:
> Hi, Namhyung and Arnaldo :)
> 
> On 10/24/2016 02:11 PM, Namhyung Kim wrote:
> > Hi Arnaldo,
> > 
> > Sorry for late reply.
> > 
> > On Fri, Oct 07, 2016 at 11:35:45AM -0300, Arnaldo Carvalho de Melo wrote:
> > > 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?
> > 
> > I think it's a flexible way to set the default behavior while it seems
> > like a little bit complicated for implementation.  Also I think it's
> > better to popup another dialog at the end and asks for comfirmation
> > (but it might not be appropriate for --stdio).
> > 
> > And to do that, we need to have a (programmable) way of dealing with
> > the configs.
> > 
> > Taeung, is there an update on your config patchset (especially for
> > write support)?
> > 
> 
> Is related this link with what you said ?
> https://lkml.org/lkml/2016/1/11/495

Yep, that kind of thing.

> 
> Yes, the config patchset would be need to be updated.
> Because the config patchset which has 'write' feature
> don't use a recent 'struct perf_config_set' so I should change it
> to use 'perf_config_set' like show_config() of builtin-config.c:36.
> 
> Do you need write support of perf-config command ?
> If this feature is more necessary than a recent patchset about default
> config array https://lkml.org/lkml/2016/9/5/17,
> I'd remake config patchset for getting and setting features first. :)

What I need is a way to add a config item with specific value.  Maybe
I can just append a line into a config file, but it needs to check
possible conflict somehow.  So I think it needs to process existing
config items properly and update with the new value.

Thanks,
Namhyung

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Scrolling down broken with "perf top --hierarchy"
  2016-10-24 16:37                 ` Namhyung Kim
@ 2016-10-27  0:45                   ` Taeung Song
  0 siblings, 0 replies; 20+ messages in thread
From: Taeung Song @ 2016-10-27  0:45 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Arnaldo Carvalho de Melo, Markus Trippelsdorf, linux-kernel,
	Ingo Molnar, Peter Zijlstra, Jiri Olsa

Sorry for late reply, Namhyung

On 10/25/2016 01:37 AM, Namhyung Kim wrote:
> Hi Taeung,
>
> On Mon, Oct 24, 2016 at 07:10:24PM +0900, Taeung Song wrote:
>> Hi, Namhyung and Arnaldo :)
>>
>> On 10/24/2016 02:11 PM, Namhyung Kim wrote:
>>> Hi Arnaldo,
>>>
>>> Sorry for late reply.
>>>
>>> On Fri, Oct 07, 2016 at 11:35:45AM -0300, Arnaldo Carvalho de Melo wrote:
>>>> 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?
>>>
>>> I think it's a flexible way to set the default behavior while it seems
>>> like a little bit complicated for implementation.  Also I think it's
>>> better to popup another dialog at the end and asks for comfirmation
>>> (but it might not be appropriate for --stdio).
>>>
>>> And to do that, we need to have a (programmable) way of dealing with
>>> the configs.
>>>
>>> Taeung, is there an update on your config patchset (especially for
>>> write support)?
>>>
>>
>> Is related this link with what you said ?
>> https://lkml.org/lkml/2016/1/11/495
>
> Yep, that kind of thing.
>
>>
>> Yes, the config patchset would be need to be updated.
>> Because the config patchset which has 'write' feature
>> don't use a recent 'struct perf_config_set' so I should change it
>> to use 'perf_config_set' like show_config() of builtin-config.c:36.
>>
>> Do you need write support of perf-config command ?
>> If this feature is more necessary than a recent patchset about default
>> config array https://lkml.org/lkml/2016/9/5/17,
>> I'd remake config patchset for getting and setting features first. :)
>
> What I need is a way to add a config item with specific value.  Maybe
> I can just append a line into a config file, but it needs to check
> possible conflict somehow.  So I think it needs to process existing
> config items properly and update with the new value.
>

I got it. Understood what you said!
I'll send a patchset for this. :)

Thanks,
Taeung

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2016-10-27  0:45 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-06 16:33 Scrolling down broken with "perf top --hierarchy" Markus Trippelsdorf
2016-10-07  1:17 ` Namhyung Kim
2016-10-07  3:51   ` Markus Trippelsdorf
2016-10-07  4:22     ` Namhyung Kim
2016-10-07  4:32       ` Markus Trippelsdorf
2016-10-07  4:53         ` Namhyung Kim
2016-10-07 14:35           ` Arnaldo Carvalho de Melo
2016-10-24  5:11             ` Namhyung Kim
2016-10-24 10:10               ` Taeung Song
2016-10-24 16:37                 ` Namhyung Kim
2016-10-27  0:45                   ` Taeung Song
2016-10-07  4:56         ` Markus Trippelsdorf
2016-10-07  5:09           ` Markus Trippelsdorf
2016-10-08 11:21             ` Markus Trippelsdorf
2016-10-10 17:54               ` Markus Trippelsdorf
2016-10-24  4:55                 ` Namhyung Kim
2016-10-24  5:53                   ` Markus Trippelsdorf
2016-10-24  6:02                     ` Namhyung Kim
2016-10-24  6:04                       ` Markus Trippelsdorf
2016-10-24  6:14                         ` Namhyung Kim

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).