Skip to content

Commit

Permalink
Inline SpawnGccStrategy
Browse files Browse the repository at this point in the history
It is the only remaining implementation of CppCompileActionContext,
which is hereby deleted.

This is in preparation for making C++ action execution async.

Progress on #6394.

PiperOrigin-RevId: 239171395
  • Loading branch information
ulfjack authored and copybara-github committed Mar 19, 2019
1 parent 43c0bb0 commit 0d55507
Show file tree
Hide file tree
Showing 7 changed files with 103 additions and 189 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import com.google.devtools.build.lib.remote.RemoteModule;
import com.google.devtools.build.lib.remote.RemoteOptions;
import com.google.devtools.build.lib.rules.android.WriteAdbArgsActionContext;
import com.google.devtools.build.lib.rules.cpp.CppCompileActionContext;
import com.google.devtools.build.lib.rules.cpp.CppIncludeExtractionContext;
import com.google.devtools.build.lib.rules.cpp.CppIncludeScanningContext;
import com.google.devtools.build.lib.runtime.BlazeModule;
Expand Down Expand Up @@ -95,7 +94,6 @@ public void executorInit(CommandEnvironment env, BuildRequest request, ExecutorB
}

builder
.addStrategyByContext(CppCompileActionContext.class, "")
.addStrategyByContext(CppIncludeExtractionContext.class, "")
.addStrategyByContext(CppIncludeScanningContext.class, "")
.addStrategyByContext(FileWriteActionContext.class, "")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import com.google.devtools.build.lib.actions.ActionEnvironment;
import com.google.devtools.build.lib.actions.ActionExecutionContext;
import com.google.devtools.build.lib.actions.ActionExecutionException;
import com.google.devtools.build.lib.actions.ActionInput;
import com.google.devtools.build.lib.actions.ActionKeyContext;
import com.google.devtools.build.lib.actions.ActionLookupData;
import com.google.devtools.build.lib.actions.ActionLookupValue;
Expand All @@ -39,11 +40,15 @@
import com.google.devtools.build.lib.actions.CommandAction;
import com.google.devtools.build.lib.actions.CommandLineExpansionException;
import com.google.devtools.build.lib.actions.CommandLines.ParamFileActionInput;
import com.google.devtools.build.lib.actions.EnvironmentalExecException;
import com.google.devtools.build.lib.actions.ExecException;
import com.google.devtools.build.lib.actions.ExecutionInfoSpecifier;
import com.google.devtools.build.lib.actions.ExecutionRequirements;
import com.google.devtools.build.lib.actions.ParameterFile.ParameterFileType;
import com.google.devtools.build.lib.actions.ResourceSet;
import com.google.devtools.build.lib.actions.SimpleSpawn;
import com.google.devtools.build.lib.actions.Spawn;
import com.google.devtools.build.lib.actions.SpawnActionContext;
import com.google.devtools.build.lib.actions.SpawnResult;
import com.google.devtools.build.lib.actions.extra.CppCompileInfo;
import com.google.devtools.build.lib.actions.extra.EnvironmentVariable;
Expand All @@ -59,7 +64,7 @@
import com.google.devtools.build.lib.profiler.SilentCloseable;
import com.google.devtools.build.lib.rules.cpp.CcCommon.CoptsFilter;
import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration;
import com.google.devtools.build.lib.rules.cpp.CppCompileActionContext.Reply;
import com.google.devtools.build.lib.rules.cpp.CppCompileActionResult.Reply;
import com.google.devtools.build.lib.rules.cpp.IncludeScanner.IncludeScanningHeaderData;
import com.google.devtools.build.lib.skyframe.ActionExecutionValue;
import com.google.devtools.build.lib.util.DependencySet;
Expand Down Expand Up @@ -1132,8 +1137,6 @@ public ActionResult execute(ActionExecutionContext actionExecutionContext)
StandardCharsets.ISO_8859_1);
}

CppCompileActionContext.Reply reply;

if (!shouldScanDotdFiles()) {
updateActionInputs(NestedSetBuilder.wrap(Order.STABLE_ORDER, additionalInputs));
}
Expand Down Expand Up @@ -1185,12 +1188,10 @@ public void close() throws IOException {
}
}

CppCompileActionResult.Reply reply;
List<SpawnResult> spawnResults;
try (Defer ignored = new Defer()) {
CppCompileActionResult cppCompileActionResult =
actionExecutionContext
.getContext(CppCompileActionContext.class)
.execWithReply(this, spawnContext);
CppCompileActionResult cppCompileActionResult = execWithReply(spawnContext);
reply = cppCompileActionResult.contextReply();
spawnResults = cppCompileActionResult.spawnResults();
} catch (ExecException e) {
Expand Down Expand Up @@ -1250,6 +1251,77 @@ public void close() throws IOException {
return ActionResult.create(spawnResults);
}

protected CppCompileActionResult execWithReply(ActionExecutionContext actionExecutionContext)
throws ExecException, InterruptedException {
// Intentionally not adding {@link CppCompileAction#inputsForInvalidation}, those are not needed
// for execution.
ImmutableList.Builder<ActionInput> inputsBuilder =
new ImmutableList.Builder<ActionInput>()
.addAll(getMandatoryInputs())
.addAll(getAdditionalInputs());
if (getParamFileActionInput() != null) {
inputsBuilder.add(getParamFileActionInput());
}
ImmutableList<ActionInput> inputs = inputsBuilder.build();
clearAdditionalInputs();

ImmutableMap<String, String> executionInfo = getExecutionInfo();
ImmutableList.Builder<ActionInput> outputs =
ImmutableList.builderWithExpectedSize(getOutputs().size() + 1);
outputs.addAll(getOutputs());
if (getDotdFile() != null && useInMemoryDotdFiles()) {
outputs.add(getDotdFile());
/*
* CppCompileAction does dotd file scanning locally inside the Bazel process and thus
* requires the dotd file contents to be available locally. In remote execution, we
* generally don't want to stage all remote outputs on the local file system and thus
* we need to tell the remote strategy (if any) to at least make the .d file available
* in-memory. We can do that via
* {@link ExecutionRequirements.REMOTE_EXECUTION_INLINE_OUTPUTS}.
*/
executionInfo =
ImmutableMap.<String, String>builderWithExpectedSize(executionInfo.size() + 1)
.putAll(executionInfo)
.put(
ExecutionRequirements.REMOTE_EXECUTION_INLINE_OUTPUTS,
getDotdFile().getExecPathString())
.build();
}

Spawn spawn =
new SimpleSpawn(
this,
ImmutableList.copyOf(getArguments()),
getEnvironment(actionExecutionContext.getClientEnv()),
executionInfo,
inputs,
outputs.build(),
estimateResourceConsumptionLocal());

SpawnActionContext context = actionExecutionContext.getContext(SpawnActionContext.class);
List<SpawnResult> spawnResults = context.exec(spawn, actionExecutionContext);
// The SpawnActionContext guarantees that the first list entry is the successful one.
SpawnResult spawnResult = spawnResults.get(0);

CppCompileActionResult.Builder cppCompileActionResultBuilder =
CppCompileActionResult.builder().setSpawnResults(spawnResults);

if (getDotdFile() != null) {
InputStream in = spawnResult.getInMemoryOutput(getDotdFile());
if (in != null) {
byte[] contents;
try {
contents = ByteStreams.toByteArray(in);
} catch (IOException e) {
throw new EnvironmentalExecException("Reading in-memory .d file failed", e);
}
cppCompileActionResultBuilder.setContextReply(
new CppCompileActionResult.InMemoryFile(contents));
}
}
return cppCompileActionResultBuilder.build();
}

@VisibleForTesting
public NestedSet<Artifact> discoverInputsFromShowIncludesFilters(
Path execRoot,
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,32 @@
import com.google.auto.value.AutoValue;
import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.actions.SpawnResult;
import java.io.IOException;
import java.util.List;
import javax.annotation.Nullable;

/** Contains information about the result of a CppCompileAction's execution. */
@AutoValue
public abstract class CppCompileActionResult {
/** Reply for the execution of a C++ compilation. */
public interface Reply {
/** Returns the contents of the .d file. */
byte[] getContents() throws IOException;
}

/** A simple in-memory implementation of Reply. */
public static class InMemoryFile implements Reply {
private final byte[] contents;

public InMemoryFile(byte[] contents) {
this.contents = contents;
}

@Override
public byte[] getContents() {
return contents;
}
}

/** Returns the SpawnResults created by the action, if any. */
public abstract List<SpawnResult> spawnResults();
Expand All @@ -34,7 +54,7 @@ public abstract class CppCompileActionResult {
* statements are actually required.)
*/
@Nullable
public abstract CppCompileActionContext.Reply contextReply();
public abstract Reply contextReply();

/** Returns a builder that can be used to construct a {@link CppCompileActionResult} object. */
public static Builder builder() {
Expand All @@ -52,7 +72,7 @@ public abstract static class Builder {
public abstract Builder setSpawnResults(List<SpawnResult> spawnResults);

/** Sets the CppCompileActionContext.Reply for the action. */
public abstract Builder setContextReply(CppCompileActionContext.Reply reply);
public abstract Builder setContextReply(Reply reply);

abstract CppCompileActionResult realBuild();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,12 +131,9 @@ public ActionResult execute(ActionExecutionContext actionExecutionContext)
// First, do a normal compilation, to generate the ".d" file. The generated object file is built
// to a temporary location (tempOutputFile) and ignored afterwards.
logger.info("Generating " + getDotdFile());
CppCompileActionContext context =
actionExecutionContext.getContext(CppCompileActionContext.class);
CppCompileActionContext.Reply reply = null;
CppCompileActionResult.Reply reply = null;
try {
CppCompileActionResult cppCompileActionResult =
context.execWithReply(this, actionExecutionContext);
CppCompileActionResult cppCompileActionResult = execWithReply(actionExecutionContext);
reply = cppCompileActionResult.contextReply();
spawnResults = cppCompileActionResult.spawnResults();
} catch (ExecException e) {
Expand Down

This file was deleted.

Loading

0 comments on commit 0d55507

Please sign in to comment.