lkml.org 
[lkml]   [2020]   [Mar]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v7 2/3] perf report: Support a new key to reload the browser
From
Date


On 3/19/2020 2:52 AM, Arnaldo Carvalho de Melo wrote:
> Em Thu, Feb 20, 2020 at 09:36:15AM +0800, Jin Yao escreveu:
>> Sometimes we may need to reload the browser to update the output since
>> some options are changed.
>>
>> This patch creates a new key K_RELOAD. Once the __cmd_report() returns
>> K_RELOAD, it would repeat the whole process, such as, read samples from
>> data file, sort the data and display in the browser.
>>
>> v7:
>> ---
>> Rebase to perf/core, no other change.
>>
>> v6:
>> ---
>> No change.
>>
>> v5:
>> ---
>> 1. Fix the 'make NO_SLANG=1' error. Define K_RELOAD in util/hist.h.
>> 2. Skip setup_sorting() in repeat path if last key is K_RELOAD.
>>
>> v4:
>> ---
>> Need to quit in perf_evsel_menu__run if key is K_RELOAD.
>>
>> v3:
>> ---
>> No change.
>>
>> v2:
>> ---
>> This is a new patch created in v2.
>>
>> Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
>> ---
>> tools/perf/builtin-report.c | 6 +++---
>> tools/perf/ui/browsers/hists.c | 1 +
>> tools/perf/ui/keysyms.h | 1 +
>> tools/perf/util/hist.h | 1 +
>> 4 files changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
>> index 862c7f8853dc..842ef92c3598 100644
>> --- a/tools/perf/builtin-report.c
>> +++ b/tools/perf/builtin-report.c
>> @@ -635,7 +635,7 @@ static int report__browse_hists(struct report *rep)
>> * Usually "ret" is the last pressed key, and we only
>> * care if the key notifies us to switch data file.
>> */
>> - if (ret != K_SWITCH_INPUT_DATA)
>> + if (ret != K_SWITCH_INPUT_DATA && ret != K_RELOAD)
>> ret = 0;
>> break;
>> case 2:
>> @@ -1469,7 +1469,7 @@ int cmd_report(int argc, const char **argv)
>> sort_order = sort_tmp;
>> }
>>
>> - if ((last_key != K_SWITCH_INPUT_DATA) &&
>> + if ((last_key != K_SWITCH_INPUT_DATA && last_key != K_RELOAD) &&
>> (setup_sorting(session->evlist) < 0)) {
>> if (sort_order)
>> parse_options_usage(report_usage, options, "s", 1);
>> @@ -1548,7 +1548,7 @@ int cmd_report(int argc, const char **argv)
>> sort__setup_elide(stdout);
>>
>> ret = __cmd_report(&report);
>> - if (ret == K_SWITCH_INPUT_DATA) {
>> + if (ret == K_SWITCH_INPUT_DATA || ret == K_RELOAD) {
>> perf_session__delete(session);
>> last_key = K_SWITCH_INPUT_DATA;
>
> Are you sure this shouldn't be:
>
> last_key = ret;
>
> ?
>

Yes, it should not be. We can reuse original flow for processing
K_SWITCH_INPUT_DATA.

> I'm applying it to test now anyway,
>
> - Arnaldo
>

Thanks!

Thanks
Jin Yao

>> goto repeat;
>> diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
>> index f36dee499320..7c091fa51a5c 100644
>> --- a/tools/perf/ui/browsers/hists.c
>> +++ b/tools/perf/ui/browsers/hists.c
>> @@ -3440,6 +3440,7 @@ static int perf_evsel_menu__run(struct evsel_menu *menu,
>> pos = perf_evsel__prev(pos);
>> goto browse_hists;
>> case K_SWITCH_INPUT_DATA:
>> + case K_RELOAD:
>> case 'q':
>> case CTRL('c'):
>> goto out;
>> diff --git a/tools/perf/ui/keysyms.h b/tools/perf/ui/keysyms.h
>> index fbfac29077f2..04cc4e5c031f 100644
>> --- a/tools/perf/ui/keysyms.h
>> +++ b/tools/perf/ui/keysyms.h
>> @@ -25,5 +25,6 @@
>> #define K_ERROR -2
>> #define K_RESIZE -3
>> #define K_SWITCH_INPUT_DATA -4
>> +#define K_RELOAD -5
>>
>> #endif /* _PERF_KEYSYMS_H_ */
>> diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
>> index 0aa63aeb58ec..bb994e030495 100644
>> --- a/tools/perf/util/hist.h
>> +++ b/tools/perf/util/hist.h
>> @@ -536,6 +536,7 @@ static inline int block_hists_tui_browse(struct block_hist *bh __maybe_unused,
>> #define K_LEFT -1000
>> #define K_RIGHT -2000
>> #define K_SWITCH_INPUT_DATA -3000
>> +#define K_RELOAD -4000
>> #endif
>>
>> unsigned int hists__sort_list_width(struct hists *hists);
>> --
>> 2.17.1
>>
>

\
 
 \ /
  Last update: 2020-03-19 02:41    [W:0.060 / U:0.168 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site