Re: [PATCH 1/6] perf timechart: always try to print at least 15 tasks

[Namhyung Kim <namhyung@kernel.org>]

Hi Stanislav,

On Tue, 22 Oct 2013 14:56:47 +0400, Stanislav Fomichev wrote:
> Always try to print at least 15 tasks no matter how long they run.
> Add -n option to specify desired number of tasks to print.

Hmm.. I think that this patch tried to do many things at once.

1. introduce while loop: it's a behavioral change so that it can be a
separate patch.  But it seems not checking the process filter - in that
case the loop is almost useless IMHO.

2. new -n option: it should update Documentation/perf-timechart.txt
also.  And the long option name "number" is too general.

3. two if(proc_num): what is this?  Is it for patch 2?

Thanks,
Namhyung

>
> Signed-off-by: Stanislav Fomichev <stfomichev@yandex-team.ru>
> ---
>  tools/perf/builtin-timechart.c | 22 +++++++++++++++-------
>  1 file changed, 15 insertions(+), 7 deletions(-)
>
> diff --git a/tools/perf/builtin-timechart.c b/tools/perf/builtin-timechart.c
> index c2e02319347a..d965f26308ed 100644
> --- a/tools/perf/builtin-timechart.c
> +++ b/tools/perf/builtin-timechart.c
> @@ -40,6 +40,8 @@
>  #define SUPPORT_OLD_POWER_EVENTS 1
>  #define PWR_EVENT_EXIT -1
>  
> +static int proc_num = 15;
> +
>  
>  static unsigned int	numcpus;
>  static u64		min_freq;	/* Lowest CPU frequency seen */
> @@ -944,15 +946,17 @@ static void write_svg_file(const char *filename)
>  {
>  	u64 i;
>  	int count;
> +	int thresh = TIME_THRESH;
>  
>  	numcpus++;
>  
>  
> -	count = determine_display_tasks(TIME_THRESH);
> -
> -	/* We'd like to show at least 15 tasks; be less picky if we have fewer */
> -	if (count < 15)
> -		count = determine_display_tasks(TIME_THRESH / 10);
> +	/* We'd like to show at least proc_num tasks;
> +	 * be less picky if we have fewer */
> +	do {
> +		count = determine_display_tasks(thresh);
> +		thresh /= 10;
> +	} while (thresh && count < proc_num);
>  
>  	open_svg(filename, numcpus, count, first_time, last_time);
>  
> @@ -963,9 +967,11 @@ static void write_svg_file(const char *filename)
>  		svg_cpu_box(i, max_freq, turbo_frequency);
>  
>  	draw_cpu_usage();
> -	draw_process_bars();
> +	if (proc_num)
> +		draw_process_bars();
>  	draw_c_p_states();
> -	draw_wakeups();
> +	if (proc_num)
> +		draw_wakeups();
>  
>  	svg_close();
>  }
> @@ -1094,6 +1100,8 @@ int cmd_timechart(int argc, const char **argv,
>  		       parse_process),
>  	OPT_STRING(0, "symfs", &symbol_conf.symfs, "directory",
>  		    "Look for files with symbols relative to this directory"),
> +	OPT_INTEGER('n', "number", &proc_num,
> +		    "min. number of tasks to print"),
>  	OPT_END()
>  	};
>  	const char * const timechart_usage[] = {