Re: [PATCH] perf tool: Improve bash command line auto-complete for multiple events with comma

From: Jin, Yao
Date: Thu Dec 21 2017 - 21:13:05 EST




On 12/22/2017 12:26 AM, Arnaldo Carvalho de Melo wrote:
Em Thu, Dec 21, 2017 at 05:26:10PM +0800, Jin Yao escreveu:
perf has perf-completion.sh to define command line auto-completion in
bash/zsh.

For record/stat -e it works for single events, but not working when
specifying multiple events with comma.

I'm testing this, and found one issue, that can be fixed in a followup
patch, I think:

If I do:

perf stat -e <TAB>

Then it will get all events and offer them as completions:

[acme@jouet perf]$ perf stat -e
Display all 1523 possibilities? (y or n)

Which is around what:

[acme@jouet perf]$ perf list --raw-dump | wc -w
1509
[acme@jouet perf]$

gives.

Ok, so if I say yes all will be presented, and the pager will be used,
etc. then I can press 'q' as soon as I find the one I want and continue
from there (the pager doesn't allow searching with '/', would be nice).

then, the behaviour changes after I add a comma:

[acme@jouet perf]$ perf stat -e cycles,<TAB>
0000-cover-letter.patch Kbuild
0001-perf-tools-Use-shell-function-for-perl-cflags-retrie.patch Kconfig
<SNIP>
[acme@jouet perf]$ perf stat -e cycles

I.e. after the comma it tries autocompletion with files in the local
directory, not with the list of all events.

Only if I have some character right after the comma is that it will look
for events and not files in the local directory:

ipc/
[acme@jouet perf]$ perf stat -e cycles,i<TAB>
icache.hit idq_uops_not_delivered.cycles_le_3_uop_deliv.core
icache.ifdata_stall ild_stall.lcp
icache.misses inst_retired.any
idq.all_dsb_cycles_4_uops inst_retired.any_p
idq.all_dsb_cycles_any_uops inst_retired.prec_dist
idq.all_mite_cycles_4_uops inst_retired.x87
idq.all_mite_cycles_any_uops instructions
idq.dsb_cycles intel_bts//
idq.dsb_uops intel_pt//
<SNIP>
[acme@jouet perf]$ perf stat -e cycles,i

Would be nice to have the same behaviour right after the comma as when
right after -e.

I'm doing a few more tests but I think this can be merged as-is and the
above be just a suggestion for improvement.

Thanks,

- Arnaldo


Hi Arnaldo,

Thanks so much for testing and merging this patch.

Yes, it could have a further improvement. I will post a follow-up patch to fix this issue.

Thanks
Jin Yao