Require explicit ExecKind in SpawnMetrics.Builder and remove SpawnMetrics.EMPTY
Previously we used `REMOTE` as the default but now we'll require setting
`ExecKind` explicitly. To make it less error prone, I've introduced a few
factory methods that set it. Also, as previously discussed, I've introduced
a new `ExecKind.OTHER` (IMHO sounds a bit better than the initial proposal
of `INTERNAL`, but I don't feel strongly, so let me know if you prefer
the latter).
We also discussed not setting `ExecKind` in `SpawnMetrics.EMPTY` (and using
something like `null`), but I found it a bit tricky to really define what
that would actually mean, now that we have `ExecKind` (e.g., should we allow
aggregating such "empty" values? if yes, should it be commutative? etc.)
But I've noticed that after the introduction of `AggregatedSpawnMetrics`,
we only use `EMPTY` in one place and it's possible to simply remove it, which
avoids the problem.
RELNOTES: None.
PiperOrigin-RevId: 314687611
diff --git a/src/test/java/com/google/devtools/build/lib/actions/AggregatedSpawnMetricsTest.java b/src/test/java/com/google/devtools/build/lib/actions/AggregatedSpawnMetricsTest.java
index 34ef58c..5505605 100644
--- a/src/test/java/com/google/devtools/build/lib/actions/AggregatedSpawnMetricsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/actions/AggregatedSpawnMetricsTest.java
@@ -27,8 +27,7 @@
@Test
public void sumAllMetrics() throws Exception {
SpawnMetrics metrics1 =
- new SpawnMetrics.Builder()
- .setExecKind(SpawnMetrics.ExecKind.REMOTE)
+ SpawnMetrics.Builder.forRemoteExec()
.setTotalTime(Duration.ofSeconds(1))
.setExecutionWallTime(Duration.ofSeconds(2))
.setInputBytes(10)
@@ -36,8 +35,7 @@
.setMemoryEstimateBytes(30)
.build();
SpawnMetrics metrics2 =
- new SpawnMetrics.Builder()
- .setExecKind(SpawnMetrics.ExecKind.REMOTE)
+ SpawnMetrics.Builder.forRemoteExec()
.setTotalTime(Duration.ofSeconds(10))
.setExecutionWallTime(Duration.ofSeconds(20))
.setInputBytes(100)
@@ -59,8 +57,7 @@
@Test
public void sumDurationMetricsMaxOther() throws Exception {
SpawnMetrics metrics1 =
- new SpawnMetrics.Builder()
- .setExecKind(SpawnMetrics.ExecKind.REMOTE)
+ SpawnMetrics.Builder.forRemoteExec()
.setTotalTime(Duration.ofSeconds(1))
.setExecutionWallTime(Duration.ofSeconds(2))
.setInputBytes(10)
@@ -68,8 +65,7 @@
.setMemoryEstimateBytes(30)
.build();
SpawnMetrics metrics2 =
- new SpawnMetrics.Builder()
- .setExecKind(SpawnMetrics.ExecKind.REMOTE)
+ SpawnMetrics.Builder.forRemoteExec()
.setTotalTime(Duration.ofSeconds(10))
.setExecutionWallTime(Duration.ofSeconds(20))
.setInputBytes(100)
@@ -91,20 +87,11 @@
@Test
public void aggregatingMetrics_preservesExecKind() throws Exception {
SpawnMetrics metrics1 =
- new SpawnMetrics.Builder()
- .setExecKind(SpawnMetrics.ExecKind.LOCAL)
- .setTotalTime(Duration.ofSeconds(1))
- .build();
+ SpawnMetrics.Builder.forLocalExec().setTotalTime(Duration.ofSeconds(1)).build();
SpawnMetrics metrics2 =
- new SpawnMetrics.Builder()
- .setExecKind(SpawnMetrics.ExecKind.REMOTE)
- .setTotalTime(Duration.ofSeconds(2))
- .build();
+ SpawnMetrics.Builder.forRemoteExec().setTotalTime(Duration.ofSeconds(2)).build();
SpawnMetrics metrics3 =
- new SpawnMetrics.Builder()
- .setExecKind(SpawnMetrics.ExecKind.WORKER)
- .setTotalTime(Duration.ofSeconds(3))
- .build();
+ SpawnMetrics.Builder.forWorkerExec().setTotalTime(Duration.ofSeconds(3)).build();
AggregatedSpawnMetrics aggregated = AggregatedSpawnMetrics.EMPTY;
aggregated = aggregated.sumAllMetrics(metrics1);
@@ -122,21 +109,18 @@
@Test
public void toString_printsOnlyRemote() throws Exception {
SpawnMetrics metrics1 =
- new SpawnMetrics.Builder()
- .setExecKind(SpawnMetrics.ExecKind.LOCAL)
+ SpawnMetrics.Builder.forLocalExec()
.setTotalTime(Duration.ofSeconds(1))
.setExecutionWallTime(Duration.ofSeconds(1))
.build();
SpawnMetrics metrics2 =
- new SpawnMetrics.Builder()
- .setExecKind(SpawnMetrics.ExecKind.REMOTE)
+ SpawnMetrics.Builder.forRemoteExec()
.setTotalTime(Duration.ofSeconds(2))
.setNetworkTime(Duration.ofSeconds(1))
.setExecutionWallTime(Duration.ofSeconds(1))
.build();
SpawnMetrics metrics3 =
- new SpawnMetrics.Builder()
- .setExecKind(SpawnMetrics.ExecKind.WORKER)
+ SpawnMetrics.Builder.forWorkerExec()
.setTotalTime(Duration.ofSeconds(3))
.setQueueTime(Duration.ofSeconds(1))
.setExecutionWallTime(Duration.ofSeconds(2))
diff --git a/src/test/java/com/google/devtools/build/lib/actions/SpawnMetricsTest.java b/src/test/java/com/google/devtools/build/lib/actions/SpawnMetricsTest.java
index 0f3c1ae..75dee98 100644
--- a/src/test/java/com/google/devtools/build/lib/actions/SpawnMetricsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/actions/SpawnMetricsTest.java
@@ -27,8 +27,7 @@
@Test
public void builder_addDurationsNonDurations() throws Exception {
SpawnMetrics metrics1 =
- new SpawnMetrics.Builder()
- .setExecKind(SpawnMetrics.ExecKind.REMOTE)
+ SpawnMetrics.Builder.forRemoteExec()
.setTotalTime(Duration.ofSeconds(1))
.setExecutionWallTime(Duration.ofSeconds(2))
.setInputBytes(10)
@@ -36,8 +35,7 @@
.setMemoryEstimateBytes(30)
.build();
SpawnMetrics metrics2 =
- new SpawnMetrics.Builder()
- .setExecKind(SpawnMetrics.ExecKind.REMOTE)
+ SpawnMetrics.Builder.forRemoteExec()
.setTotalTime(Duration.ofSeconds(10))
.setExecutionWallTime(Duration.ofSeconds(20))
.setInputBytes(100)
@@ -46,8 +44,7 @@
.build();
SpawnMetrics result =
- new SpawnMetrics.Builder()
- .setExecKind(SpawnMetrics.ExecKind.REMOTE)
+ SpawnMetrics.Builder.forRemoteExec()
.addDurations(metrics1)
.addDurations(metrics2)
.addNonDurations(metrics1)
@@ -64,8 +61,7 @@
@Test
public void builder_addDurationsMaxNonDurations() throws Exception {
SpawnMetrics metrics1 =
- new SpawnMetrics.Builder()
- .setExecKind(SpawnMetrics.ExecKind.REMOTE)
+ SpawnMetrics.Builder.forRemoteExec()
.setTotalTime(Duration.ofSeconds(1))
.setExecutionWallTime(Duration.ofSeconds(2))
.setInputBytes(10)
@@ -73,8 +69,7 @@
.setMemoryEstimateBytes(30)
.build();
SpawnMetrics metrics2 =
- new SpawnMetrics.Builder()
- .setExecKind(SpawnMetrics.ExecKind.REMOTE)
+ SpawnMetrics.Builder.forRemoteExec()
.setTotalTime(Duration.ofSeconds(10))
.setExecutionWallTime(Duration.ofSeconds(20))
.setInputBytes(100)
@@ -83,8 +78,7 @@
.build();
SpawnMetrics result =
- new SpawnMetrics.Builder()
- .setExecKind(SpawnMetrics.ExecKind.REMOTE)
+ SpawnMetrics.Builder.forRemoteExec()
.addDurations(metrics1)
.addDurations(metrics2)
.maxNonDurations(metrics1)
diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java
index ad1f45f..aac8abd 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java
@@ -975,7 +975,7 @@
@Test
public void accountingAddsDurationsForStages() {
SpawnMetrics.Builder builder =
- new SpawnMetrics.Builder()
+ SpawnMetrics.Builder.forRemoteExec()
.setQueueTime(Duration.ofSeconds(1))
.setSetupTime(Duration.ofSeconds(2))
.setExecutionWallTime(Duration.ofSeconds(2))
@@ -1026,10 +1026,7 @@
}
private RemoteSpawnRunner newSpawnRunner() {
- return newSpawnRunner(
- executor,
- /* reporter= */ null,
- /* topLevelOutputs= */ ImmutableSet.of());
+ return newSpawnRunner(executor, /* reporter= */ null, /* topLevelOutputs= */ ImmutableSet.of());
}
private RemoteSpawnRunner newSpawnRunner(Reporter reporter) {
