Re: [PATCH v4] perf sched map: Add command-name, fuzzy-name options to filter the output map

[Madadi Vineeth Reddy <vineethr@linux.ibm.com>]

Hi Namhyung,

On 25/06/24 10:26, Namhyung Kim wrote:
> Hello,
> 
> On Tue, Jun 18, 2024 at 11:37:08PM +0530, Madadi Vineeth Reddy wrote:
>> By default, perf sched map prints sched-in events for all the tasks
>> which may not be required all the time as it prints lot of symbols
>> and rows to the terminal.
>>
>> With --command-name option, one could specify the specific command(s)
>> for which the map has to be shown. This would help in analyzing the
>> CPU usage patterns easier for that specific command(s). Since multiple
>> PID's might have the same command name, using command-name filter
>> would be more useful for debugging.
>>
>> Multiple command names can be given with a comma separator without
>> whitespace.
>>
>> The --fuzzy-name option can be used if fuzzy name matching is required.
>> For example, "taskname" can be matched to any string that contains
>> "taskname" as its substring.
> 
> Can we split the changes into separate commit?
> 
>  1. add --command-name filter, but I think --task-name is better.
>  2. add multiple name support using CSV
>  3. add --fuzzy-name support
> 
> Although change 2 and 3 can be trivial, having them separately would be
> better for reviewers.
> 

Sure, I will do that.

>>
>> For other tasks, instead of printing the symbol, ** is printed and
>> the same . is used to represent idle. ** is used instead of symbol
>> for other tasks because it helps in clear visualization of command(s)
>> of interest and secondly the symbol itself doesn't mean anything
>> because the sched-in of that symbol will not be printed(first sched-in
>> contains pid and the corresponding symbol).
> 
> I feel like '**' might be getting too much attention in the output.  As
> it's not interested, less characters like '-' could be used.
> 

Yes, '-' seems better, will do that.

>>
>> When using the --command-name option, the sched-out time is represented
>> by a '+'. Since not all task sched-in events are printed, the sched-out
>> time of the relevant commands might be lost. This representation ensures
>> that the sched-out time of the interested commands is not overlooked.
>> The sched-out values for non-current CPUs are skipped because the sched-out
>> symbol would be irrelevant.
> 
> Well, I think it's also a sched-in of another task, we can use '*- '
> instead of adding a new '+' sign for sched-out.
> 

Sure, will change it.

>>
>> 6.10.0-rc1
>> ==========
>> *A0                     794225.687532 secs A0 => migration/0:18
>>   *.                    794225.687544 secs .  => swapper:0
>>    .  *B0               794225.687628 secs B0 => migration/1:21
>>    .  *.                794225.687639 secs
>>    .   .  *C0           794225.687704 secs C0 => migration/2:26
>>    .   .  *.            794225.687715 secs
>>   *D0  .   .            794225.687829 secs D0 => perf-exec:332914
>>    D0  .   .  *.        794225.687926 secs
>>    D0  .   .  *E0       794225.689369 secs E0 => schbench:332916
>>    D0  .  *F0  E0       794225.689409 secs F0 => schbench:332917
>>
>> 6.10.0-rc1 + patch (--command-name perf)
>> =============
>>    ** *A0  **  **       794226.581112 secs A0 => perf:332915
>>       +A0               794226.581145 secs
>>    ** *A0  **  **       794227.582150 secs
>>       +A0               794227.582162 secs
>>    ** *A0  .   .        794228.583167 secs
>>       +A0               794228.583177 secs
>>    ** *A0  **  **       794229.634027 secs
>>       +A0               794229.634040 secs
>>    ** *A0  .   **       794230.635045 secs
>>       +A0               794230.635058 secs
>>    **  **  ** *B0       794231.204272 secs B0 => perf:332912
>>               +B0       794231.204352 secs
> 
> Something like:
> 
>      -  *A0  -   -
>      -  *-   -   -
>      -  *A0  -   -
>      -  *-   -   -
>   ...
>      -   -   -  *B0
>      -   -   -  *-
>
 
Got it. I will send a v5 with the suggested changes. Thank you for reviewing it.

Thanks and Regards
Madadi Vineeth Reddy

> Thanks,
> Namhyung
>