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

[Jin, Yao]

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