refactor usage of ProfilePhaseStatistics, no need to hand it around via Chart

--
MOS_MIGRATED_REVID=103079475

5 files changed, 41 insertions, 75 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/profiler/chart/AggregatingChartCreator.java b/src/main/java/com/google/devtools/build/lib/profiler/chart/AggregatingChartCreator.java
index c0eb26dad1..3a81934374 100644
--- a/src/main/java/com/google/devtools/build/lib/profiler/chart/AggregatingChartCreator.java
+++ b/src/main/java/com/google/devtools/build/lib/profiler/chart/AggregatingChartCreator.java
@@ -16,11 +16,9 @@ package com.google.devtools.build.lib.profiler.chart;
 
 import com.google.devtools.build.lib.profiler.ProfileInfo;
 import com.google.devtools.build.lib.profiler.ProfileInfo.Task;
-import com.google.devtools.build.lib.profiler.ProfilePhaseStatistics;
 import com.google.devtools.build.lib.profiler.ProfilerTask;
 
 import java.util.EnumSet;
-import java.util.List;
 import java.util.Set;
 
 /**
@@ -64,12 +62,6 @@ public class AggregatingChartCreator implements ChartCreator {
   /** The data of the profiled build. */
   private final ProfileInfo info;
 
-  /**
-   * Statistics of the profiled build. This is expected to be a formatted
-   * string, ready to be printed out.
-   */
-  private final List<ProfilePhaseStatistics> statistics;
-
   /** If true, VFS related information is added to the chart. */
   private final boolean showVFS;
 
@@ -90,31 +82,25 @@ public class AggregatingChartCreator implements ChartCreator {
    * VFS related data to the generated chart.
    *
    * @param info the data of the profiled build
-   * @param statistics Statistics of the profiled build. This is expected to be
-   *        a formatted string, ready to be printed out.
    */
-  public AggregatingChartCreator(ProfileInfo info, List<ProfilePhaseStatistics> statistics) {
-    this(info, statistics, false);
+  public AggregatingChartCreator(ProfileInfo info) {
+    this(info, false);
   }
 
   /**
    * Creates the chart creator.
    *
    * @param info the data of the profiled build
-   * @param statistics Statistics of the profiled build. This is expected to be
-   *        a formatted string, ready to be printed out.
    * @param showVFS if true, VFS related information is added to the chart
    */
-  public AggregatingChartCreator(ProfileInfo info, List<ProfilePhaseStatistics> statistics,
-      boolean showVFS) {
+  public AggregatingChartCreator(ProfileInfo info, boolean showVFS) {
     this.info = info;
-    this.statistics = statistics;
     this.showVFS = showVFS;
   }
 
   @Override
   public Chart create() {
-    Chart chart = new Chart(info.comment, statistics);
+    Chart chart = new Chart(info.comment);
     CommonChartCreator.createCommonChartItems(chart, info);
     createTypes(chart);
 
diff --git a/src/main/java/com/google/devtools/build/lib/profiler/chart/Chart.java b/src/main/java/com/google/devtools/build/lib/profiler/chart/Chart.java
index 93c7c8181a..b13fb8f1c2 100644
--- a/src/main/java/com/google/devtools/build/lib/profiler/chart/Chart.java
+++ b/src/main/java/com/google/devtools/build/lib/profiler/chart/Chart.java
@@ -15,8 +15,6 @@
 package com.google.devtools.build.lib.profiler.chart;
 
 import com.google.common.base.Preconditions;
-import com.google.devtools.build.lib.profiler.ProfilePhaseStatistics;
-
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
@@ -34,9 +32,6 @@ public class Chart {
   /** The title of the chart. */
   private final String title;
 
-  /** Statistics of the profiled build. */
-  private final List<ProfilePhaseStatistics> statistics;
-
   /** The rows of the chart. */
   private final Map<Long, ChartRow> rows = new HashMap<>();
 
@@ -59,14 +54,10 @@ public class Chart {
    * Creates a chart.
    *
    * @param title the title of the chart
-   * @param statistics Statistics of the profiled build. This is expected to be
-   *        a formatted string, ready to be printed out.
    */
-  public Chart(String title, List<ProfilePhaseStatistics> statistics) {
+  public Chart(String title) {
     Preconditions.checkNotNull(title);
-    Preconditions.checkNotNull(statistics);
     this.title = title;
-    this.statistics = statistics;
   }
 
   /**
@@ -219,10 +210,6 @@ public class Chart {
     return title;
   }
 
-  public List<ProfilePhaseStatistics> getStatistics() {
-    return statistics;
-  }
-
   public int getRowCount() {
     return rows.size();
   }
diff --git a/src/main/java/com/google/devtools/build/lib/profiler/chart/DetailedChartCreator.java b/src/main/java/com/google/devtools/build/lib/profiler/chart/DetailedChartCreator.java
index 1e097c3c59..2689b34cb1 100644
--- a/src/main/java/com/google/devtools/build/lib/profiler/chart/DetailedChartCreator.java
+++ b/src/main/java/com/google/devtools/build/lib/profiler/chart/DetailedChartCreator.java
@@ -17,11 +17,9 @@ package com.google.devtools.build.lib.profiler.chart;
 import com.google.devtools.build.lib.profiler.ProfileInfo;
 import com.google.devtools.build.lib.profiler.ProfileInfo.CriticalPathEntry;
 import com.google.devtools.build.lib.profiler.ProfileInfo.Task;
-import com.google.devtools.build.lib.profiler.ProfilePhaseStatistics;
 import com.google.devtools.build.lib.profiler.ProfilerTask;
 
 import java.util.EnumSet;
-import java.util.List;
 
 /**
  * Implementation of {@link ChartCreator} that creates Gantt Charts that contain
@@ -33,26 +31,17 @@ public class DetailedChartCreator implements ChartCreator {
   private final ProfileInfo info;
 
   /**
-   * Statistics of the profiled build. This is expected to be a formatted
-   * string, ready to be printed out.
-   */
-  private final List<ProfilePhaseStatistics> statistics;
-
-  /**
    * Creates the chart creator.
    *
    * @param info the data of the profiled build
-   * @param statistics Statistics of the profiled build. This is expected to be
-   *        a formatted string, ready to be printed out.
    */
-  public DetailedChartCreator(ProfileInfo info, List<ProfilePhaseStatistics> statistics) {
+  public DetailedChartCreator(ProfileInfo info) {
     this.info = info;
-    this.statistics = statistics;
   }
 
   @Override
   public Chart create() {
-    Chart chart = new Chart(info.comment, statistics);
+    Chart chart = new Chart(info.comment);
     CommonChartCreator.createCommonChartItems(chart, info);
     createTypes(chart);
 
diff --git a/src/main/java/com/google/devtools/build/lib/profiler/chart/HtmlChartVisitor.java b/src/main/java/com/google/devtools/build/lib/profiler/chart/HtmlChartVisitor.java
index d6c9c0ed9b..f9c8993707 100644
--- a/src/main/java/com/google/devtools/build/lib/profiler/chart/HtmlChartVisitor.java
+++ b/src/main/java/com/google/devtools/build/lib/profiler/chart/HtmlChartVisitor.java
@@ -14,8 +14,6 @@
 
 package com.google.devtools.build.lib.profiler.chart;
 
-import com.google.devtools.build.lib.profiler.ProfilePhaseStatistics;
-
 import java.io.PrintStream;
 import java.util.List;
 
@@ -93,9 +91,6 @@ public class HtmlChartVisitor implements ChartVisitor {
 
     heading("Legend", 2);
     printLegend(chart.getSortedTypes());
-
-    heading("Statistics", 2);
-    printStatistics(chart.getStatistics());
 }
 
   @Override
@@ -244,26 +239,6 @@ public class HtmlChartVisitor implements ChartVisitor {
     out.println("</div>");
   }
 
-  private void printStatistics(List<ProfilePhaseStatistics> statistics) {
-    boolean first = true;
-
-    out.println("<table border=\"0\" width=\"100%\"><tr>");
-    for (ProfilePhaseStatistics stat : statistics) {
-      if (!first) {
-        out.println("<td><div style=\"width:20px;\">&#160;</div></td>");
-      } else {
-        first = false;
-      }
-      out.println("<td valign=\"top\">");
-      String title = stat.getTitle();
-      if (!title.isEmpty()) {
-        heading(title, 3);
-      }
-      out.println("<pre>" + stat.getStatistics() + "</pre></td>");
-    }
-    out.println("</tr></table>");
-  }
-
   /**
    * Prints a head-line at the current position in the document.
    *
diff --git a/src/main/java/com/google/devtools/build/lib/profiler/chart/HtmlCreator.java b/src/main/java/com/google/devtools/build/lib/profiler/chart/HtmlCreator.java
index c9aca2cf49..4059301a03 100644
--- a/src/main/java/com/google/devtools/build/lib/profiler/chart/HtmlCreator.java
+++ b/src/main/java/com/google/devtools/build/lib/profiler/chart/HtmlCreator.java
@@ -35,20 +35,31 @@ public final class HtmlCreator {
   private final HtmlChartVisitor chartVisitor;
   private final Optional<SkylarkStatistics> skylarkStats;
 
+  /**
+   * Pre-formatted statistics for each phase of the profiled build.
+   */
+  private final List<ProfilePhaseStatistics> statistics;
+
   private HtmlCreator(
       PrintStream out,
       Chart chart,
       Optional<SkylarkStatistics> skylarkStats,
-      int htmlPixelsPerSecond) {
+      int htmlPixelsPerSecond,
+      List<ProfilePhaseStatistics> statistics) {
     this.out = out;
     this.chart = chart;
     chartVisitor = new HtmlChartVisitor(out, htmlPixelsPerSecond);
     this.skylarkStats = skylarkStats;
+    this.statistics = statistics;
   }
 
   private void print() {
     htmlFrontMatter();
     chart.accept(chartVisitor);
+
+    out.println("<h2>Statistics</h2>");
+    printPhaseStatistics();
+
     if (skylarkStats.isPresent()) {
       skylarkStats.get().printHtmlBody();
     }
@@ -58,7 +69,6 @@ public final class HtmlCreator {
   private void htmlFrontMatter() {
     out.println("<html><head>");
     out.printf("<title>%s</title>", chart.getTitle());
-
     chartVisitor.printCss(chart.getSortedTypes());
 
     if (skylarkStats.isPresent()) {
@@ -75,6 +85,25 @@ public final class HtmlCreator {
   }
 
   /**
+   * Print a table from {@link #statistics} arranging the phases side by side.
+   */
+  private void printPhaseStatistics() {
+    out.println("<table border=\"0\" width=\"100%\"><tr>");
+    String statsSeparator = "";
+    for (ProfilePhaseStatistics stat : statistics) {
+      out.println(statsSeparator);
+      out.println("<td valign=\"top\" style=\"margin: 0 10 0;\">");
+      String title = stat.getTitle();
+      if (!title.isEmpty()) {
+        out.println(String.format("<h3>%s</h3>", title));
+      }
+      out.println("<pre>" + stat.getStatistics() + "</pre></td>");
+      statsSeparator = "<td><div style=\"width:20px;\">&#160;</div></td>";
+    }
+    out.println("</tr></table>");
+  }
+
+  /**
    * Writes the HTML profiling information.
    * @param info
    * @param htmlFile
@@ -94,14 +123,14 @@ public final class HtmlCreator {
       ChartCreator chartCreator;
       Optional<SkylarkStatistics> skylarkStats;
       if (detailed) {
-        chartCreator = new DetailedChartCreator(info, statistics);
+        chartCreator = new DetailedChartCreator(info);
         skylarkStats = Optional.of(new SkylarkStatistics(out, info));
       } else {
-        chartCreator = new AggregatingChartCreator(info, statistics);
+        chartCreator = new AggregatingChartCreator(info);
         skylarkStats = Optional.absent();
       }
       Chart chart = chartCreator.create();
-      new HtmlCreator(out, chart, skylarkStats, htmlPixelsPerSecond).print();
+      new HtmlCreator(out, chart, skylarkStats, htmlPixelsPerSecond, statistics).print();
     }
   }
 }