Convert --use_action_cache to a regular option
RELNOTES: Convert --use_action_cache to a regular option
--
PiperOrigin-RevId: 148804881
MOS_MIGRATED_REVID=148804881
diff --git a/src/main/cpp/blaze.cc b/src/main/cpp/blaze.cc
index bfb8e15..4240be5 100644
--- a/src/main/cpp/blaze.cc
+++ b/src/main/cpp/blaze.cc
@@ -482,11 +482,6 @@
} else {
result.push_back("--use_custom_exit_code_on_abrupt_exit=false");
}
- if (globals->options->use_action_cache) {
- result.push_back("--use_action_cache=true");
- } else {
- result.push_back("--use_action_cache=false");
- }
// This is only for Blaze reporting purposes; the real interpretation of the
// jvm flags occurs when we set up the java command line.
diff --git a/src/main/cpp/startup_options.cc b/src/main/cpp/startup_options.cc
index f7786b4..1254cc2 100644
--- a/src/main/cpp/startup_options.cc
+++ b/src/main/cpp/startup_options.cc
@@ -57,8 +57,7 @@
invocation_policy(NULL),
client_debug(false),
use_custom_exit_code_on_abrupt_exit(true),
- java_logging_formatter("java.util.logging.SimpleFormatter"),
- use_action_cache(true) {
+ java_logging_formatter("java.util.logging.SimpleFormatter") {
bool testing = !blaze::GetEnv("TEST_TMPDIR").empty();
if (testing) {
output_root = MakeAbsolute(blaze::GetEnv("TEST_TMPDIR"));
@@ -84,8 +83,7 @@
"write_command_log",
"watchfs",
"client_debug",
- "use_custom_exit_code_on_abrupt_exit",
- "use_action_cache"};
+ "use_custom_exit_code_on_abrupt_exit"};
unary_options = {"output_base", "install_base",
"output_user_root", "host_jvm_profile", "host_javabase",
"host_jvm_args", "bazelrc", "blazerc", "io_nice_level",
@@ -285,12 +283,6 @@
} else if (GetNullaryOption(arg, "--nouse_custom_exit_code_on_abrupt_exit")) {
use_custom_exit_code_on_abrupt_exit = false;
option_sources["use_custom_exit_code_on_abrupt_exit"] = rcfile;
- } else if (GetNullaryOption(arg, "--nouse_action_cache")) {
- use_action_cache = false;
- option_sources["use_action_cache"] = rcfile;
- } else if (GetNullaryOption(arg, "--use_action_cache")) {
- use_action_cache = true;
- option_sources["use_action_cache"] = rcfile;
} else if ((value = GetUnaryOption(
arg, next_arg, "--connect_timeout_secs")) != NULL) {
if (!blaze_util::safe_strto32(value, &connect_timeout_secs) ||
diff --git a/src/main/cpp/startup_options.h b/src/main/cpp/startup_options.h
index cddebe3..7bde6e7 100644
--- a/src/main/cpp/startup_options.h
+++ b/src/main/cpp/startup_options.h
@@ -215,9 +215,6 @@
// Value of the java.util.logging.FileHandler.formatter Java property.
std::string java_logging_formatter;
- // Whether to use the action cache.
- bool use_action_cache;
-
protected:
// Constructor for subclasses only so that site-specific extensions of this
// class can override the product name. The product_name must be the
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java b/src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java
index eac03d9..16f926a 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ActionCacheChecker.java
@@ -13,6 +13,7 @@
// limitations under the License.
package com.google.devtools.build.lib.actions;
+import com.google.auto.value.AutoValue;
import com.google.common.base.Predicate;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
@@ -52,15 +53,42 @@
private final ActionCache actionCache;
private final Predicate<? super Action> executionFilter;
private final ArtifactResolver artifactResolver;
- // True iff --verbose_explanations flag is set.
- private final boolean verboseExplanations;
+ private final CacheConfig cacheConfig;
- public ActionCacheChecker(ActionCache actionCache, ArtifactResolver artifactResolver,
- Predicate<? super Action> executionFilter, boolean verboseExplanations) {
+ /** Cache config parameters for ActionCacheChecker. */
+ @AutoValue
+ public abstract static class CacheConfig {
+ abstract boolean enabled();
+ // True iff --verbose_explanations flag is set.
+ abstract boolean verboseExplanations();
+
+ public static Builder builder() {
+ return new AutoValue_ActionCacheChecker_CacheConfig.Builder();
+ }
+
+ /** Builder for ActionCacheChecker.CacheConfig. */
+ @AutoValue.Builder
+ public abstract static class Builder {
+ public abstract Builder setVerboseExplanations(boolean value);
+
+ public abstract Builder setEnabled(boolean value);
+
+ public abstract CacheConfig build();
+ }
+ }
+
+ public ActionCacheChecker(
+ ActionCache actionCache,
+ ArtifactResolver artifactResolver,
+ Predicate<? super Action> executionFilter,
+ @Nullable CacheConfig cacheConfig) {
this.actionCache = actionCache;
this.executionFilter = executionFilter;
this.artifactResolver = artifactResolver;
- this.verboseExplanations = verboseExplanations;
+ this.cacheConfig =
+ cacheConfig != null
+ ? cacheConfig
+ : CacheConfig.builder().setEnabled(true).setVerboseExplanations(false).build();
}
public boolean isActionExecutionProhibited(Action action) {
@@ -72,6 +100,9 @@
* If yes, returns it - otherwise uses first output file as a key
*/
private ActionCache.Entry getCacheEntry(Action action) {
+ if (!cacheConfig.enabled()) {
+ return null; // ignore existing cache when disabled.
+ }
for (Artifact output : action.getOutputs()) {
ActionCache.Entry entry = actionCache.get(output.getExecPathString());
if (entry != null) {
@@ -114,7 +145,7 @@
private void reportCommand(EventHandler handler, Action action) {
if (handler != null) {
- if (verboseExplanations) {
+ if (cacheConfig.verboseExplanations()) {
String keyDescription = action.describeKey();
reportRebuild(handler, action, keyDescription == null
? "action command has changed"
@@ -128,7 +159,7 @@
private void reportClientEnv(EventHandler handler, Action action, Map<String, String> used) {
if (handler != null) {
- if (verboseExplanations) {
+ if (cacheConfig.verboseExplanations()) {
StringBuilder message = new StringBuilder();
message.append("Effective client environment has changed. Now using\n");
for (Map.Entry<String, String> entry : used.entrySet()) {
@@ -194,6 +225,9 @@
}
return null;
}
+ if (!cacheConfig.enabled()) {
+ return new Token(getKeyString(action));
+ }
Iterable<Artifact> actionInputs = action.getInputs();
// Resolve action inputs from cache, if necessary.
boolean inputsDiscovered = action.inputsDiscovered();
@@ -259,6 +293,10 @@
public void afterExecution(
Action action, Token token, MetadataHandler metadataHandler, Map<String, String> clientEnv)
throws IOException {
+ if (!cacheConfig.enabled()) {
+ // Action cache is disabled, don't generate digests.
+ return;
+ }
Preconditions.checkArgument(token != null);
String key = token.cacheKey;
if (actionCache.get(key) != null) {
@@ -267,11 +305,7 @@
}
Map<String, String> usedClientEnv = computeUsedClientEnv(action, clientEnv);
ActionCache.Entry entry =
- actionCache.newEntry(action.getKey(), usedClientEnv, action.discoversInputs());
- if (entry == null) {
- // Action cache is disabled, don't generate digests.
- return;
- }
+ new ActionCache.Entry(action.getKey(), usedClientEnv, action.discoversInputs());
for (Artifact output : action.getOutputs()) {
// Remove old records from the cache if they used different key.
String execPath = output.getExecPathString();
@@ -370,6 +404,10 @@
*/
protected void checkMiddlemanAction(Action action, EventHandler handler,
MetadataHandler metadataHandler) {
+ if (!cacheConfig.enabled()) {
+ // Action cache is disabled, don't generate digests.
+ return;
+ }
Artifact middleman = action.getPrimaryOutput();
String cacheKey = middleman.getExecPathString();
ActionCache.Entry entry = actionCache.get(cacheKey);
@@ -390,19 +428,12 @@
// Compute the aggregated middleman digest.
// Since we never validate action key for middlemen, we should not store
// it in the cache entry and just use empty string instead.
- entry = actionCache.newEntry("", ImmutableMap.<String, String>of(), false);
- if (entry != null) {
- for (Artifact input : action.getInputs()) {
- entry.addFile(input.getExecPath(), metadataHandler.getMetadataMaybe(input));
- }
+ entry = new ActionCache.Entry("", ImmutableMap.<String, String>of(), false);
+ for (Artifact input : action.getInputs()) {
+ entry.addFile(input.getExecPath(), metadataHandler.getMetadataMaybe(input));
}
}
- // Action cache is disabled, skip the digest.
- if (entry == null) {
- return;
- }
-
metadataHandler.setDigestForVirtualArtifact(middleman, entry.getFileDigest());
if (changed) {
actionCache.put(cacheKey, entry);
diff --git a/src/main/java/com/google/devtools/build/lib/actions/cache/ActionCache.java b/src/main/java/com/google/devtools/build/lib/actions/cache/ActionCache.java
index 0cdec63..1e123e5 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/cache/ActionCache.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/cache/ActionCache.java
@@ -59,13 +59,6 @@
void remove(String key);
/**
- * Constructs an {@code Entry}.
- * @return new {@code Entry} or null if the cache is disabled.
- */
- @Nullable
- Entry newEntry(String key, Map<String, String> usedClientEnv, boolean discoversInputs);
-
- /**
* An entry in the ActionCache that contains all action input and output
* artifact paths and their metadata plus action key itself.
*
diff --git a/src/main/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCache.java b/src/main/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCache.java
index 1be6efa..1dee811 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCache.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/cache/CompactPersistentActionCache.java
@@ -338,11 +338,6 @@
}
}
- @Override
- public Entry newEntry(String key, Map<String, String> usedClientEnv, boolean discoversInputs) {
- return new Entry(key, usedClientEnv, discoversInputs);
- }
-
/**
* @return action data encoded as a byte[] array.
*/
diff --git a/src/main/java/com/google/devtools/build/lib/actions/cache/StubActionCache.java b/src/main/java/com/google/devtools/build/lib/actions/cache/StubActionCache.java
deleted file mode 100644
index f1d8c8b..0000000
--- a/src/main/java/com/google/devtools/build/lib/actions/cache/StubActionCache.java
+++ /dev/null
@@ -1,46 +0,0 @@
-// Copyright 2017 The Bazel Authors. All rights reserved.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.google.devtools.build.lib.actions.cache;
-
-import java.io.PrintStream;
-import java.util.Map;
-
-/** An {@link ActionCache} which does not store entries. */
-public class StubActionCache implements ActionCache {
-
- @Override
- public void put(String key, Entry entry) {}
-
- @Override
- public Entry get(String key) {
- return null;
- }
-
- @Override
- public void remove(String key) {}
-
- @Override
- public long save() {
- return 0;
- }
-
- @Override
- public void dump(PrintStream out) {}
-
- @Override
- public Entry newEntry(String key, Map<String, String> usedClientEnv, boolean discoversInputs) {
- return null;
- }
-}
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequest.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequest.java
index 5252ff6..190a362 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequest.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/BuildRequest.java
@@ -37,7 +37,6 @@
import com.google.devtools.common.options.OptionsBase;
import com.google.devtools.common.options.OptionsClassProvider;
import com.google.devtools.common.options.OptionsProvider;
-
import java.util.ArrayList;
import java.util.List;
import java.util.UUID;
@@ -265,6 +264,14 @@
public String getSymlinkPrefix(String productName) {
return symlinkPrefix == null ? productName + "-" : symlinkPrefix;
}
+
+ @Option(
+ name = "use_action_cache",
+ defaultValue = "true",
+ category = "undocumented",
+ help = "Whether to use the action cache"
+ )
+ public boolean useActionCache;
}
/**
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java b/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java
index 48bc11c..51a47ff 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/ExecutionTool.java
@@ -650,7 +650,6 @@
SkyframeExecutor skyframeExecutor,
ModifiedFileSet modifiedOutputFiles) {
BuildRequest.BuildRequestOptions options = request.getBuildOptions();
- boolean verboseExplanations = options.verboseExplanations;
boolean keepGoing = request.getViewOptions().keepGoing;
Path actionOutputRoot = env.getDirectories().getActionConsoleOutputDirectory();
@@ -663,12 +662,24 @@
skyframeExecutor.setActionOutputRoot(actionOutputRoot);
ArtifactFactory artifactFactory = env.getSkyframeBuildView().getArtifactFactory();
- return new SkyframeBuilder(skyframeExecutor,
- new ActionCacheChecker(actionCache, artifactFactory, executionFilter, verboseExplanations),
- keepGoing, actualJobs,
+ return new SkyframeBuilder(
+ skyframeExecutor,
+ new ActionCacheChecker(
+ actionCache,
+ artifactFactory,
+ executionFilter,
+ ActionCacheChecker.CacheConfig.builder()
+ .setEnabled(options.useActionCache)
+ .setVerboseExplanations(options.verboseExplanations)
+ .build()),
+ keepGoing,
+ actualJobs,
request.getPackageCacheOptions().checkOutputFiles
- ? modifiedOutputFiles : ModifiedFileSet.NOTHING_MODIFIED,
- options.finalizeActions, fileCache, request.getBuildOptions().progressReportInterval);
+ ? modifiedOutputFiles
+ : ModifiedFileSet.NOTHING_MODIFIED,
+ options.finalizeActions,
+ fileCache,
+ request.getBuildOptions().progressReportInterval);
}
private void configureResourceManager(BuildRequest request) {
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeServerStartupOptions.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeServerStartupOptions.java
index 3f6bcf1..fe4fe8c 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeServerStartupOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeServerStartupOptions.java
@@ -298,12 +298,4 @@
+ "Please do not use this flag."
)
public boolean useCustomExitCodeOnAbruptExit;
-
- @Option(
- name = "use_action_cache",
- defaultValue = "true",
- category = "server startup",
- help = "Whether to use the action cache"
- )
- public boolean useActionCache;
}
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeWorkspace.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeWorkspace.java
index e2d08b1..98c56c4 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeWorkspace.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeWorkspace.java
@@ -21,7 +21,6 @@
import com.google.common.eventbus.SubscriberExceptionHandler;
import com.google.devtools.build.lib.actions.cache.ActionCache;
import com.google.devtools.build.lib.actions.cache.CompactPersistentActionCache;
-import com.google.devtools.build.lib.actions.cache.StubActionCache;
import com.google.devtools.build.lib.analysis.BlazeDirectories;
import com.google.devtools.build.lib.analysis.WorkspaceStatusAction;
import com.google.devtools.build.lib.analysis.config.BinTools;
@@ -220,16 +219,6 @@
*/
public ActionCache getPersistentActionCache(Reporter reporter) throws IOException {
if (actionCache == null) {
- boolean useActionCache =
- runtime
- .getStartupOptionsProvider()
- .getOptions(BlazeServerStartupOptions.class)
- .useActionCache;
- if (!useActionCache) {
- reporter.handle(Event.info("Action cache disabled"));
- actionCache = new StubActionCache();
- return actionCache;
- }
try (AutoProfiler p = profiledAndLogged("Loading action cache", ProfilerTask.INFO, LOG)) {
try {
actionCache = new CompactPersistentActionCache(getCacheDirectory(), runtime.getClock());
diff --git a/src/test/java/com/google/devtools/build/lib/actions/util/ActionCacheTestHelper.java b/src/test/java/com/google/devtools/build/lib/actions/util/ActionCacheTestHelper.java
index 59f14d1..5ade36e 100644
--- a/src/test/java/com/google/devtools/build/lib/actions/util/ActionCacheTestHelper.java
+++ b/src/test/java/com/google/devtools/build/lib/actions/util/ActionCacheTestHelper.java
@@ -14,9 +14,7 @@
package com.google.devtools.build.lib.actions.util;
import com.google.devtools.build.lib.actions.cache.ActionCache;
-import com.google.devtools.build.lib.actions.cache.ActionCache.Entry;
import java.io.PrintStream;
-import java.util.Map;
/**
* Utilities for tests that use the action cache.
@@ -45,10 +43,5 @@
@Override
public void dump(PrintStream out) {}
-
- @Override
- public Entry newEntry(String key, Map<String, String> env, boolean discoversInputs) {
- return new Entry(key, env, discoversInputs);
- }
};
}
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java b/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java
index ea4d53d..cccb456 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/TimestampBuilderTestCase.java
@@ -243,7 +243,7 @@
executor,
keepGoing, /*explain=*/
false,
- new ActionCacheChecker(actionCache, null, ALWAYS_EXECUTE_FILTER, false),
+ new ActionCacheChecker(actionCache, null, ALWAYS_EXECUTE_FILTER, null),
null);
List<SkyKey> keys = new ArrayList<>();
@@ -438,11 +438,6 @@
public void dump(PrintStream out) {
out.println("In-memory action cache has " + actionCache.size() + " records");
}
-
- @Override
- public Entry newEntry(String key, Map<String, String> usedClientEnv, boolean discoversInputs) {
- return new Entry(key, usedClientEnv, discoversInputs);
- }
}
private static class SingletonActionLookupKey extends ActionLookupValue.ActionLookupKey {
