Refactor spawn stat computation.
This should be a no-op change that will allow us more easily to expose the
different runners to the build metrics in the BEP.
PiperOrigin-RevId: 344808222
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BuildSummaryStatsModule.java b/src/main/java/com/google/devtools/build/lib/runtime/BuildSummaryStatsModule.java
index b7eaaf1..daad07a 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BuildSummaryStatsModule.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BuildSummaryStatsModule.java
@@ -14,6 +14,7 @@
package com.google.devtools.build.lib.runtime;
import com.google.common.base.Joiner;
+import com.google.common.collect.ImmutableMap;
import com.google.common.eventbus.AllowConcurrentEvents;
import com.google.common.eventbus.EventBus;
import com.google.common.eventbus.Subscribe;
@@ -169,9 +170,10 @@
}
}
- String spawnSummary = spawnStats.getSummary();
+ ImmutableMap<String, Integer> spawnSummary = spawnStats.getSummary();
+ String spawnSummaryString = SpawnStats.convertSummaryToString(spawnSummary);
if (statsSummary) {
- reporter.handle(Event.info(spawnSummary));
+ reporter.handle(Event.info(spawnSummaryString));
reporter.handle(
Event.info(
String.format(
@@ -191,11 +193,13 @@
executionTime / 1000.0)));
} else {
reporter.handle(Event.info(Joiner.on(", ").join(items)));
- reporter.handle(Event.info(spawnSummary));
+ reporter.handle(Event.info(spawnSummaryString));
}
- event.getResult().getBuildToolLogCollection()
- .addDirectValue("process stats", spawnSummary.getBytes(StandardCharsets.UTF_8));
+ event
+ .getResult()
+ .getBuildToolLogCollection()
+ .addDirectValue("process stats", spawnSummaryString.getBytes(StandardCharsets.UTF_8));
} finally {
if (criticalPathComputer != null) {
eventBus.unregister(criticalPathComputer);
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/SpawnStats.java b/src/main/java/com/google/devtools/build/lib/runtime/SpawnStats.java
index 45360a7..1d241bf 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/SpawnStats.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/SpawnStats.java
@@ -16,12 +16,14 @@
import com.google.common.collect.ConcurrentHashMultiset;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Multiset;
import com.google.devtools.build.lib.actions.ActionResult;
import com.google.devtools.build.lib.actions.SpawnResult;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Comparator;
+import java.util.Map;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicLong;
import javax.annotation.concurrent.ThreadSafe;
@@ -57,29 +59,30 @@
/*
* Returns a human-readable summary of spawns counted.
*/
- public String getSummary() {
+ public ImmutableMap<String, Integer> getSummary() {
return getSummary(REPORT_FIRST);
}
/*
* Returns a human-readable summary of spawns counted.
*/
- public String getSummary(ImmutableList<String> reportFirst) {
- ResultString result = new ResultString();
+ public ImmutableMap<String, Integer> getSummary(ImmutableList<String> reportFirst) {
+ ImmutableMap.Builder<String, Integer> result = ImmutableMap.builder();
int numActionsWithoutInternal = runners.size();
int numActionsTotal = totalNumberOfActions.get();
+ result.put("total", numActionsTotal);
// First report cache results.
for (String s : reportFirst) {
int count = runners.setCount(s, 0);
if (count > 0) {
- result.add(s, count);
+ result.put(s, count);
}
}
// Account for internal actions such as SymlinkTree.
if (numActionsWithoutInternal < numActionsTotal) {
- result.add("internal", numActionsTotal - numActionsWithoutInternal);
+ result.put("internal", numActionsTotal - numActionsWithoutInternal);
}
// Sort the rest alphabetically
@@ -87,33 +90,34 @@
Collections.sort(list, Comparator.comparing(e -> e.getElement()));
for (Multiset.Entry<String> e : list) {
- result.add(e.getElement(), e.getCount());
+ result.put(e.getElement(), e.getCount());
}
- return numActionsTotal + " process" + (numActionsTotal == 1 ? "" : "es") + result + ".";
+ return result.build();
}
- private static class ResultString {
- StringBuilder result = new StringBuilder();
- int runnersNum = 0;
-
- public void add(String name, int count) {
- runnersNum += 1;
-
- if (result.length() > 0) {
- result.append(", ");
- }
- result.append(count);
- result.append(" ");
- result.append(name);
+ public static String convertSummaryToString(ImmutableMap<String, Integer> spawnSummary) {
+ Integer total = spawnSummary.get("total");
+ if (total == 0) {
+ return "0 processes.";
}
- @Override
- public String toString() {
- if (runnersNum == 0) {
- return "";
- }
- return ": " + result;
+ StringBuilder stringSummary = new StringBuilder();
+ stringSummary.append(total).append(" process");
+ if (total > 1) {
+ stringSummary.append("es");
}
+ String separator = ": ";
+
+ for (Map.Entry<String, Integer> runnerStats : spawnSummary.entrySet()) {
+ if ("total".equals(runnerStats.getKey())) {
+ continue;
+ }
+ stringSummary.append(separator);
+ separator = ", ";
+ stringSummary.append(runnerStats.getValue()).append(' ').append(runnerStats.getKey());
+ }
+ stringSummary.append('.');
+ return stringSummary.toString();
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/runtime/SpawnStatsTest.java b/src/test/java/com/google/devtools/build/lib/runtime/SpawnStatsTest.java
index 51d1b3e..b9fe083 100644
--- a/src/test/java/com/google/devtools/build/lib/runtime/SpawnStatsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/runtime/SpawnStatsTest.java
@@ -37,21 +37,23 @@
@Test
public void emptySet() {
- assertThat(stats.getSummary()).isEqualTo("0 processes.");
+ assertThat(SpawnStats.convertSummaryToString(stats.getSummary())).isEqualTo("0 processes.");
}
@Test
public void one() {
stats.countRunnerName("foo");
stats.incrementActionCount();
- assertThat(stats.getSummary()).isEqualTo("1 process: 1 foo.");
+ assertThat(SpawnStats.convertSummaryToString(stats.getSummary()))
+ .isEqualTo("1 process: 1 foo.");
}
@Test
public void oneRemote() {
stats.countRunnerName("remote cache hit");
stats.incrementActionCount();
- assertThat(stats.getSummary()).isEqualTo("1 process: 1 remote cache hit.");
+ assertThat(SpawnStats.convertSummaryToString(stats.getSummary()))
+ .isEqualTo("1 process: 1 remote cache hit.");
}
@Test
@@ -60,7 +62,8 @@
stats.countRunnerName("foo");
stats.incrementActionCount();
}
- assertThat(stats.getSummary()).isEqualTo("2 processes: 2 foo.");
+ assertThat(SpawnStats.convertSummaryToString(stats.getSummary()))
+ .isEqualTo("2 processes: 2 foo.");
}
@Test
@@ -74,7 +77,8 @@
for (int i = 0; i < 6; i++) {
stats.incrementActionCount();
}
- assertThat(stats.getSummary()).isEqualTo("6 processes: 1 a, 2 b, 3 c.");
+ assertThat(SpawnStats.convertSummaryToString(stats.getSummary()))
+ .isEqualTo("6 processes: 1 a, 2 b, 3 c.");
}
@Test
@@ -88,7 +92,8 @@
for (int i = 0; i < 6; i++) {
stats.incrementActionCount();
}
- assertThat(stats.getSummary()).isEqualTo("6 processes: 3 a, 2 b, 1 c.");
+ assertThat(SpawnStats.convertSummaryToString(stats.getSummary()))
+ .isEqualTo("6 processes: 3 a, 2 b, 1 c.");
}
@Test
@@ -103,7 +108,8 @@
for (int i = 0; i < 7; i++) {
stats.incrementActionCount();
}
- assertThat(stats.getSummary()).isEqualTo("7 processes: 1 remote cache hit, 3 a, 2 b, 1 c.");
+ assertThat(SpawnStats.convertSummaryToString(stats.getSummary()))
+ .isEqualTo("7 processes: 1 remote cache hit, 3 a, 2 b, 1 c.");
}
private final SpawnResult rA =
@@ -119,7 +125,8 @@
stats.countActionResult(ActionResult.create(spawns));
stats.incrementActionCount();
- assertThat(stats.getSummary()).isEqualTo("1 process: 1 abc.");
+ assertThat(SpawnStats.convertSummaryToString(stats.getSummary()))
+ .isEqualTo("1 process: 1 abc.");
}
@Test
@@ -135,7 +142,8 @@
for (int i = 0; i < 3; i++) {
stats.incrementActionCount();
}
- assertThat(stats.getSummary()).isEqualTo("3 processes: 3 abc.");
+ assertThat(SpawnStats.convertSummaryToString(stats.getSummary()))
+ .isEqualTo("3 processes: 3 abc.");
}
@Test
@@ -151,7 +159,8 @@
for (int i = 0; i < 3; i++) {
stats.incrementActionCount();
}
- assertThat(stats.getSummary()).isEqualTo("3 processes: 2 abc, 1 cde.");
+ assertThat(SpawnStats.convertSummaryToString(stats.getSummary()))
+ .isEqualTo("3 processes: 2 abc, 1 cde.");
}
@Test
@@ -178,13 +187,15 @@
for (int i = 0; i < 12; i++) {
stats.incrementActionCount();
}
- assertThat(stats.getSummary()).isEqualTo("12 processes: 9 abc, 3 cde.");
+ assertThat(SpawnStats.convertSummaryToString(stats.getSummary()))
+ .isEqualTo("12 processes: 9 abc, 3 cde.");
}
@Test
public void onlyInternal() {
stats.incrementActionCount();
- assertThat(stats.getSummary()).isEqualTo("1 process: 1 internal.");
+ assertThat(SpawnStats.convertSummaryToString(stats.getSummary()))
+ .isEqualTo("1 process: 1 internal.");
}
@Test
@@ -201,7 +212,7 @@
for (int i = 0; i < 11; i++) {
stats.incrementActionCount();
}
- assertThat(stats.getSummary())
+ assertThat(SpawnStats.convertSummaryToString(stats.getSummary()))
.isEqualTo("11 processes: 1 remote cache hit, 2 internal, 3 a, 2 b, 1 c, 2 z.");
}
}
