Skip to content

Commit

Permalink
Remove SpawnRunner.execAsync(), and with it, FutureSpawn.
Browse files Browse the repository at this point in the history 
RELNOTES: None.
PiperOrigin-RevId: 491938422
Change-Id: I719bf22932651b9762bde1de932905af5fdd7f5a
  • Loading branch information
@lberki @copybara-github
lberki authored and copybara-github committed Nov 30, 2022
1 parent f61e2e9 commit 0d2b4d6
Show file tree
Hide file tree
Showing 8 changed files with 59 additions and 210 deletions.
128 changes: 0 additions & 128 deletions src/main/java/com/google/devtools/build/lib/actions/FutureSpawn.java
View file

This file was deleted.

2 changes: 1 addition & 1 deletion src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ public ImmutableList<SpawnResult> exec(
Instant startTime =
Instant.ofEpochMilli(actionExecutionContext.getClock().currentTimeMillis());
// Actual execution.
spawnResult = spawnRunner.execAsync(spawn, context).get();
spawnResult = spawnRunner.exec(spawn, context);
actionExecutionContext
.getEventHandler()
.post(new SpawnExecutedEvent(spawn, spawnResult, startTime));
Expand Down
19 changes: 0 additions & 19 deletions src/main/java/com/google/devtools/build/lib/exec/SpawnRunner.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import com.google.devtools.build.lib.actions.ArtifactPathResolver;
import com.google.devtools.build.lib.actions.ExecException;
import com.google.devtools.build.lib.actions.ForbiddenActionInputException;
import com.google.devtools.build.lib.actions.FutureSpawn;
import com.google.devtools.build.lib.actions.LostInputsExecException;
import com.google.devtools.build.lib.actions.MetadataProvider;
import com.google.devtools.build.lib.actions.Spawn;
Expand Down Expand Up @@ -281,24 +280,6 @@ SortedMap<PathFragment, ActionInput> getInputMapping(PathFragment baseDirectory)
FileSystem getActionFileSystem();
}

/**
* Run the given spawn asynchronously. The default implementation is synchronous for migration.
*
* @param spawn the spawn to run
* @param context the spawn execution context
* @return the result from running the spawn
* @throws InterruptedException if the calling thread was interrupted, or if the runner could not
* lock the output files (see {@link SpawnExecutionContext#lockOutputFiles(int, String,
* FileOutErr)})
* @throws IOException if something went wrong reading or writing to the local file system
* @throws ExecException if the request is malformed
*/
default FutureSpawn execAsync(Spawn spawn, SpawnExecutionContext context)
throws InterruptedException, IOException, ExecException, ForbiddenActionInputException {
// TODO(ulfjack): Remove this default implementation. [exec-async]
return FutureSpawn.immediate(exec(spawn, context));
}

/**
* Run the given spawn.
*
Expand Down
64 changes: 30 additions & 34 deletions src/test/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategyTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
import com.google.devtools.build.lib.actions.ActionExecutionContext;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.ArtifactRoot;
import com.google.devtools.build.lib.actions.FutureSpawn;
import com.google.devtools.build.lib.actions.MetadataProvider;
import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.actions.SpawnExecutedEvent;
Expand Down Expand Up @@ -114,16 +113,16 @@ public void testZeroExit() throws Exception {
when(actionExecutionContext.getExecRoot()).thenReturn(execRoot);
SpawnResult spawnResult =
new SpawnResult.Builder().setStatus(Status.SUCCESS).setRunnerName("test").build();
when(spawnRunner.execAsync(any(Spawn.class), any(SpawnExecutionContext.class)))
.thenReturn(FutureSpawn.immediate(spawnResult));
when(spawnRunner.exec(any(Spawn.class), any(SpawnExecutionContext.class)))
.thenReturn(spawnResult);

List<SpawnResult> spawnResults =
new TestedSpawnStrategy(execRoot, spawnRunner).exec(SIMPLE_SPAWN, actionExecutionContext);

assertThat(spawnResults).containsExactly(spawnResult);

// Must only be called exactly once.
verify(spawnRunner).execAsync(any(Spawn.class), any(SpawnExecutionContext.class));
verify(spawnRunner).exec(any(Spawn.class), any(SpawnExecutionContext.class));
}

@Test
Expand All @@ -136,17 +135,17 @@ public void testEventPosting() throws Exception {
doAnswer(
invocation -> {
clock.advanceMillis(1);
return FutureSpawn.immediate(spawnResult);
return spawnResult;
})
.when(spawnRunner)
.execAsync(any(Spawn.class), any(SpawnExecutionContext.class));
.exec(any(Spawn.class), any(SpawnExecutionContext.class));

ImmutableList<SpawnResult> spawnResults =
new TestedSpawnStrategy(execRoot, spawnRunner).exec(SIMPLE_SPAWN, actionExecutionContext);

assertThat(spawnResults).containsExactly(spawnResult);
// Must only be called exactly once.
verify(spawnRunner).execAsync(any(Spawn.class), any(SpawnExecutionContext.class));
verify(spawnRunner).exec(any(Spawn.class), any(SpawnExecutionContext.class));
assertThat(eventHandler.getPosts()).hasSize(1);
SpawnExecutedEvent event = (SpawnExecutedEvent) eventHandler.getPosts().get(0);
assertThat(event.getStartTimeInstant()).isEqualTo(beforeTime);
Expand All @@ -165,8 +164,7 @@ public void testNonZeroExit() throws Exception {
.setFailureDetail(NON_ZERO_EXIT_DETAILS)
.setRunnerName("test")
.build();
when(spawnRunner.execAsync(any(Spawn.class), any(SpawnExecutionContext.class)))
.thenReturn(FutureSpawn.immediate(result));
when(spawnRunner.exec(any(Spawn.class), any(SpawnExecutionContext.class))).thenReturn(result);

SpawnExecException e =
assertThrows(
Expand All @@ -177,7 +175,7 @@ public void testNonZeroExit() throws Exception {
.exec(SIMPLE_SPAWN, actionExecutionContext));
assertThat(e.getSpawnResult()).isSameInstanceAs(result);
// Must only be called exactly once.
verify(spawnRunner).execAsync(any(Spawn.class), any(SpawnExecutionContext.class));
verify(spawnRunner).exec(any(Spawn.class), any(SpawnExecutionContext.class));
}

@Test
Expand All @@ -193,7 +191,7 @@ public void testCacheHit() throws Exception {
List<SpawnResult> spawnResults =
new TestedSpawnStrategy(execRoot, spawnRunner).exec(SIMPLE_SPAWN, actionExecutionContext);
assertThat(spawnResults).containsExactly(spawnResult);
verify(spawnRunner, never()).execAsync(any(Spawn.class), any(SpawnExecutionContext.class));
verify(spawnRunner, never()).exec(any(Spawn.class), any(SpawnExecutionContext.class));
}

@Test
Expand All @@ -208,16 +206,16 @@ public void testCacheMiss() throws Exception {
when(actionExecutionContext.getExecRoot()).thenReturn(execRoot);
SpawnResult spawnResult =
new SpawnResult.Builder().setStatus(Status.SUCCESS).setRunnerName("test").build();
when(spawnRunner.execAsync(any(Spawn.class), any(SpawnExecutionContext.class)))
.thenReturn(FutureSpawn.immediate(spawnResult));
when(spawnRunner.exec(any(Spawn.class), any(SpawnExecutionContext.class)))
.thenReturn(spawnResult);

List<SpawnResult> spawnResults =
new TestedSpawnStrategy(execRoot, spawnRunner).exec(SIMPLE_SPAWN, actionExecutionContext);

assertThat(spawnResults).containsExactly(spawnResult);

// Must only be called exactly once.
verify(spawnRunner).execAsync(any(Spawn.class), any(SpawnExecutionContext.class));
verify(spawnRunner).exec(any(Spawn.class), any(SpawnExecutionContext.class));
verify(entry).store(eq(spawnResult));
}

Expand All @@ -231,16 +229,16 @@ public void testExec_whenLocalCaches_usesNoCache() throws Exception {
when(actionExecutionContext.getExecRoot()).thenReturn(execRoot);
SpawnResult spawnResult =
new SpawnResult.Builder().setStatus(Status.SUCCESS).setRunnerName("test").build();
when(spawnRunner.execAsync(any(Spawn.class), any(SpawnExecutionContext.class)))
.thenReturn(FutureSpawn.immediate(spawnResult));
when(spawnRunner.exec(any(Spawn.class), any(SpawnExecutionContext.class)))
.thenReturn(spawnResult);

List<SpawnResult> spawnResults =
new TestedSpawnStrategy(execRoot, spawnRunner).exec(SIMPLE_SPAWN, actionExecutionContext);

assertThat(spawnResults).containsExactly(spawnResult);

// Must only be called exactly once.
verify(spawnRunner).execAsync(any(Spawn.class), any(SpawnExecutionContext.class));
verify(spawnRunner).exec(any(Spawn.class), any(SpawnExecutionContext.class));
verifyNoInteractions(cache);
}

Expand All @@ -259,8 +257,8 @@ public void testExec_usefulCacheInDynamicExecution() throws Exception {
when(actionExecutionContext.getExecRoot()).thenReturn(execRoot);
SpawnResult spawnResult =
new SpawnResult.Builder().setStatus(Status.SUCCESS).setRunnerName("test").build();
when(spawnRunner.execAsync(any(Spawn.class), any(SpawnExecutionContext.class)))
.thenReturn(FutureSpawn.immediate(spawnResult));
when(spawnRunner.exec(any(Spawn.class), any(SpawnExecutionContext.class)))
.thenReturn(spawnResult);

List<SpawnResult> spawnResults =
new TestedSpawnStrategy(execRoot, spawnRunner)
Expand All @@ -269,7 +267,7 @@ public void testExec_usefulCacheInDynamicExecution() throws Exception {
assertThat(spawnResults).containsExactly(spawnResult);

// Must only be called exactly once.
verify(spawnRunner).execAsync(any(Spawn.class), any(SpawnExecutionContext.class));
verify(spawnRunner).exec(any(Spawn.class), any(SpawnExecutionContext.class));
verify(entry).store(eq(spawnResult));
}

Expand All @@ -284,8 +282,8 @@ public void testExec_nonUsefulCacheInDynamicExecution() throws Exception {
when(actionExecutionContext.getExecRoot()).thenReturn(execRoot);
SpawnResult spawnResult =
new SpawnResult.Builder().setStatus(Status.SUCCESS).setRunnerName("test").build();
when(spawnRunner.execAsync(any(Spawn.class), any(SpawnExecutionContext.class)))
.thenReturn(FutureSpawn.immediate(spawnResult));
when(spawnRunner.exec(any(Spawn.class), any(SpawnExecutionContext.class)))
.thenReturn(spawnResult);

List<SpawnResult> spawnResults =
new TestedSpawnStrategy(execRoot, spawnRunner)
Expand All @@ -294,7 +292,7 @@ public void testExec_nonUsefulCacheInDynamicExecution() throws Exception {
assertThat(spawnResults).containsExactly(spawnResult);

// Must only be called exactly once.
verify(spawnRunner).execAsync(any(Spawn.class), any(SpawnExecutionContext.class));
verify(spawnRunner).exec(any(Spawn.class), any(SpawnExecutionContext.class));
verify(cache).usefulInDynamicExecution();
verifyNoMoreInteractions(cache);
}
Expand All @@ -316,8 +314,7 @@ public void testCacheMissWithNonZeroExit() throws Exception {
.setFailureDetail(NON_ZERO_EXIT_DETAILS)
.setRunnerName("test")
.build();
when(spawnRunner.execAsync(any(Spawn.class), any(SpawnExecutionContext.class)))
.thenReturn(FutureSpawn.immediate(result));
when(spawnRunner.exec(any(Spawn.class), any(SpawnExecutionContext.class))).thenReturn(result);

SpawnExecException e =
assertThrows(
Expand All @@ -328,7 +325,7 @@ public void testCacheMissWithNonZeroExit() throws Exception {
.exec(SIMPLE_SPAWN, actionExecutionContext));
assertThat(e.getSpawnResult()).isSameInstanceAs(result);
// Must only be called exactly once.
verify(spawnRunner).execAsync(any(Spawn.class), any(SpawnExecutionContext.class));
verify(spawnRunner).exec(any(Spawn.class), any(SpawnExecutionContext.class));
verify(entry).store(eq(result));
}

Expand Down Expand Up @@ -525,15 +522,14 @@ private void setUpExecutionContext(ExecutionOptions executionOptions, RemoteOpti
.thenReturn(
new SpawnLogContext(
execRoot, messageOutput, executionOptions, remoteOptions, SyscallCache.NO_CACHE));
when(spawnRunner.execAsync(any(Spawn.class), any(SpawnExecutionContext.class)))
when(spawnRunner.exec(any(Spawn.class), any(SpawnExecutionContext.class)))
.thenReturn(
FutureSpawn.immediate(
new SpawnResult.Builder()
.setStatus(Status.NON_ZERO_EXIT)
.setExitCode(23)
.setFailureDetail(NON_ZERO_EXIT_DETAILS)
.setRunnerName("runner")
.build()));
new SpawnResult.Builder()
.setStatus(Status.NON_ZERO_EXIT)
.setExitCode(23)
.setFailureDetail(NON_ZERO_EXIT_DETAILS)
.setRunnerName("runner")
.build());
when(actionExecutionContext.getMetadataProvider()).thenReturn(mock(MetadataProvider.class));
}

Expand Down
Loading

0 comments on commit 0d2b4d6

Please sign in to comment.