From d1c1388e7af5d725ef98af08570356519b24d338 Mon Sep 17 00:00:00 2001 From: Olivier Notteghem Date: Fri, 3 Apr 2026 16:21:11 -0700 Subject: [PATCH] [Android] Add --experimental_test_content_key to enable test execution avoidance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: Adds --experimental_test_content_key, a new Bazel flag that makes remote test cache hits insensitive to unused dependency changes. Problem Today, when a transitive dependency changes (even one whose classes the test never uses), the TestRunner action's Merkle tree changes and the test re-executes. This causes unnecessary test runs in CI, especially for large targets with many transitive deps — a compile-time ABI change to an unused library invalidates every test that transitively depends on it, even though the test's behavior is completely unchanged. Solution When --experimental_test_content_key is enabled, two things happen atomically for each TestRunner spawn: 1. Content key injection — RemoteExecutionService reads a .content_key file produced by a build aspect (see android-content-key-remote-cache repo). This file contains a SHA-256 hash of only the class bytecodes the test transitively uses (derived from .jdeps files). The hash is appended to the cache salt, becoming the sole discriminator for what the test actually depends on at the class level. 2. Jar omission — SpawnScrubber.withJarOmission() is applied programmatically to exclude first-party .jar inputs from the Merkle tree. External/maven jars (under .../bin/external/) are intentionally kept in the Merkle tree so version bumps are caught naturally. This keeps jar-omission in sync with key injection without requiring an xplat.cfg entry. Atomic gate: both steps are gated on the content key file being present on disk. If the file is absent (aspect not run, --experimental_remote_download_regex not set, stale files from a disabled flag, etc.), jar omission is suppressed and the full Merkle tree acts as the discriminator — no false cache hits. ┌──────────────────────────────────────┬────────────────────────┬────────────┬──────────────────────────────────────┐ │ Scenario │ Jar omission │ CK in salt │ Result │ ├──────────────────────────────────────┼────────────────────────┼────────────┼──────────────────────────────────────┤ │ Flag OFF │ No │ No │ Standard Bazel key (unchanged) │ ├──────────────────────────────────────┼────────────────────────┼────────────┼──────────────────────────────────────┤ │ Flag ON, CK absent │ No │ No │ Full jar Merkle = safe discriminator │ ├──────────────────────────────────────┼────────────────────────┼────────────┼──────────────────────────────────────┤ │ Flag ON, CK present │ Yes (first-party only) │ Yes │ CK-based cache key │ ├──────────────────────────────────────┼────────────────────────┼────────────┼──────────────────────────────────────┤ │ Flag OFF, stale .content_key on disk │ No │ No │ Stale files ignored — safe │ └──────────────────────────────────────┴────────────────────────┴────────────┴──────────────────────────────────────┘ --- .../google/devtools/build/lib/remote/BUILD | 1 + .../lib/remote/RemoteExecutionService.java | 70 +++++++++++++++++-- .../devtools/build/lib/remote/Scrubber.java | 29 ++++++++ .../lib/remote/options/RemoteOptions.java | 13 ++++ 4 files changed, 108 insertions(+), 5 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/remote/BUILD b/src/main/java/com/google/devtools/build/lib/remote/BUILD index e0fc26f51cc5e3..aca6bafcf396a5 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/BUILD +++ b/src/main/java/com/google/devtools/build/lib/remote/BUILD @@ -59,6 +59,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib:runtime", "//src/main/java/com/google/devtools/build/lib:runtime/command_line_path_factory", "//src/main/java/com/google/devtools/build/lib/actions", + "//src/main/java/com/google/devtools/build/lib/cmdline", "//src/main/java/com/google/devtools/build/lib/actions:action_input_helper", "//src/main/java/com/google/devtools/build/lib/actions:action_lookup_data", "//src/main/java/com/google/devtools/build/lib/actions:action_output_directory_helper", diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java index 519905904227f9..4519ef2dfc40ff 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java @@ -65,6 +65,7 @@ import com.google.common.util.concurrent.SettableFuture; import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.actions.ArtifactPathResolver; import com.google.devtools.build.lib.actions.EnvironmentalExecException; import com.google.devtools.build.lib.actions.ExecException; @@ -512,7 +513,8 @@ public MerkleTree uncachedBuildMerkleTreeVisitor( } @Nullable - private static ByteString buildSalt(Spawn spawn, @Nullable SpawnScrubber spawnScrubber, ImmutableMap mnemonicCacheSalts) { + private static ByteString buildSalt( + Spawn spawn, @Nullable SpawnScrubber spawnScrubber, @Nullable String contentKey, ImmutableMap mnemonicCacheSalts) { CacheSalt.Builder saltBuilder = CacheSalt.newBuilder().setMayBeExecutedRemotely(Spawns.mayBeExecutedRemotely(spawn)); @@ -522,9 +524,14 @@ private static ByteString buildSalt(Spawn spawn, @Nullable SpawnScrubber spawnSc saltBuilder.setWorkspace(workspace); } - if (spawnScrubber != null) { - saltBuilder.setScrubSalt( - CacheSalt.ScrubSalt.newBuilder().setSalt(spawnScrubber.getSalt()).build()); + String scrubSalt = spawnScrubber != null ? spawnScrubber.getSalt() : ""; + + if (contentKey != null) { + scrubSalt = scrubSalt + ":ck=" + contentKey; + } + + if (!scrubSalt.isEmpty()) { + saltBuilder.setScrubSalt(CacheSalt.ScrubSalt.newBuilder().setSalt(scrubSalt).build()); } // Add mnemonic-specific salt for targeted cache invalidation. @@ -536,6 +543,45 @@ private static ByteString buildSalt(Spawn spawn, @Nullable SpawnScrubber spawnSc return saltBuilder.build().toByteString(); } + /** + * Reads the content key file for a TestRunner spawn. Derives the path from the spawn's output + * paths: a test log at {@code bazel-out//testlogs///test.log} implies the + * content key at {@code bazel-out//bin//.content_key}. + * + *

Returns null if the file cannot be found or read (e.g. aspect not enabled). + */ + @Nullable + private String readContentKey(Spawn spawn) { + Label label = spawn.getTargetLabel(); + if (label == null) { + return null; + } + String relativeContentKeyPath = + label.getPackageName() + "/" + label.getName() + ".content_key"; + + for (ActionInput output : spawn.getOutputFiles()) { + String path = output.getExecPathString(); + int idx = path.indexOf("/testlogs/"); + if (idx >= 0) { + // e.g. bazel-out/k8-fastbuild/testlogs/foo/bar/baz/test.log + // → bazel-out/k8-fastbuild/bin/foo/bar/baz.content_key + String configPrefix = path.substring(0, idx); // "bazel-out/k8-fastbuild" + Path keyFile = + execRoot.getRelative(configPrefix + "/bin/" + relativeContentKeyPath); + if (keyFile.exists()) { + try { + return new String(keyFile.getInputStream().readAllBytes(), + java.nio.charset.StandardCharsets.UTF_8).trim(); + } catch (java.io.IOException e) { + // Non-fatal: fall back to normal cache key. + } + } + break; + } + } + return null; + } + /** * Semaphore for limiting the concurrent number of Merkle tree input roots we compute and keep in * memory. @@ -653,6 +699,20 @@ public RemoteAction buildRemoteAction(Spawn spawn, SpawnExecutionContext context RemotePathResolver.createMapped(baseRemotePathResolver, execRoot, spawn.getPathMapper()); ToolSignature toolSignature = getToolSignature(spawn, context); SpawnScrubber spawnScrubber = scrubber != null ? scrubber.forSpawn(spawn) : null; + + // For TestRunner actions with --experimental_test_content_key: inject jar omission + // programmatically when the content key is present. This keeps jar-omission in sync with + // key injection without requiring an xplat.cfg entry — when the key is absent (aspect not + // run, file not downloaded, stale files from a disabled flag, etc.), jars remain in the + // Merkle tree and serve as the cache discriminator (safe fallback, no false hits). + String contentKeyForSalt = null; + if (remoteOptions.testContentKey && "TestRunner".equals(spawn.getMnemonic())) { + contentKeyForSalt = readContentKey(spawn); + if (contentKeyForSalt != null && spawnScrubber != null) { + spawnScrubber = spawnScrubber.withJarOmission(); + } + } + final MerkleTree merkleTree = buildInputMerkleTree(spawn, context, toolSignature, spawnScrubber, remotePathResolver); @@ -684,7 +744,7 @@ public RemoteAction buildRemoteAction(Spawn spawn, SpawnExecutionContext context platform, context.getTimeout(), Spawns.mayBeCachedRemotely(spawn), - buildSalt(spawn, spawnScrubber, mnemonicCacheSalts)); + buildSalt(spawn, spawnScrubber, contentKeyForSalt, mnemonicCacheSalts)); ActionKey actionKey = digestUtil.computeActionKey(action); diff --git a/src/main/java/com/google/devtools/build/lib/remote/Scrubber.java b/src/main/java/com/google/devtools/build/lib/remote/Scrubber.java index 7fd399624a8b1f..e06ee7be8865cb 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/Scrubber.java +++ b/src/main/java/com/google/devtools/build/lib/remote/Scrubber.java @@ -138,6 +138,35 @@ private SpawnScrubber(Config.Rule ruleProto) { this.salt = ruleProto.getTransform().getSalt(); } + private SpawnScrubber(SpawnScrubber base, ImmutableList extraOmittedInputPatterns) { + this.mnemonicPattern = base.mnemonicPattern; + this.labelPattern = base.labelPattern; + this.kindPattern = base.kindPattern; + this.matchTools = base.matchTools; + this.omittedInputPatterns = + ImmutableList.builder() + .addAll(base.omittedInputPatterns) + .addAll(extraOmittedInputPatterns) + .build(); + this.omittedInputPatternsExternal = base.omittedInputPatternsExternal; + this.argReplacements = base.argReplacements; + this.omittedEnvVars = base.omittedEnvVars; + this.salt = base.salt; + } + + /** Returns a new SpawnScrubber identical to this one but with first-party .jar inputs omitted. + * + *

External/maven jars (under .../bin/external/) are intentionally kept in the Merkle tree + * so that version bumps are detected naturally without needing the content key to cover them. + * First-party jars are omitted because the content key already captures exactly which + * first-party class bytecodes the test uses. + */ + public SpawnScrubber withJarOmission() { + return new SpawnScrubber( + this, + ImmutableList.of(Pattern.compile(".*/bin/(?!external/).*[.]jar$"))); + } + private String emptyToAll(String s) { return s.isEmpty() ? ".*" : s; } diff --git a/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java b/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java index 8ebee903f8608b..32bb75d357c952 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java +++ b/src/main/java/com/google/devtools/build/lib/remote/options/RemoteOptions.java @@ -86,6 +86,19 @@ public final class RemoteOptions extends CommonRemoteOptions { + "disable TLS.") public String remoteExecutor; + @Option( + name = "experimental_test_content_key", + defaultValue = "false", + documentationCategory = OptionDocumentationCategory.REMOTE, + effectTags = {OptionEffectTag.UNKNOWN}, + help = + "If enabled, TestRunner actions include a per-test content key in the remote cache" + + " salt. The content key (produced by //rules/test:compute_test_key_aspect) hashes" + + " only the class bytecodes transitively used by the test. Combined with jar-omission" + + " scrubbing rules, this makes remote cache hits insensitive to unused dependency" + + " changes, avoiding unnecessary test re-executions.") + public boolean testContentKey; + @Option( name = "experimental_remote_execution_keepalive", defaultValue = "false",