Collect action cache hits, misses, and reasons for the misses.
As a bonus, this brings in a bunch of new unit tests for the
ActionCacheChecker.
RELNOTES: None.
PiperOrigin-RevId: 170059577
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 1a6c4c8..fc64c75 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
@@ -24,6 +24,7 @@
import com.google.devtools.build.lib.actions.cache.DigestUtils;
import com.google.devtools.build.lib.actions.cache.Metadata;
import com.google.devtools.build.lib.actions.cache.MetadataHandler;
+import com.google.devtools.build.lib.actions.cache.Protos.ActionCacheStatistics.MissReason;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.events.EventKind;
@@ -285,32 +286,38 @@
if (unconditionalExecution(action)) {
Preconditions.checkState(action.isVolatile());
reportUnconditionalExecution(handler, action);
- return true; // must execute - unconditional execution is requested.
+ actionCache.accountMiss(MissReason.UNCONDITIONAL_EXECUTION);
+ return true;
}
if (entry == null) {
reportNewAction(handler, action);
- return true; // must execute -- no cache entry (e.g. first build)
+ actionCache.accountMiss(MissReason.NOT_CACHED);
+ return true;
}
if (entry.isCorrupted()) {
reportCorruptedCacheEntry(handler, action);
- return true; // cache entry is corrupted - must execute
+ actionCache.accountMiss(MissReason.CORRUPTED_CACHE_ENTRY);
+ return true;
} else if (validateArtifacts(entry, action, actionInputs, metadataHandler, true)) {
reportChanged(handler, action);
- return true; // files have changed
+ actionCache.accountMiss(MissReason.DIFFERENT_FILES);
+ return true;
} else if (!entry.getActionKey().equals(action.getKey())) {
reportCommand(handler, action);
- return true; // must execute -- action key is different
+ actionCache.accountMiss(MissReason.DIFFERENT_ACTION_KEY);
+ return true;
}
Map<String, String> usedClientEnv = computeUsedClientEnv(action, clientEnv);
if (!entry.getUsedClientEnvDigest().equals(DigestUtils.fromEnv(usedClientEnv))) {
reportClientEnv(handler, action, usedClientEnv);
- return true; // different values taken from the environment -- must execute
+ actionCache.accountMiss(MissReason.DIFFERENT_ENVIRONMENT);
+ return true;
}
-
entry.getFileDigest();
- return false; // cache hit
+ actionCache.accountHit();
+ return false;
}
private static Metadata getMetadataOrConstant(MetadataHandler metadataHandler, Artifact artifact)
@@ -460,13 +467,16 @@
if (entry != null) {
if (entry.isCorrupted()) {
reportCorruptedCacheEntry(handler, action);
+ actionCache.accountMiss(MissReason.CORRUPTED_CACHE_ENTRY);
changed = true;
} else if (validateArtifacts(entry, action, action.getInputs(), metadataHandler, false)) {
reportChanged(handler, action);
+ actionCache.accountMiss(MissReason.DIFFERENT_FILES);
changed = true;
}
} else {
reportChangedDeps(handler, action);
+ actionCache.accountMiss(MissReason.DIFFERENT_DEPS);
changed = true;
}
if (changed) {
@@ -482,6 +492,8 @@
metadataHandler.setDigestForVirtualArtifact(middleman, entry.getFileDigest());
if (changed) {
actionCache.put(cacheKey, entry);
+ } else {
+ actionCache.accountHit();
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/actions/BUILD b/src/main/java/com/google/devtools/build/lib/actions/BUILD
index db44f36..f481e5f 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/actions/BUILD
@@ -35,6 +35,7 @@
"//src/main/java/com/google/devtools/build/skyframe",
"//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
"//src/main/java/com/google/devtools/common/options",
+ "//src/main/protobuf:action_cache_java_proto",
"//src/main/protobuf:extra_actions_base_java_proto",
"//third_party:auto_value",
"//third_party:guava",
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 59eb31f..fd1756a0 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
@@ -15,7 +15,10 @@
package com.google.devtools.build.lib.actions.cache;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Lists;
+import com.google.devtools.build.lib.actions.cache.Protos.ActionCacheStatistics;
+import com.google.devtools.build.lib.actions.cache.Protos.ActionCacheStatistics.MissReason;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadCompatible;
import com.google.devtools.build.lib.util.Preconditions;
import com.google.devtools.build.lib.vfs.PathFragment;
@@ -67,6 +70,10 @@
* will continue to return same result regardless of internal data transformations).
*/
final class Entry {
+ /** Unique instance to represent a corrupted cache entry. */
+ public static final ActionCache.Entry CORRUPTED =
+ new ActionCache.Entry(null, ImmutableMap.<String, String>of(), false);
+
private final String actionKey;
@Nullable
// Null iff the corresponding action does not do input discovery.
@@ -141,7 +148,7 @@
* Returns true if this cache entry is corrupted and should be ignored.
*/
public boolean isCorrupted() {
- return actionKey == null;
+ return this == CORRUPTED;
}
/**
@@ -194,4 +201,22 @@
* Dumps action cache content into the given PrintStream.
*/
void dump(PrintStream out);
+
+ /** Accounts one cache hit. */
+ void accountHit();
+
+ /** Accounts one cache miss for the given reason. */
+ void accountMiss(MissReason reason);
+
+ /**
+ * Populates the given builder with statistics.
+ *
+ * <p>The extracted values are not guaranteed to be a consistent snapshot of the metrics tracked
+ * by the action cache. Therefore, even if it is safe to call this function at any point in time,
+ * this should only be called once there are no actions running.
+ */
+ void mergeIntoActionCacheStatistics(ActionCacheStatistics.Builder builder);
+
+ /** Resets the current statistics to zero. */
+ void resetStatistics();
}
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 70015af..52cca10 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
@@ -16,7 +16,8 @@
import static java.nio.charset.StandardCharsets.ISO_8859_1;
import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
+import com.google.devtools.build.lib.actions.cache.Protos.ActionCacheStatistics;
+import com.google.devtools.build.lib.actions.cache.Protos.ActionCacheStatistics.MissReason;
import com.google.devtools.build.lib.clock.Clock;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ConditionallyThreadSafe;
import com.google.devtools.build.lib.profiler.AutoProfiler;
@@ -35,9 +36,11 @@
import java.nio.BufferUnderflowException;
import java.nio.ByteBuffer;
import java.util.Collection;
+import java.util.EnumMap;
import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
import java.util.logging.Logger;
/**
@@ -154,8 +157,9 @@
private final PersistentMap<Integer, byte[]> map;
private final PersistentStringIndexer indexer;
- static final ActionCache.Entry CORRUPTED =
- new ActionCache.Entry(null, ImmutableMap.<String, String>of(), false);
+
+ private final AtomicInteger hits = new AtomicInteger();
+ private final Map<MissReason, AtomicInteger> misses = new EnumMap<>(MissReason.class);
public CompactPersistentActionCache(Path cacheRoot, Clock clock) throws IOException {
Path cacheFile = cacheFile(cacheRoot);
@@ -187,6 +191,15 @@
throw new IOException("Failed action cache referential integrity check: " + integrityError);
}
}
+
+ for (MissReason reason : MissReason.values()) {
+ if (reason == MissReason.UNRECOGNIZED) {
+ // The presence of this enum value is a protobuf artifact and confuses our metrics
+ // externalization code below. Just skip it.
+ continue;
+ }
+ misses.put(reason, new AtomicInteger(0));
+ }
}
/**
@@ -254,7 +267,7 @@
return data != null ? CompactPersistentActionCache.decode(indexer, data) : null;
} catch (IOException e) {
// return entry marked as corrupted.
- return CORRUPTED;
+ return ActionCache.Entry.CORRUPTED;
}
}
@@ -420,7 +433,7 @@
if (source.remaining() > 0) {
throw new IOException("serialized entry data has not been fully decoded");
}
- return new Entry(
+ return new ActionCache.Entry(
actionKey,
usedClientEnvDigest,
count == NO_INPUT_DISCOVERY_COUNT ? null : builder.build(),
@@ -429,4 +442,38 @@
throw new IOException("encoded entry data is incomplete", e);
}
}
+
+ @Override
+ public void accountHit() {
+ hits.incrementAndGet();
+ }
+
+ @Override
+ public void accountMiss(MissReason reason) {
+ AtomicInteger counter = misses.get(reason);
+ Preconditions.checkNotNull(counter, "Miss reason %s was not registered in the misses map "
+ + "during cache construction", reason);
+ counter.incrementAndGet();
+ }
+
+ @Override
+ public void mergeIntoActionCacheStatistics(ActionCacheStatistics.Builder builder) {
+ builder.setHits(hits.get());
+
+ int totalMisses = 0;
+ for (Map.Entry<MissReason, AtomicInteger> entry : misses.entrySet()) {
+ int count = entry.getValue().get();
+ builder.addMissDetailsBuilder().setReason(entry.getKey()).setCount(count);
+ totalMisses += count;
+ }
+ builder.setMisses(totalMisses);
+ }
+
+ @Override
+ public void resetStatistics() {
+ hits.set(0);
+ for (Map.Entry<MissReason, AtomicInteger> entry : misses.entrySet()) {
+ entry.getValue().set(0);
+ }
+ }
}
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 1285961..af7659b 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
@@ -380,6 +380,7 @@
request.getBuildOptions().getSymlinkPrefix(productName), productName);
ActionCache actionCache = getActionCache();
+ actionCache.resetStatistics();
SkyframeExecutor skyframeExecutor = env.getSkyframeExecutor();
Builder builder = createBuilder(
request, actionCache, skyframeExecutor, modifiedOutputFiles);
@@ -734,6 +735,7 @@
*/
private void saveActionCache(ActionCache actionCache) {
ActionCacheStatistics.Builder builder = ActionCacheStatistics.newBuilder();
+ actionCache.mergeIntoActionCacheStatistics(builder);
AutoProfiler p =
AutoProfiler.profiledAndLogged("Saving action cache", ProfilerTask.INFO, logger);
