Scrolling down broken with "perf top --hierarchy"

DISCUSSION Markus Trippelsdorf 4 participants 20 messages Oct 6, 2016 – Oct 27, 2016 perf events

Share & tools

Markus Trippelsdorf

author

2016-10-06 16:33 UTC #0 lore raw

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

Signature · 1 line

Markus

Namhyung Kim

maintainer

2016-10-07 01:18 UTC #1 lore raw

Hi Markus,

Quoted 5 lines · Markus Trippelsdorf

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.

Quoted 1 line

From 38ef0f20e6b787fcdab265307ac679cfac6dd0ff Mon Sep 17 00:00:00 2001

From: Namhyung Kim <[email protected]>
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 <[email protected]>

Signed-off-by Namhyung Kim <[email protected]>

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

tools/perf/ui/browsers/hists.c +2 -1

index fb8e42c7507a..4ffff7be9299 100644
@@ -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);

Signature · 1 line

2.9.3

Markus Trippelsdorf

author

2016-10-07 03:51 UTC #2 lore raw

Quoted 8 lines · Namhyung Kim

Quoted 5 lines · Markus Trippelsdorf
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.

Signature · 1 line

Markus

Namhyung Kim

maintainer

2016-10-07 04:22 UTC #3 lore raw

Quoted 11 lines · Markus Trippelsdorf

Quoted 8 lines · Namhyung Kim
Quoted 5 lines · Markus Trippelsdorf
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

Markus Trippelsdorf

author

2016-10-07 04:32 UTC #4 lore raw

Quoted 9 lines · Namhyung Kim

Quoted 8 lines · Markus Trippelsdorf
Quoted 7 lines · Namhyung Kim
Quoted 5 lines · Markus Trippelsdorf
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).

Quoted 5 lines

Quoted 3 lines
Quoted 1 line
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.)

Signature · 1 line

Markus

Namhyung Kim

maintainer

2016-10-07 04:54 UTC #5 lore raw

Cc-ing perf maintainers,

Quoted 10 lines · Markus Trippelsdorf

Quoted 9 lines · Namhyung Kim
Quoted 8 lines · Markus Trippelsdorf
Quoted 7 lines · Namhyung Kim
Quoted 5 lines · Markus Trippelsdorf
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).

Quoted 7 lines

Quoted 5 lines
Quoted 3 lines
Quoted 1 line
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.

Quoted 4 lines

(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

Arnaldo Carvalho de Melo

maintainer

2016-10-07 14:35 UTC #6 lore raw

Em Fri, Oct 07, 2016 at 01:53:57PM +0900, Namhyung Kim escreveu:

Quoted 28 lines

Cc-ing perf maintainers,
Quoted 10 lines · Markus Trippelsdorf
Quoted 9 lines · Namhyung Kim
Quoted 8 lines · Markus Trippelsdorf
Quoted 7 lines · Namhyung Kim
Quoted 5 lines · Markus Trippelsdorf
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).
Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
Quoted 1 line
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.
Quoted 4 lines
(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.)

Quoted 8 lines

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

Quoted 5 lines

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

Namhyung Kim

maintainer

2016-10-24 05:11 UTC #7 lore raw

Hi Arnaldo,

Sorry for late reply.

Quoted 47 lines · Arnaldo Carvalho de Melo

Em Fri, Oct 07, 2016 at 01:53:57PM +0900, Namhyung Kim escreveu:
Quoted 28 lines
Cc-ing perf maintainers,
Quoted 10 lines · Markus Trippelsdorf
Quoted 9 lines · Namhyung Kim
Quoted 8 lines · Markus Trippelsdorf
Quoted 7 lines · Namhyung Kim
Quoted 5 lines · Markus Trippelsdorf
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).
Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
Quoted 1 line
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.
Quoted 4 lines
(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.)
Quoted 8 lines
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.

Quoted 1 line

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

Taeung Song

2016-10-24 10:10 UTC #8 lore raw

Hi, Namhyung and Arnaldo :)

Quoted 59 lines · Namhyung Kim

Hi Arnaldo,
Sorry for late reply.
Quoted 47 lines · Arnaldo Carvalho de Melo
Em Fri, Oct 07, 2016 at 01:53:57PM +0900, Namhyung Kim escreveu:
Quoted 28 lines
Cc-ing perf maintainers,
Quoted 10 lines · Markus Trippelsdorf
Quoted 9 lines · Namhyung Kim
Quoted 8 lines · Markus Trippelsdorf
Quoted 7 lines · Namhyung Kim
Quoted 5 lines · Markus Trippelsdorf
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).
Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
Quoted 1 line
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.
Quoted 4 lines
(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.)
Quoted 8 lines
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.
Quoted 1 line
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).

Quoted 6 lines

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

Namhyung Kim

maintainer

2016-10-24 16:38 UTC #9 lore raw

Hi Taeung,

Quoted 65 lines · Taeung Song

Hi, Namhyung and Arnaldo :)
Quoted 62 lines · Namhyung Kim
Hi Arnaldo,
Sorry for late reply.
Quoted 47 lines · Arnaldo Carvalho de Melo
Em Fri, Oct 07, 2016 at 01:53:57PM +0900, Namhyung Kim escreveu:
Quoted 28 lines
Cc-ing perf maintainers,
Quoted 10 lines · Markus Trippelsdorf
Quoted 9 lines · Namhyung Kim
Quoted 8 lines · Markus Trippelsdorf
Quoted 7 lines · Namhyung Kim
Quoted 5 lines · Markus Trippelsdorf
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).
Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
Quoted 1 line
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.
Quoted 4 lines
(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.)
Quoted 8 lines
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.
Quoted 1 line
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.

Quoted 6 lines

Quoted 3 lines
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.

Quoted 10 lines

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

Taeung Song

2016-10-27 00:45 UTC #10 lore raw

Sorry for late reply, Namhyung

Quoted 88 lines · Namhyung Kim

Hi Taeung,
Quoted 65 lines · Taeung Song
Hi, Namhyung and Arnaldo :)
Quoted 62 lines · Namhyung Kim
Hi Arnaldo,
Sorry for late reply.
Quoted 47 lines · Arnaldo Carvalho de Melo
Em Fri, Oct 07, 2016 at 01:53:57PM +0900, Namhyung Kim escreveu:
Quoted 28 lines
Cc-ing perf maintainers,
Quoted 10 lines · Markus Trippelsdorf
Quoted 9 lines · Namhyung Kim
Quoted 8 lines · Markus Trippelsdorf
Quoted 7 lines · Namhyung Kim
Quoted 5 lines · Markus Trippelsdorf
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).
Quoted 7 lines
Quoted 5 lines
Quoted 3 lines
Quoted 1 line
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.
Quoted 4 lines
(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.)
Quoted 8 lines
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.
Quoted 1 line
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.
Quoted 6 lines
Quoted 3 lines
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.
Quoted 10 lines
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. :)

Quoted 5 lines

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

Markus Trippelsdorf

author

2016-10-07 04:56 UTC #11 lore raw

In reply to Markus Trippelsdorf

Quoted 10 lines · Markus Trippelsdorf

Quoted 9 lines · Namhyung Kim
Quoted 8 lines · Markus Trippelsdorf
Quoted 7 lines · Namhyung Kim
Quoted 5 lines · Markus Trippelsdorf
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).

Quoted 7 lines

Quoted 5 lines
Quoted 3 lines
Quoted 1 line
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.

Signature · 1 line

Markus

Markus Trippelsdorf

author

2016-10-07 05:09 UTC #12 lore raw

Quoted 13 lines · Markus Trippelsdorf

Quoted 12 lines · Markus Trippelsdorf
Quoted 11 lines · Namhyung Kim
Quoted 10 lines · Markus Trippelsdorf
Quoted 8 lines · Namhyung Kim
Quoted 5 lines · Markus Trippelsdorf
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.

Quoted 8 lines

Quoted 5 lines
Quoted 3 lines
Quoted 1 line
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.

Signature · 1 line

Markus

Markus Trippelsdorf

author

2016-10-08 11:21 UTC #13 lore raw

Quoted 18 lines · Markus Trippelsdorf

Quoted 17 lines · Markus Trippelsdorf
Quoted 16 lines · Markus Trippelsdorf
Quoted 14 lines · Namhyung Kim
Quoted 11 lines · Markus Trippelsdorf
Quoted 8 lines · Namhyung Kim
Quoted 5 lines · Markus Trippelsdorf
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?

Quoted 7 lines

Quoted 4 lines
Quoted 1 line
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

...

Signature · 1 line

Markus

Markus Trippelsdorf

author

2016-10-10 17:54 UTC #14 lore raw

Quoted 44 lines · Markus Trippelsdorf

Quoted 18 lines · Markus Trippelsdorf
Quoted 17 lines · Markus Trippelsdorf
Quoted 16 lines · Markus Trippelsdorf
Quoted 14 lines · Namhyung Kim
Quoted 11 lines · Markus Trippelsdorf
Quoted 8 lines · Namhyung Kim
Quoted 5 lines · Markus Trippelsdorf
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?
Quoted 7 lines
Quoted 4 lines
Quoted 1 line
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
(quoted diff) +1 -1
+  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

Review

Quoted 10 lines

(quoted diff) +0 -0

       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:

tools/perf/util/hist.c +1 -1

index b02992efb513..7e468fa56980 100644
@@ -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);

Signature · 1 line

Markus

Namhyung Kim

maintainer

2016-10-24 04:55 UTC #15 lore raw

Hi,

Sorry for late reply.

Quoted 56 lines · Markus Trippelsdorf

Quoted 44 lines · Markus Trippelsdorf
Quoted 18 lines · Markus Trippelsdorf
Quoted 17 lines · Markus Trippelsdorf
Quoted 16 lines · Markus Trippelsdorf
Quoted 14 lines · Namhyung Kim
Quoted 11 lines · Markus Trippelsdorf
Quoted 8 lines · Namhyung Kim
Quoted 5 lines · Markus Trippelsdorf
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?
Quoted 7 lines
Quoted 4 lines
Quoted 1 line
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
(quoted diff) +1 -1
+  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
Review
Quoted 10 lines
(quoted diff) +0 -0
       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
...

Quoted 3 lines

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

Quoted 1 line

From 83383fa1412ddb4a1b7113aea799df63887bb4f8 Mon Sep 17 00:00:00 2001

From: Namhyung Kim <[email protected]>
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 <[email protected]>

Signed-off-by Namhyung Kim <[email protected]>

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

tools/perf/util/hist.c +6 -6

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

Signature · 1 line

2.10.0

Markus Trippelsdorf

author

2016-10-24 05:53 UTC #16 lore raw

Quoted 61 lines · Namhyung Kim

Hi,
Sorry for late reply.
Quoted 56 lines · Markus Trippelsdorf
Quoted 54 lines · Markus Trippelsdorf
Quoted 18 lines · Markus Trippelsdorf
Quoted 17 lines · Markus Trippelsdorf
Quoted 16 lines · Markus Trippelsdorf
Quoted 14 lines · Namhyung Kim
Quoted 11 lines · Markus Trippelsdorf
Quoted 8 lines · Namhyung Kim
Quoted 5 lines · Markus Trippelsdorf
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?
Quoted 7 lines
Quoted 4 lines
Quoted 1 line
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
(quoted diff) +1 -1
+  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
...

Quoted 6 lines

Quoted 3 lines
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.

Signature · 1 line

Markus

Namhyung Kim

maintainer

2016-10-24 06:03 UTC #17 lore raw

Quoted 48 lines · Markus Trippelsdorf

Quoted 47 lines · Namhyung Kim
Hi,
Sorry for late reply.
Quoted 41 lines · Markus Trippelsdorf
Quoted 26 lines · Markus Trippelsdorf
Quoted 7 lines · Markus Trippelsdorf
Quoted 3 lines · Markus Trippelsdorf
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
(quoted diff) +1 -1
+  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
Review
Quoted 10 lines
(quoted diff) +0 -0
       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.

Quoted 6 lines

Quoted 2 lines
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

Markus Trippelsdorf

author

2016-10-24 06:04 UTC #18 lore raw

Quoted 49 lines · Namhyung Kim

Quoted 48 lines · Markus Trippelsdorf
Quoted 47 lines · Namhyung Kim
Hi,
Sorry for late reply.
Quoted 41 lines · Markus Trippelsdorf
Quoted 26 lines · Markus Trippelsdorf
Quoted 7 lines · Markus Trippelsdorf
Quoted 3 lines · Markus Trippelsdorf
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
(quoted diff) +1 -1
+  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
Review
Quoted 10 lines
(quoted diff) +0 -0
       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.

Quoted 8 lines

Quoted 6 lines
Quoted 2 lines
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.

Signature · 1 line

Markus

Namhyung Kim

maintainer

2016-10-24 06:14 UTC #19 lore raw

Quoted 7 lines · Markus Trippelsdorf

Quoted 4 lines · Namhyung Kim
Quoted 1 line · Markus Trippelsdorf
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