diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionInputPrefetcher.java b/src/main/java/com/google/devtools/build/lib/actions/ActionInputPrefetcher.java index 0435d17eb3c35b..fb2b07c9ae4cde 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ActionInputPrefetcher.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ActionInputPrefetcher.java @@ -28,6 +28,11 @@ public ListenableFuture prefetchFiles( // Do nothing. return immediateVoidFuture(); } + + @Override + public boolean supportsPartialTreeArtifactInputs() { + return true; + } }; /** @@ -39,4 +44,10 @@ public ListenableFuture prefetchFiles( */ ListenableFuture prefetchFiles( Iterable inputs, MetadataProvider metadataProvider); + + /** + * Whether the prefetcher is able to fetch individual files in a tree artifact without fetching + * the entire tree artifact. + */ + boolean supportsPartialTreeArtifactInputs(); } diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcher.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcher.java index 5ae39e9172d9f1..d128e83a6f6bad 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcher.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionInputFetcher.java @@ -61,6 +61,12 @@ class RemoteActionInputFetcher extends AbstractActionInputPrefetcher { this.remoteCache = Preconditions.checkNotNull(remoteCache); } + @Override + public boolean supportsPartialTreeArtifactInputs() { + // This prefetcher is unable to fetch only individual files inside a tree artifact. + return false; + } + @Override protected void prefetchVirtualActionInput(VirtualActionInput input) throws IOException { if (!(input instanceof EmptyActionInput)) { diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java index 952e2dd573c641..4a116c0fc2b482 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionExecutionFunction.java @@ -1253,7 +1253,8 @@ private R accumulateInputs( topLevelFilesets, input, value, - env); + env, + skyframeActionExecutor.supportsPartialTreeArtifactInputs()); } if (actionExecutionFunctionExceptionHandler != null) { diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionInputMapHelper.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionInputMapHelper.java index f024131f12f511..412659253f3ea4 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionInputMapHelper.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionInputMapHelper.java @@ -25,6 +25,7 @@ import com.google.devtools.build.lib.actions.Artifact.ArchivedTreeArtifact; import com.google.devtools.build.lib.actions.Artifact.DerivedArtifact; import com.google.devtools.build.lib.actions.Artifact.SpecialArtifact; +import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact; import com.google.devtools.build.lib.actions.FileArtifactValue; import com.google.devtools.build.lib.actions.FilesetOutputSymlink; import com.google.devtools.build.lib.analysis.actions.SymlinkAction; @@ -47,7 +48,8 @@ static void addToMap( Map> topLevelFilesets, Artifact key, SkyValue value, - Environment env) + Environment env, + boolean prefetcherSupportsPartialTreeArtifacts) throws InterruptedException { addToMap( inputMap, @@ -58,7 +60,8 @@ static void addToMap( key, value, env, - MetadataConsumerForMetrics.NO_OP); + MetadataConsumerForMetrics.NO_OP, + prefetcherSupportsPartialTreeArtifacts); } /** @@ -74,7 +77,8 @@ static void addToMap( Artifact key, SkyValue value, Environment env, - MetadataConsumerForMetrics consumer) + MetadataConsumerForMetrics consumer, + boolean prefetcherSupportsPartialTreeArtifacts) throws InterruptedException { if (value instanceof RunfilesArtifactValue) { // Note: we don't expand the .runfiles/MANIFEST file into the inputs. The reason for that @@ -120,16 +124,35 @@ static void addToMap( /*depOwner=*/ key); consumer.accumulate(treeArtifactValue); } else if (value instanceof ActionExecutionValue) { - FileArtifactValue metadata = ((ActionExecutionValue) value).getExistingFileArtifactValue(key); - inputMap.put(key, metadata, key); - if (key.isFileset()) { - ImmutableList filesets = getFilesets(env, (SpecialArtifact) key); - if (filesets != null) { - topLevelFilesets.put(key, filesets); - consumer.accumulate(filesets); - } + if (!prefetcherSupportsPartialTreeArtifacts && key instanceof TreeFileArtifact) { + // If we're unable to prefetch individual files in a tree artifact, include the full tree + // artifact in the action inputs. This makes actions that consume partial tree artifacts + // (such as the ones generated by SpawnActionTemplate or CppCompileActionTemplate) less + // efficient, but is needed until https://github.com/bazelbuild/bazel/issues/16333 is fixed. + SpecialArtifact treeArtifact = key.getParent(); + TreeArtifactValue treeArtifactValue = + ((ActionExecutionValue) value).getTreeArtifactValue(treeArtifact); + expandTreeArtifactAndPopulateArtifactData( + treeArtifact, + treeArtifactValue, + expandedArtifacts, + archivedTreeArtifacts, + inputMap, + /* depOwner= */ treeArtifact); + consumer.accumulate(treeArtifactValue); } else { - consumer.accumulate(metadata); + FileArtifactValue metadata = + ((ActionExecutionValue) value).getExistingFileArtifactValue(key); + inputMap.put(key, metadata, key); + if (key.isFileset()) { + ImmutableList filesets = getFilesets(env, (SpecialArtifact) key); + if (filesets != null) { + topLevelFilesets.put(key, filesets); + consumer.accumulate(filesets); + } + } else { + consumer.accumulate(metadata); + } } } else { Preconditions.checkArgument(value instanceof FileArtifactValue, "Unexpected value %s", value); diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java index df7cb7534d80ce..1dfec25289ade8 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java @@ -218,7 +218,8 @@ public SkyValue compute(SkyKey skyKey, Environment env) input, artifactValue, env, - currentConsumer); + currentConsumer, + skyframeActionExecutor.supportsPartialTreeArtifactInputs()); if (!allArtifactsAreImportant && importantArtifactSet.contains(input)) { // Calling #addToMap a second time with `input` and `artifactValue` will perform no-op // updates to the secondary collections passed in (eg. expandedArtifacts, @@ -233,7 +234,8 @@ public SkyValue compute(SkyKey skyKey, Environment env) input, artifactValue, env, - MetadataConsumerForMetrics.NO_OP); + MetadataConsumerForMetrics.NO_OP, + skyframeActionExecutor.supportsPartialTreeArtifactInputs()); } } } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java index 6329c467285eb3..342d97715e8944 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java @@ -320,6 +320,10 @@ boolean useArchivedTreeArtifacts(ActionAnalysisMetadata action) { .test(action.getMnemonic()); } + boolean supportsPartialTreeArtifactInputs() { + return actionInputPrefetcher.supportsPartialTreeArtifactInputs(); + } + boolean publishTargetSummaries() { return options.getOptions(BuildEventProtocolOptions.class).publishTargetSummary; } diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorTest.java index c806bc3b5861a2..5f4bedca32fb94 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorTest.java @@ -879,6 +879,7 @@ public void testSharedActionsRacing() throws Exception { // is running. This way, both actions will check the action cache beforehand and try to update // the action cache post-build. final CountDownLatch inputsRequested = new CountDownLatch(2); + skyframeExecutor.configureActionExecutor(/* fileCache= */ null, ActionInputPrefetcher.NONE); skyframeExecutor .getEvaluator() .injectGraphTransformerForTesting( @@ -1231,6 +1232,7 @@ public void sharedActionTemplate() throws Exception { ActionTemplate template2 = new DummyActionTemplate(baseOutput, sharedOutput2, ActionOwner.SYSTEM_ACTION_OWNER); ActionLookupValue shared2Ct = createActionLookupValue(template2, shared2); + skyframeExecutor.configureActionExecutor(/* fileCache= */ null, ActionInputPrefetcher.NONE); // Inject the "configured targets" into the graph. skyframeExecutor .getDifferencerForTesting() @@ -1645,6 +1647,7 @@ public void analysisEventsNotStoredInExecution() throws Exception { createActionLookupValue(action2, lc2), null, NestedSetBuilder.create(Order.STABLE_ORDER, Event.warn("analysis warning 2"))); + skyframeExecutor.configureActionExecutor(/* fileCache= */ null, ActionInputPrefetcher.NONE); skyframeExecutor .getDifferencerForTesting() .inject(ImmutableMap.of(lc1, ctValue1, lc2, ctValue2)); diff --git a/src/test/shell/bazel/remote/build_without_the_bytes_test.sh b/src/test/shell/bazel/remote/build_without_the_bytes_test.sh index 477a2f0bd8335f..3680f845c1846f 100755 --- a/src/test/shell/bazel/remote/build_without_the_bytes_test.sh +++ b/src/test/shell/bazel/remote/build_without_the_bytes_test.sh @@ -70,14 +70,7 @@ else declare -r EXE_EXT="" fi -function test_cc_tree() { - if [[ "$PLATFORM" == "darwin" ]]; then - # TODO(b/37355380): This test is disabled due to RemoteWorker not supporting - # setting SDKROOT and DEVELOPER_DIR appropriately, as is required of - # action executors in order to select the appropriate Xcode toolchain. - return 0 - fi - +function setup_cc_tree() { mkdir -p a cat > a/BUILD <& "$TEST_log" \ - || fail "Failed to build //a:tree_cc with minimal downloads" + || fail "Failed to build //a:tree_cc with remote executor and minimal downloads" +} + +function test_cc_tree_remote_cache() { + if [[ "$PLATFORM" == "darwin" ]]; then + # TODO(b/37355380): This test is disabled due to RemoteWorker not supporting + # setting SDKROOT and DEVELOPER_DIR appropriately, as is required of + # action executors in order to select the appropriate Xcode toolchain. + return 0 + fi + + setup_cc_tree + + bazel build \ + --remote_cache=grpc://localhost:${worker_port} \ + --remote_download_minimal \ + //a:tree_cc >& "$TEST_log" \ + || fail "Failed to build //a:tree_cc with remote cache and minimal downloads" } function test_cc_include_scanning_and_minimal_downloads() {