workers: Restart worker processes when their binary has changed since they were launched.

--
MOS_MIGRATED_REVID=107050157

15 files changed, 349 insertions, 104 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java b/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java
index 49c7c15d3d..3a2bddcbff 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java
@@ -54,11 +54,32 @@ public abstract class AbstractAction implements Action, SkylarkValue {
   public static final ResourceSet DEFAULT_RESOURCE_SET =
       ResourceSet.createWithRamCpuIo(250, 0.5, 0);
 
-  // owner/inputs/outputs attributes below should never be directly accessed even
-  // within AbstractAction itself. The appropriate getter methods should be used
-  // instead. This has to be done due to the fact that the getter methods can be
-  // overridden in subclasses.
+  /**
+   * The owner/inputs/outputs attributes below should never be directly accessed even within
+   * AbstractAction itself. The appropriate getter methods should be used instead. This has to be
+   * done due to the fact that the getter methods can be overridden in subclasses.
+   */
   private final ActionOwner owner;
+
+  /**
+   * Tools are a subset of inputs and used by the WorkerSpawnStrategy to determine whether a
+   * compiler has changed since the last time it was used. This should include all artifacts that
+   * the tool does not dynamically reload / check on each unit of work - e.g. its own binary, the
+   * JDK for Java binaries, shared libraries, ... but not a configuration file, if it reloads that
+   * when it has changed.
+   *
+   * <p>If the "tools" set does not contain exactly the right set of artifacts, the following can
+   * happen: If an artifact that should be included is missing, the tool might not be restarted when
+   * it should, and builds can become incorrect (example: The compiler binary is not part of this
+   * set, then the compiler gets upgraded, but the worker strategy still reuses the old version).
+   * If an artifact that should *not* be included is accidentally part of this set, the worker
+   * process will be restarted more often that is necessary - e.g. if a file that is unique to each
+   * unit of work, e.g. the source code that a compiler should compile for a compile action, is
+   * part of this set, then the worker will never be reused and will be restarted for each unit of
+   * work.
+   */
+  private final Iterable<Artifact> tools;
+
   // The variable inputs is non-final only so that actions that discover their inputs can modify it.
   private Iterable<Artifact> inputs;
   private final RunfilesSupplier runfilesSupplier;
@@ -72,16 +93,38 @@ public abstract class AbstractAction implements Action, SkylarkValue {
   protected AbstractAction(ActionOwner owner,
                            Iterable<Artifact> inputs,
                            Iterable<Artifact> outputs) {
-    this(owner, inputs, EmptyRunfilesSupplier.INSTANCE, outputs);
+    this(owner, ImmutableList.<Artifact>of(), inputs, EmptyRunfilesSupplier.INSTANCE, outputs);
   }
 
-  protected AbstractAction(ActionOwner owner,
+  /**
+   * Construct an abstract action with the specified tools, inputs and outputs;
+   */
+  protected AbstractAction(
+      ActionOwner owner,
+      Iterable<Artifact> tools,
+      Iterable<Artifact> inputs,
+      Iterable<Artifact> outputs) {
+    this(owner, tools, inputs, EmptyRunfilesSupplier.INSTANCE, outputs);
+  }
+
+  protected AbstractAction(
+      ActionOwner owner,
+      Iterable<Artifact> inputs,
+      RunfilesSupplier runfilesSupplier,
+      Iterable<Artifact> outputs) {
+    this(owner, ImmutableList.<Artifact>of(), inputs, runfilesSupplier, outputs);
+  }
+
+  protected AbstractAction(
+      ActionOwner owner,
+      Iterable<Artifact> tools,
       Iterable<Artifact> inputs,
       RunfilesSupplier runfilesSupplier,
       Iterable<Artifact> outputs) {
     Preconditions.checkNotNull(owner);
     // TODO(bazel-team): Use RuleContext.actionOwner here instead
     this.owner = new ActionOwnerDescription(owner);
+    this.tools = CollectionUtils.makeImmutable(tools);
     this.inputs = CollectionUtils.makeImmutable(inputs);
     this.outputs = ImmutableSet.copyOf(outputs);
     this.runfilesSupplier = Preconditions.checkNotNull(runfilesSupplier,
@@ -126,6 +169,11 @@ public abstract class AbstractAction implements Action, SkylarkValue {
         "Method must be overridden for actions that may have unknown inputs.");
   }
 
+  @Override
+  public Iterable<Artifact> getTools() {
+    return tools;
+  }
+
   /**
    * Should only be overridden by actions that need to optionally insert inputs. Actions that
    * discover their inputs should use {@link #setInputs} to set the new iterable of inputs when they
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionMetadata.java b/src/main/java/com/google/devtools/build/lib/actions/ActionMetadata.java
index 408b717125..c8c82c86ca 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionMetadata.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionMetadata.java
@@ -95,6 +95,19 @@ public interface ActionMetadata {
   boolean discoversInputs();
 
   /**
+   * Returns the tool Artifacts that this Action depends upon. May be empty. This is a subset of
+   * getInputs().
+   *
+   * <p>This may be used by spawn strategies to determine whether an external tool has not changed
+   * since the last time it was used and could thus be reused, or whether it has to be restarted.
+   *
+   * <p>See {@link AbstractAction#getTools()} for an explanation of why it's important that this
+   * set contains exactly the right set of artifacts in order for the build to stay correct and the
+   * worker strategy to work.
+   */
+  Iterable<Artifact> getTools();
+
+  /**
    * Returns the input Artifacts that this Action depends upon. May be empty.
    *
    * <p>During execution, the {@link Iterable} returned by {@code getInputs} <em>must not</em> be
diff --git a/src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java b/src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java
index 511295c522..0a0955538e 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/BaseSpawn.java
@@ -195,6 +195,11 @@ public class BaseSpawn implements Spawn {
   }
 
   @Override
+  public Iterable<? extends ActionInput> getToolFiles() {
+    return action.getTools();
+  }
+
+  @Override
   public Iterable<? extends ActionInput> getInputFiles() {
     return action.getInputs();
   }
diff --git a/src/main/java/com/google/devtools/build/lib/actions/DelegateSpawn.java b/src/main/java/com/google/devtools/build/lib/actions/DelegateSpawn.java
index 678b05c75c..2c0a0ad5ca 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/DelegateSpawn.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/DelegateSpawn.java
@@ -80,6 +80,11 @@ public class DelegateSpawn implements Spawn {
   }
 
   @Override
+  public Iterable<? extends ActionInput> getToolFiles() {
+    return spawn.getToolFiles();
+  }
+
+  @Override
   public Iterable<? extends ActionInput> getInputFiles() {
     return spawn.getInputFiles();
   }
diff --git a/src/main/java/com/google/devtools/build/lib/actions/Spawn.java b/src/main/java/com/google/devtools/build/lib/actions/Spawn.java
index e4c9399913..d027f70570 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/Spawn.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/Spawn.java
@@ -84,6 +84,21 @@ public interface Spawn {
   ImmutableMap<String, String> getEnvironment();
 
   /**
+   * Returns the list of files that are required to execute this spawn (e.g. the compiler binary),
+   * in contrast to files necessary for the tool to do its work (e.g. source code to be compiled).
+   *
+   * <p>The returned set of files is a subset of what getInputFiles() returns.
+   *
+   * <p>This method explicitly does not expand middleman artifacts. Pass the result
+   * to an appropriate utility method on {@link com.google.devtools.build.lib.actions.Artifact} to
+   * expand the middlemen.
+   *
+   * <p>This is for use with persistent workers, so we can restart workers when their binaries
+   * have changed.
+   */
+  Iterable<? extends ActionInput> getToolFiles();
+
+  /**
    * Returns the list of files that this command may read.
    *
    * <p>This method explicitly does not expand middleman artifacts. Pass the result
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java
index 10c5a8a9e4..895c518c63 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java
@@ -103,6 +103,7 @@ public class SpawnAction extends AbstractAction {
    * All collections provided must not be subsequently modified.
    *
    * @param owner the owner of the Action.
+   * @param tools the set of files comprising the tool that does the work (e.g. compiler).
    * @param inputs the set of all files potentially read by this action; must
    *        not be subsequently modified.
    * @param outputs the set of all files written by this action; must not be
@@ -116,17 +117,30 @@ public class SpawnAction extends AbstractAction {
    * @param progressMessage the message printed during the progression of the build
    * @param mnemonic the mnemonic that is reported in the master log.
    */
-  public SpawnAction(ActionOwner owner,
-      Iterable<Artifact> inputs, Iterable<Artifact> outputs,
+  public SpawnAction(
+      ActionOwner owner,
+      Iterable<Artifact> tools,
+      Iterable<Artifact> inputs,
+      Iterable<Artifact> outputs,
       ResourceSet resourceSet,
       CommandLine argv,
       Map<String, String> environment,
       String progressMessage,
       String mnemonic) {
-    this(owner, inputs, outputs,
-        resourceSet, argv, ImmutableMap.copyOf(environment),
-        ImmutableMap.<String, String>of(), progressMessage,
-        ImmutableMap.<PathFragment, Artifact>of(), mnemonic, false, null);
+    this(
+        owner,
+        tools,
+        inputs,
+        outputs,
+        resourceSet,
+        argv,
+        ImmutableMap.copyOf(environment),
+        ImmutableMap.<String, String>of(),
+        progressMessage,
+        ImmutableMap.<PathFragment, Artifact>of(),
+        mnemonic,
+        false,
+        null);
   }
 
   /**
@@ -135,25 +149,28 @@ public class SpawnAction extends AbstractAction {
    * <p>All collections provided must not be subsequently modified.
    *
    * @param owner the owner of the Action.
-   * @param inputs the set of all files potentially read by this action; must
-   *        not be subsequently modified.
-   * @param outputs the set of all files written by this action; must not be
-   *        subsequently modified.
+   * @param tools the set of files comprising the tool that does the work (e.g. compiler). This is
+   *        a subset of "inputs" and is only used by the WorkerSpawnStrategy.
+   * @param inputs the set of all files potentially read by this action; must not be subsequently
+   *        modified.
+   * @param outputs the set of all files written by this action; must not be subsequently modified.
    * @param resourceSet the resources consumed by executing this Action
    * @param environment the map of environment variables.
    * @param executionInfo out-of-band information for scheduling the spawn.
-   * @param argv the argv array (including argv[0]) of arguments to pass. This
-   *        is merely a list of options to the executable, and is uninterpreted
-   *        by the build tool for the purposes of dependency checking; typically
-   *        it may include the names of input and output files, but this is not
-   *        necessary.
+   * @param argv the argv array (including argv[0]) of arguments to pass. This is merely a list of
+   *        options to the executable, and is uninterpreted by the build tool for the purposes of
+   *        dependency checking; typically it may include the names of input and output files, but
+   *        this is not necessary.
    * @param progressMessage the message printed during the progression of the build
    * @param inputManifests entries in inputs that are symlink manifest files.
    *        These are passed to remote execution in the environment rather than as inputs.
    * @param mnemonic the mnemonic that is reported in the master log.
    */
-  public SpawnAction(ActionOwner owner,
-      Iterable<Artifact> inputs, Iterable<Artifact> outputs,
+  public SpawnAction(
+      ActionOwner owner,
+      Iterable<Artifact> tools,
+      Iterable<Artifact> inputs,
+      Iterable<Artifact> outputs,
       ResourceSet resourceSet,
       CommandLine argv,
       ImmutableMap<String, String> environment,
@@ -163,7 +180,7 @@ public class SpawnAction extends AbstractAction {
       String mnemonic,
       boolean executeUnconditionally,
       ExtraActionInfoSupplier<?> extraActionInfoSupplier) {
-    super(owner, inputs, outputs);
+    super(owner, tools, inputs, outputs);
     this.resourceSet = resourceSet;
     this.executionInfo = executionInfo;
     this.environment = environment;
@@ -276,6 +293,9 @@ public class SpawnAction extends AbstractAction {
     f.addString(GUID);
     f.addStrings(argv.arguments());
     f.addString(getMnemonic());
+    // We don't need the toolManifests here, because they are a subset of the inputManifests by
+    // definition and the output of an action shouldn't change whether something is considered a
+    // tool or not.
     f.addInt(inputManifests.size());
     for (Map.Entry<PathFragment, Artifact> input : inputManifests.entrySet()) {
       f.addString(input.getKey().getPathString() + "/");
@@ -413,7 +433,6 @@ public class SpawnAction extends AbstractAction {
       inputs.removeAll(filesets);
       inputs.removeAll(inputManifests.values());
       return inputs;
-      // Also expand middleman artifacts.
     }
   }
 
@@ -422,9 +441,11 @@ public class SpawnAction extends AbstractAction {
    */
   public static class Builder {
 
+    private final NestedSetBuilder<Artifact> toolsBuilder = NestedSetBuilder.stableOrder();
     private final NestedSetBuilder<Artifact> inputsBuilder =
         NestedSetBuilder.stableOrder();
     private final List<Artifact> outputs = new ArrayList<>();
+    private final Map<PathFragment, Artifact> toolManifests = new LinkedHashMap<>();
     private final Map<PathFragment, Artifact> inputManifests = new LinkedHashMap<>();
     private ResourceSet resourceSet = AbstractAction.DEFAULT_RESOURCE_SET;
     private ImmutableMap<String, String> environment = ImmutableMap.of();
@@ -452,8 +473,10 @@ public class SpawnAction extends AbstractAction {
      * Creates a builder that is a copy of another builder.
      */
     public Builder(Builder other) {
+      this.toolsBuilder.addTransitive(other.toolsBuilder.build());
       this.inputsBuilder.addTransitive(other.inputsBuilder.build());
       this.outputs.addAll(other.outputs);
+      this.toolManifests.putAll(other.toolManifests);
       this.inputManifests.putAll(other.inputManifests);
       this.resourceSet = other.resourceSet;
       this.environment = other.environment;
@@ -521,25 +544,53 @@ public class SpawnAction extends AbstractAction {
       if (paramsFile != null) {
         actualCommandLine = ParamFileHelper.createWithParamsFile(argv, arguments, commandLine,
             isShellCommand, owner, actions, paramFileInfo, paramsFile);
+        inputsBuilder.add(paramsFile);
       } else {
         actualCommandLine = ParamFileHelper.createWithoutParamsFile(argv, arguments, commandLine,
             isShellCommand);
       }
 
-      Iterable<Artifact> actualInputs = collectActualInputs(paramsFile);
-
-      actions.add(0, new SpawnAction(owner, actualInputs, ImmutableList.copyOf(outputs),
-          resourceSet, actualCommandLine, environment, executionInfo, progressMessage,
-          ImmutableMap.copyOf(inputManifests), mnemonic, executeUnconditionally,
-          extraActionInfoSupplier));
+      NestedSet<Artifact> tools = toolsBuilder.build();
+
+      // Tools are by definition a subset of the inputs, so make sure they're present there, too.
+      NestedSet<Artifact> inputsAndTools =
+          NestedSetBuilder.<Artifact>stableOrder()
+              .addTransitive(inputsBuilder.build())
+              .addTransitive(tools)
+              .build();
+
+      LinkedHashMap<PathFragment, Artifact> inputAndToolManifests =
+          new LinkedHashMap<>(inputManifests);
+      inputAndToolManifests.putAll(toolManifests);
+
+      actions.add(
+          0,
+          new SpawnAction(
+              owner,
+              tools,
+              inputsAndTools,
+              ImmutableList.copyOf(outputs),
+              resourceSet,
+              actualCommandLine,
+              environment,
+              executionInfo,
+              progressMessage,
+              ImmutableMap.copyOf(inputAndToolManifests),
+              mnemonic,
+              executeUnconditionally,
+              extraActionInfoSupplier));
       return actions.toArray(new Action[actions.size()]);
     }
 
-    private Iterable<Artifact> collectActualInputs(Artifact parameterFile) {
-      if (parameterFile != null) {
-        inputsBuilder.add(parameterFile);
-      }
-      return inputsBuilder.build();
+    /**
+     * Adds an artifact that is necessary for executing the spawn itself (e.g. a compiler), in
+     * contrast to an artifact that is necessary for the spawn to do its work (e.g. source code).
+     *
+     * <p>The artifact is implicitly added to the inputs of the action as well.
+     */
+    public Builder addTool(Artifact tool) {
+      toolsBuilder.add(tool);
+      return this;
     }
 
     /**
@@ -551,6 +602,14 @@ public class SpawnAction extends AbstractAction {
     }
 
     /**
+     * Adds tools to this action.
+     */
+    public Builder addTools(Iterable<Artifact> artifacts) {
+      toolsBuilder.addAll(artifacts);
+      return this;
+    }
+
+    /**
      * Adds inputs to this action.
      */
     public Builder addInputs(Iterable<Artifact> artifacts) {
@@ -566,6 +625,11 @@ public class SpawnAction extends AbstractAction {
       return this;
     }
 
+    private Builder addToolManifest(Artifact artifact, PathFragment remote) {
+      toolManifests.put(remote, artifact);
+      return this;
+    }
+
     public Builder addInputManifest(Artifact artifact, PathFragment remote) {
       inputManifests.put(remote, artifact);
       return this;
@@ -651,13 +715,13 @@ public class SpawnAction extends AbstractAction {
      * {@link #setShellCommand(String)}.
      */
     public Builder setExecutable(Artifact executable) {
+      addTool(executable);
       return setExecutable(executable.getExecPath());
     }
 
     /**
-     * Sets the executable as a configured target. Automatically adds the files
-     * to run to the inputs and uses the executable of the target as the
-     * executable.
+     * Sets the executable as a configured target. Automatically adds the files to run to the tools
+     * and inputs and uses the executable of the target as the executable.
      *
      * <p>Calling this method overrides any previous values set via calls to
      * {@link #setExecutable(Artifact)}, {@link #setJavaExecutable}, or
@@ -670,13 +734,11 @@ public class SpawnAction extends AbstractAction {
     }
 
     /**
-     * Sets the executable as a configured target. Automatically adds the files
-     * to run to the inputs and uses the executable of the target as the
-     * executable.
+     * Sets the executable as a configured target. Automatically adds the files to run to the tools
+     * and inputs and uses the executable of the target as the executable.
      *
-     * <p>Calling this method overrides any previous values set via calls to
-     * {@link #setExecutable}, {@link #setJavaExecutable}, or
-     * {@link #setShellCommand(String)}.
+     * <p>Calling this method overrides any previous values set via calls to {@link #setExecutable},
+     * {@link #setJavaExecutable}, or {@link #setShellCommand(String)}.
      */
     public Builder setExecutable(FilesToRunProvider executableProvider) {
       Preconditions.checkArgument(executableProvider.getExecutable() != null,
@@ -692,7 +754,7 @@ public class SpawnAction extends AbstractAction {
       executableArgs.add("-Xverify:none");
       executableArgs.addAll(jvmArgs);
       Collections.addAll(executableArgs, launchArgs);
-      inputsBuilder.add(deployJar);
+      toolsBuilder.add(deployJar);
       this.isShellCommand = false;
       return this;
     }
@@ -759,12 +821,15 @@ public class SpawnAction extends AbstractAction {
     }
 
     /**
-     * Adds an executable and its runfiles, so it can be called from a shell command.
+     * Adds an executable and its runfiles, which is necessary for executing the spawn itself (e.g.
+     * a compiler), in contrast to artifacts that are necessary for the spawn to do its work (e.g.
+     * source code).
      */
     public Builder addTool(FilesToRunProvider tool) {
-      addInputs(tool.getFilesToRun());
+      addTools(tool.getFilesToRun());
       if (tool.getRunfilesManifest() != null) {
-        addInputManifest(tool.getRunfilesManifest(),
+        addToolManifest(
+            tool.getRunfilesManifest(),
             BaseSpawn.runfilesForFragment(tool.getExecutable().getExecPath()));
       }
       return this;
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRule.java
index 146fd2588c..c37f3b33a1 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRule.java
@@ -130,10 +130,17 @@ public class GenRule implements RuleConfiguredTargetFactory {
       inputs.add(ruleContext.getAnalysisEnvironment().getVolatileWorkspaceStatusArtifact());
     }
 
-    ruleContext.registerAction(new GenRuleAction(
-        ruleContext.getActionOwner(), inputs.build(), filesToBuild, argv, env,
-        ImmutableMap.copyOf(executionInfo), commandHelper.getRemoteRunfileManifestMap(),
-        message + ' ' + ruleContext.getLabel()));
+    ruleContext.registerAction(
+        new GenRuleAction(
+            ruleContext.getActionOwner(),
+            commandHelper.getResolvedTools(),
+            inputs.build(),
+            filesToBuild,
+            argv,
+            env,
+            ImmutableMap.copyOf(executionInfo),
+            commandHelper.getRemoteRunfileManifestMap(),
+            message + ' ' + ruleContext.getLabel()));
 
     RunfilesProvider runfilesProvider = withData(
         // No runfiles provided if not a data dependency.
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRuleAction.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRuleAction.java
index c9990c4ff6..7481cdcadc 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRuleAction.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRuleAction.java
@@ -37,7 +37,9 @@ public final class GenRuleAction extends SpawnAction {
       // Not chosen scientifically/carefully.  300MB memory, 100% CPU, no I/O.
       ResourceSet.createWithRamCpuIo(300, 1.0, 0.0);
 
-  public GenRuleAction(ActionOwner owner,
+  public GenRuleAction(
+      ActionOwner owner,
+      Iterable<Artifact> tools,
       Iterable<Artifact> inputs,
       Iterable<Artifact> outputs,
       List<String> argv,
@@ -45,10 +47,20 @@ public final class GenRuleAction extends SpawnAction {
       ImmutableMap<String, String> executionInfo,
       ImmutableMap<PathFragment, Artifact> runfilesManifests,
       String progressMessage) {
-    super(owner, inputs, outputs, GENRULE_RESOURCES,
-        CommandLine.of(argv, false), environment, executionInfo, progressMessage,
+    super(
+        owner,
+        tools,
+        inputs,
+        outputs,
+        GENRULE_RESOURCES,
+        CommandLine.of(argv, false),
+        environment,
+        executionInfo,
+        progressMessage,
         runfilesManifests,
-        "Genrule", false, null);
+        "Genrule",
+        false,
+        null);
   }
 
   @Override
diff --git a/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraAction.java b/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraAction.java
index 13cb5b5c56..6bf19d3672 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/extra/ExtraAction.java
@@ -17,6 +17,7 @@ package com.google.devtools.build.lib.rules.extra;
 import com.google.common.base.Preconditions;
 import com.google.common.base.Predicates;
 import com.google.common.collect.Collections2;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.actions.AbstractAction;
@@ -74,6 +75,7 @@ public final class ExtraAction extends SpawnAction {
       String mnemonic) {
     super(
         shadowedAction.getOwner(),
+        ImmutableList.<Artifact>of(),
         createInputs(shadowedAction.getInputs(), extraActionInputs),
         outputs,
         AbstractAction.DEFAULT_RESOURCE_SET,
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java
index 72ab4fb934..23bbcc9514 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java
@@ -167,30 +167,32 @@ public class JavaCompileAction extends AbstractAction {
    *        parts (for example, ide_build_info) need access to the data
    * @param commandLine the actual invocation command line
    */
-  private JavaCompileAction(ActionOwner owner,
-                            Iterable<Artifact> baseInputs,
-                            Collection<Artifact> outputs,
-                            CommandLine javaCompileCommandLine,
-                            CommandLine commandLine,
-                            PathFragment classDirectory,
-                            Artifact outputJar,
-                            NestedSet<Artifact> classpathEntries,
-                            Collection<Artifact> extdirInputs,
-                            List<Artifact> processorPath,
-                            Artifact langtoolsJar,
-                            Artifact javaBuilderJar,
-                            Iterable<Artifact> instrumentationJars,
-                            List<String> processorNames,
-                            Collection<Artifact> messages,
-                            Map<PathFragment, Artifact> resources,
-                            Collection<Artifact> classpathResources,
-                            Collection<Artifact> sourceJars,
-                            Collection<Artifact> sourceFiles,
-                            List<String> javacOpts,
-                            Collection<Artifact> directJars,
-                            BuildConfiguration.StrictDepsMode strictJavaDeps,
-                            Collection<Artifact> compileTimeDependencyArtifacts) {
-    super(owner, NestedSetBuilder.<Artifact>stableOrder()
+  private JavaCompileAction(
+      ActionOwner owner,
+      NestedSet<Artifact> tools,
+      Iterable<Artifact> baseInputs,
+      Collection<Artifact> outputs,
+      CommandLine javaCompileCommandLine,
+      CommandLine commandLine,
+      PathFragment classDirectory,
+      Artifact outputJar,
+      NestedSet<Artifact> classpathEntries,
+      Collection<Artifact> extdirInputs,
+      List<Artifact> processorPath,
+      List<String> processorNames,
+      Collection<Artifact> messages,
+      Map<PathFragment, Artifact> resources,
+      Collection<Artifact> classpathResources,
+      Collection<Artifact> sourceJars,
+      Collection<Artifact> sourceFiles,
+      List<String> javacOpts,
+      Collection<Artifact> directJars,
+      BuildConfiguration.StrictDepsMode strictJavaDeps,
+      Collection<Artifact> compileTimeDependencyArtifacts) {
+    super(
+        owner,
+        tools,
+        NestedSetBuilder.<Artifact>stableOrder()
             .addTransitive(classpathEntries)
             .addAll(processorPath)
             .addAll(messages)
@@ -200,9 +202,7 @@ public class JavaCompileAction extends AbstractAction {
             .addAll(sourceFiles)
             .addAll(compileTimeDependencyArtifacts)
             .addAll(baseInputs)
-            .add(langtoolsJar)
-            .add(javaBuilderJar)
-            .addAll(instrumentationJars)
+            .addTransitive(tools)
             .build(),
         outputs);
     this.javaCompileCommandLine = javaCompileCommandLine;
@@ -888,7 +888,16 @@ public class JavaCompileAction extends AbstractAction {
           semantics.getJavaBuilderMainClass(),
           configuration.getHostPathSeparator());
 
-      return new JavaCompileAction(owner,
+      NestedSet<Artifact> tools =
+          NestedSetBuilder.<Artifact>stableOrder()
+              .add(langtoolsJar)
+              .add(javaBuilderJar)
+              .addAll(instrumentationJars)
+              .build();
+
+      return new JavaCompileAction(
+          owner,
+          tools,
           baseInputs,
           outputs,
           paramFileContents,
@@ -898,9 +907,6 @@ public class JavaCompileAction extends AbstractAction {
           classpathEntries,
           extdirInputs,
           processorPath,
-          langtoolsJar,
-          javaBuilderJar,
-          instrumentationJars,
           processorNames,
           translations,
           resources,
diff --git a/src/main/java/com/google/devtools/build/lib/worker/Worker.java b/src/main/java/com/google/devtools/build/lib/worker/Worker.java
index f239bc4c0c..dcfd09a614 100644
--- a/src/main/java/com/google/devtools/build/lib/worker/Worker.java
+++ b/src/main/java/com/google/devtools/build/lib/worker/Worker.java
@@ -14,6 +14,7 @@
 package com.google.devtools.build.lib.worker;
 
 import com.google.common.base.Preconditions;
+import com.google.common.hash.HashCode;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.Reporter;
 import com.google.devtools.build.lib.vfs.Path;
@@ -40,11 +41,13 @@ final class Worker {
   private final int workerId;
   private final Process process;
   private final Thread shutdownHook;
+  private final HashCode workerFilesHash;
 
-  private Worker(Process process, Thread shutdownHook, int pid) {
+  private Worker(Process process, Thread shutdownHook, int pid, HashCode workerFilesHash) {
     this.process = process;
     this.shutdownHook = shutdownHook;
     this.workerId = pid;
+    this.workerFilesHash = workerFilesHash;
   }
 
   static Worker create(WorkerKey key, Path logDir, Reporter reporter, boolean verbose)
@@ -83,7 +86,7 @@ final class Worker {
                   + logFile));
     }
 
-    return new Worker(process, shutdownHook, workerId);
+    return new Worker(process, shutdownHook, workerId, key.getWorkerFilesHash());
   }
 
   void destroy() {
@@ -125,6 +128,10 @@ final class Worker {
     return this.workerId;
   }
 
+  HashCode getWorkerFilesHash() {
+    return workerFilesHash;
+  }
+
   boolean isAlive() {
     // This is horrible, but Process.isAlive() is only available from Java 8 on and this is the
     // best we can do prior to that.
diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerFactory.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerFactory.java
index b15ca6e736..5deafce1f4 100644
--- a/src/main/java/com/google/devtools/build/lib/worker/WorkerFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerFactory.java
@@ -72,10 +72,12 @@ final class WorkerFactory extends BaseKeyedPooledObjectFactory<WorkerKey, Worker
   }
 
   /**
-   * The worker is considered to be valid when its process is still alive.
+   * The worker is considered to be valid when its files have not changed on disk and its process is
+   * still alive.
    */
   @Override
   public boolean validateObject(WorkerKey key, PooledObject<Worker> p) {
-    return p.getObject().isAlive();
+    Worker worker = p.getObject();
+    return key.getWorkerFilesHash().equals(worker.getWorkerFilesHash()) && worker.isAlive();
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerKey.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerKey.java
index 076b1d4638..86494ea586 100644
--- a/src/main/java/com/google/devtools/build/lib/worker/WorkerKey.java
+++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerKey.java
@@ -16,6 +16,7 @@ package com.google.devtools.build.lib.worker;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.hash.HashCode;
 import com.google.devtools.build.lib.actions.BaseSpawn;
 import com.google.devtools.build.lib.vfs.Path;
 
@@ -32,11 +33,23 @@ final class WorkerKey {
   private final Path workDir;
   private final String mnemonic;
 
-  WorkerKey(List<String> args, Map<String, String> env, Path workDir, String mnemonic) {
+  /**
+   * This is used during validation whether a worker is still usable. It is not used to uniquely
+   * identify a kind of worker, thus it is not to be used by the .equals() / .hashCode() methods.
+   */
+  private final HashCode workerFilesHash;
+
+  WorkerKey(
+      List<String> args,
+      Map<String, String> env,
+      Path workDir,
+      String mnemonic,
+      HashCode workerFilesHash) {
     this.args = ImmutableList.copyOf(Preconditions.checkNotNull(args));
     this.env = ImmutableMap.copyOf(Preconditions.checkNotNull(env));
     this.workDir = Preconditions.checkNotNull(workDir);
     this.mnemonic = Preconditions.checkNotNull(mnemonic);
+    this.workerFilesHash = Preconditions.checkNotNull(workerFilesHash);
   }
 
   public ImmutableList<String> getArgs() {
@@ -55,6 +68,10 @@ final class WorkerKey {
     return mnemonic;
   }
 
+  public HashCode getWorkerFilesHash() {
+    return workerFilesHash;
+  }
+
   @Override
   public boolean equals(Object o) {
     if (this == o) {
diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerPool.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerPool.java
index 47086aee8b..03eb7d6a54 100644
--- a/src/main/java/com/google/devtools/build/lib/worker/WorkerPool.java
+++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerPool.java
@@ -19,9 +19,6 @@ import com.google.devtools.build.lib.vfs.Path;
 import org.apache.commons.pool2.impl.GenericKeyedObjectPool;
 import org.apache.commons.pool2.impl.GenericKeyedObjectPoolConfig;
 
-import java.util.HashSet;
-import java.util.Set;
-
 import javax.annotation.concurrent.ThreadSafe;
 
 /**
@@ -33,7 +30,6 @@ import javax.annotation.concurrent.ThreadSafe;
 @ThreadSafe
 final class WorkerPool extends GenericKeyedObjectPool<WorkerKey, Worker> {
   final WorkerFactory workerFactory;
-  final Set<Worker> workers = new HashSet<>();
 
   public WorkerPool(WorkerFactory factory, GenericKeyedObjectPoolConfig config) {
     super(factory, config);
diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnStrategy.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnStrategy.java
index cf893ea8e2..f143ab6576 100644
--- a/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnStrategy.java
@@ -19,7 +19,12 @@ import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Iterables;
 import com.google.common.eventbus.EventBus;
 import com.google.common.eventbus.Subscribe;
+import com.google.common.hash.HashCode;
+import com.google.common.hash.Hasher;
+import com.google.common.hash.Hashing;
 import com.google.devtools.build.lib.actions.ActionExecutionContext;
+import com.google.devtools.build.lib.actions.ActionInput;
+import com.google.devtools.build.lib.actions.ActionInputFileCache;
 import com.google.devtools.build.lib.actions.ChangedFilesMessage;
 import com.google.devtools.build.lib.actions.ExecException;
 import com.google.devtools.build.lib.actions.ExecutionStrategy;
@@ -40,6 +45,8 @@ import com.google.devtools.build.lib.worker.WorkerProtocol.WorkRequest;
 import com.google.devtools.build.lib.worker.WorkerProtocol.WorkResponse;
 import com.google.devtools.common.options.OptionsClassProvider;
 
+import java.io.IOException;
+import java.nio.charset.Charset;
 import java.util.concurrent.atomic.AtomicBoolean;
 
 /**
@@ -48,9 +55,17 @@ import java.util.concurrent.atomic.AtomicBoolean;
  */
 @ExecutionStrategy(name = { "worker" }, contextType = SpawnActionContext.class)
 final class WorkerSpawnStrategy implements SpawnActionContext {
+  public static final String REASON_HEURISTIC =
+      "Not using worker strategy, because incrementalHeuristic says no";
+  public static final String REASON_NO_FLAGFILE =
+      "Not using worker strategy, because last argument does not contain a @flagfile";
+  public static final String REASON_NO_TOOLS =
+      "Not using worker strategy, because the action has no tools";
+
   private final WorkerPool workers;
   private final IncrementalHeuristic incrementalHeuristic;
   private final StandaloneSpawnStrategy standaloneStrategy;
+  private final WorkerOptions options;
   private final boolean verboseFailures;
   private final int maxRetries;
 
@@ -62,7 +77,7 @@ final class WorkerSpawnStrategy implements SpawnActionContext {
       boolean verboseFailures,
       int maxRetries) {
     Preconditions.checkNotNull(optionsProvider);
-    WorkerOptions options = optionsProvider.getOptions(WorkerOptions.class);
+    this.options = optionsProvider.getOptions(WorkerOptions.class);
     this.incrementalHeuristic = new IncrementalHeuristic(options.workerMaxChangedFiles);
     eventBus.register(incrementalHeuristic);
     this.workers = Preconditions.checkNotNull(workers);
@@ -75,6 +90,8 @@ final class WorkerSpawnStrategy implements SpawnActionContext {
   public void exec(Spawn spawn, ActionExecutionContext actionExecutionContext)
       throws ExecException, InterruptedException {
     Executor executor = actionExecutionContext.getExecutor();
+    EventHandler eventHandler = executor.getEventHandler();
+
     if (executor.reportsSubcommands()) {
       executor.reportSubcommand(
           Label.print(spawn.getOwner().getLabel())
@@ -85,16 +102,29 @@ final class WorkerSpawnStrategy implements SpawnActionContext {
     }
 
     if (!incrementalHeuristic.shouldUseWorkers()) {
+      if (options.workerVerbose) {
+        eventHandler.handle(Event.info(REASON_HEURISTIC));
+      }
       standaloneStrategy.exec(spawn, actionExecutionContext);
       return;
     }
 
-    // We assume that the spawn to be executed always gets a single argument, which is a flagfile
-    // prefixed with @ and that it will start in persistent mode when we don't pass it one.
-    // Thus, we can extract the last element from its args (which will be the flagfile) to start the
-    // persistent mode and then pass it the flagfile via a WorkRequest to make it actually do the
-    // work.
+    // We assume that the spawn to be executed always gets a @flagfile argument, which contains the
+    // flags related to the work itself (as opposed to start-up options for the executed tool).
+    // Thus, we can extract the last element from its args (which will be the @flagfile), expand it
+    // and put that into the WorkRequest instead.
     if (!Iterables.getLast(spawn.getArguments()).startsWith("@")) {
+      if (options.workerVerbose) {
+        eventHandler.handle(Event.info(REASON_NO_FLAGFILE));
+      }
+      standaloneStrategy.exec(spawn, actionExecutionContext);
+      return;
+    }
+
+    if (Iterables.isEmpty(spawn.getToolFiles())) {
+      if (options.workerVerbose) {
+        eventHandler.handle(Event.info(REASON_NO_TOOLS));
+      }
       standaloneStrategy.exec(spawn, actionExecutionContext);
       return;
     }
@@ -108,9 +138,13 @@ final class WorkerSpawnStrategy implements SpawnActionContext {
         .build();
     ImmutableMap<String, String> env = spawn.getEnvironment();
     Path workDir = actionExecutionContext.getExecutor().getExecRoot();
-    WorkerKey key = new WorkerKey(args, env, workDir, spawn.getMnemonic());
 
     try {
+      HashCode workerFilesHash =
+          combineActionInputHashes(
+              spawn.getToolFiles(), actionExecutionContext.getActionInputFileCache());
+      WorkerKey key = new WorkerKey(args, env, workDir, spawn.getMnemonic(), workerFilesHash);
+
       WorkResponse response = execInWorker(executor.getEventHandler(), paramFile, key, maxRetries);
 
       outErr.getErrorStream().write(response.getOutputBytes().toByteArray());
@@ -127,6 +161,17 @@ final class WorkerSpawnStrategy implements SpawnActionContext {
     }
   }
 
+  private HashCode combineActionInputHashes(
+      Iterable<? extends ActionInput> toolFiles, ActionInputFileCache actionInputFileCache)
+      throws IOException {
+    Hasher hasher = Hashing.sha256().newHasher();
+    for (ActionInput tool : toolFiles) {
+      hasher.putString(tool.getExecPathString(), Charset.defaultCharset());
+      hasher.putBytes(actionInputFileCache.getDigest(tool).toByteArray());
+    }
+    return hasher.hash();
+  }
+
   private WorkResponse execInWorker(
       EventHandler eventHandler, String paramFile, WorkerKey key, int retriesLeft)
       throws Exception {