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

[Jiri Olsa]

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

>  	} 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