Skip to content

Commit

Permalink
Pass MetadataHandler to ActionInputPrefetcher
Browse files Browse the repository at this point in the history
Also, allow it to respond to interrupts.

Progress towards bazelbuild#6862
  • Loading branch information
buchgr committed Feb 5, 2019
1 parent bac30fe commit 39a15fa
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,15 @@
// limitations under the License.
package com.google.devtools.build.lib.actions;

import java.io.IOException;

/** Prefetches files to local disk. */
public interface ActionInputPrefetcher {
public static final ActionInputPrefetcher NONE =
new ActionInputPrefetcher() {
@Override
public void prefetchFiles(Iterable<? extends ActionInput> input) {
public void prefetchFiles(Iterable<? extends ActionInput> inputs,
MetadataProvider metadataProvider) {
// Do nothing.
}
};
Expand All @@ -28,5 +31,5 @@ public void prefetchFiles(Iterable<? extends ActionInput> input) {
*
* <p>For any path not under this prefetcher's control, the call should be a no-op.
*/
void prefetchFiles(Iterable<? extends ActionInput> input);
void prefetchFiles(Iterable<? extends ActionInput> inputs, MetadataProvider metadataProvider) throws IOException, InterruptedException;
}
Original file line number Diff line number Diff line change
Expand Up @@ -181,11 +181,11 @@ public int getId() {
}

@Override
public void prefetchInputs() throws IOException {
public void prefetchInputs() throws IOException, InterruptedException {
if (Spawns.shouldPrefetchInputsForLocalExecution(spawn)) {
actionExecutionContext
.getActionInputPrefetcher()
.prefetchFiles(getInputMapping(true).values());
.prefetchFiles(getInputMapping(true).values(), getMetadataProvider());
}
}

Expand Down
38 changes: 22 additions & 16 deletions src/main/java/com/google/devtools/build/lib/exec/SpawnRunner.java
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@
// limitations under the License.
package com.google.devtools.build.lib.exec;

import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.actions.ActionInput;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander;
import com.google.devtools.build.lib.actions.ArtifactPathResolver;
import com.google.devtools.build.lib.actions.ExecException;
Expand All @@ -25,6 +27,7 @@
import com.google.devtools.build.lib.vfs.PathFragment;
import java.io.IOException;
import java.time.Duration;
import java.util.Collection;
import java.util.SortedMap;

/**
Expand Down Expand Up @@ -129,6 +132,7 @@ public enum ProgressStatus {
* by different threads, so they MUST not call any shared non-thread-safe objects.
*/
interface SpawnExecutionContext {

/**
* Returns a unique id for this spawn, to be used for logging. Note that a single spawn may be
* passed to multiple {@link SpawnRunner} implementations, so any log entries should also
Expand All @@ -137,25 +141,15 @@ interface SpawnExecutionContext {
int getId();

/**
* Prefetches the Spawns input files to the local machine. There are cases where Bazel runs on a
* network file system, and prefetching the files in parallel is a significant performance win.
* This should only be called by local strategies when local execution is imminent.
*
* <p>Should be called with the equivalent of:
* <code>
* policy.prefetchInputs(
* Iterables.filter(policy.getInputMapping().values(), Predicates.notNull()));
* </code>
* Prefetches the {@link Spawn}'s input files to the local machine.
*
* <p>Note in particular that {@link #getInputMapping} may return {@code null} values, but
* this method does not accept {@code null} values.
* <p>This is used by remote caching / execution to only download remote build outputs to the
* local machine that are strictly required. It's also used as a performance optimization in
* cases when Bazel runs on a network filesystem.
*
* <p>The reason why this method requires passing in the inputs is that getInputMapping may be
* slow to compute, so if the implementation already called it, we don't want to compute it
* again. I suppose we could require implementations to memoize getInputMapping (but not compute
* it eagerly), and that may change in the future.
* <p>This should only be called by local strategies when local execution is imminent.
*/
void prefetchInputs() throws IOException;
void prefetchInputs() throws IOException, InterruptedException;

/**
* The input file metadata cache for this specific spawn, which can be used to efficiently
Expand Down Expand Up @@ -199,6 +193,18 @@ SortedMap<PathFragment, ActionInput> getInputMapping(boolean expandTreeArtifacts

/** Reports a progress update to the Spawn strategy. */
void report(ProgressStatus state, String name);

/**
* Returns the collection of files that this command must write and make available via
* the local {@link com.google.devtools.build.lib.vfs.FileSystem}. The returned output
* artifacts are a subset of the action's output artifacts.
*
* <p>This is for use with remote execution, where as an optimization we don't want to
* download all output files.
*/
default Collection<Artifact> getRequiredLocalOutputs() {
return ImmutableList.of();
}
}

/**
Expand Down

0 comments on commit 39a15fa

Please sign in to comment.