Re: [PATCH v2] perf diff: Report noisy for cycles diff

["Jin, Yao" <yao.jin@linux.intel.com>]

On 8/6/2019 4:34 PM, Jiri Olsa wrote:
> On Thu, Jul 25, 2019 at 06:14:32AM +0800, Jin Yao wrote:
> 
> SNIP
> 
>>   static int cycles_printf(struct hist_entry *he, struct hist_entry *pair,
>> -			 struct perf_hpp *hpp, int width)
>> +			 struct perf_hpp *hpp, int width __maybe_unused)
>>   {
>>   	struct block_hist *bh = container_of(he, struct block_hist, he);
>>   	struct block_hist *bh_pair = container_of(pair, struct block_hist, he);
>>   	struct hist_entry *block_he;
>>   	struct block_info *bi;
>> -	char buf[128];
>> +	char buf[128], spark[32];
>>   	char *start_line, *end_line;
>> +	int ret = 0, pad;
>> +	char pfmt[20] = " ";
>> +	double d;
>>   
>>   	block_he = hists__get_entry(&bh_pair->block_hists, bh->block_idx);
>>   	if (!block_he) {
>> @@ -1350,18 +1375,56 @@ static int cycles_printf(struct hist_entry *he, struct hist_entry *pair,
>>   	end_line = map__srcline(he->ms.map, bi->sym->start + bi->end,
>>   				he->ms.sym);
>>   
>> -	if ((start_line != SRCLINE_UNKNOWN) && (end_line != SRCLINE_UNKNOWN)) {
>> -		scnprintf(buf, sizeof(buf), "[%s -> %s] %4ld",
>> -			  start_line, end_line, block_he->diff.cycles);
>> +	if (show_noisy) {
>> +		ret = print_stat_spark(spark, sizeof(spark),
>> +				       &block_he->diff.stats);
>> +		d = rel_stddev_stats(stddev_stats(&block_he->diff.stats),
>> +				     avg_stats(&block_he->diff.stats));
>> +
>> +		if ((start_line != SRCLINE_UNKNOWN) &&
>> +		    (end_line != SRCLINE_UNKNOWN)) {
>> +			scnprintf(buf, sizeof(buf),
>> +				  "[%s -> %s] %4ld  %s%5.1f%% %s",
>> +				  start_line, end_line, block_he->diff.cycles,
>> +				  "\u00B1", d, spark);
>> +		} else {
>> +			scnprintf(buf, sizeof(buf),
>> +				  "[%7lx -> %7lx] %4ld  %s%5.1f%% %s",
>> +				  bi->start, bi->end, block_he->diff.cycles,
>> +				  "\u00B1", d, spark);
>> +		}
>> +
>> +		if (ret > 0) {
>> +			pad = 8 - ((ret - 1) / 3);
>> +			scnprintf(pfmt, 20, "%%%ds",
>> +				  81 + (2 * ((ret - 1) / 3)) - pad);
>> +			ret = scnprintf(hpp->buf, hpp->size, pfmt, buf);
>> +			if (pad > 0) {
>> +				ret += scnprintf(hpp->buf + ret,
>> +						 hpp->size - ret,
>> +						 "%-*s", pad, " ");
>> +			}
>> +		} else {
>> +			ret = scnprintf(hpp->buf, hpp->size, "%73s", buf);
>> +			ret += scnprintf(hpp->buf + ret, hpp->size - ret,
>> +					 "%-*s", 8, " ");
>> +		}
> 
> 
> hum, why isn't the histogram in the separate column?
> looks like there's lot of duplicated code in here
> 
> thanks,
> jirka
> 

Yes, it'd better add the histogram in a separate column. But it's not 
very easy to add that. Anyway let me double check if I can find a less 
complicated method for that.

Thanks
Jin Yao


>>   	} else {
>> -		scnprintf(buf, sizeof(buf), "[%7lx -> %7lx] %4ld",
>> -			  bi->start, bi->end, block_he->diff.cycles);
>> +		if ((start_line != SRCLINE_UNKNOWN) &&
>> +		    (end_line != SRCLINE_UNKNOWN)) {
>> +			scnprintf(buf, sizeof(buf), "[%s -> %s] %4ld",
>> +				  start_line, end_line, block_he->diff.cycles);
>> +		} else {
>> +			scnprintf(buf, sizeof(buf), "[%7lx -> %7lx] %4ld",
>> +				  bi->start, bi->end, block_he->diff.cycles);
>> +		}
>> +
>> +		ret = scnprintf(hpp->buf, hpp->size, "%*s", width, buf);
>>   	}
>>   
>>   	free_srcline(start_line);
>>   	free_srcline(end_line);
>> -
>> -	return scnprintf(hpp->buf, hpp->size, "%*s", width, buf);
>> +	return ret;
>>   }
> 
> SNIP
>