From b704fdb18b8201ddfc017ab91b9d0a63892a4cd2 Mon Sep 17 00:00:00 2001 From: Googler Date: Mon, 20 Jan 2025 09:35:11 -0800 Subject: [PATCH] Introduce FileOpNodeMemoizingLookup for tracking file dependencies. This change introduces `FileOpNodeMemoizingLookup`, a new class responsible for accurately determining the file dependencies of both analysis (configured target) and execution (action) SkyValues. **Key Changes:** * **FileOpNodeMemoizingLookup:** This new class efficiently computes and caches file dependencies by performing a concurrent, memoized, and recursive traversal of Skyframe edges. It distinguishes between analysis and execution file dependencies, storing them separately within `FileOpNode` (previously `FileSystemOperationNode`). * **Refined `AbstractNestedFileOpNodes`:** The `AbstractNestedFileOpNodes` class (which is used to store a set of `FileOpNode`s) is now split into two subclasses: * `NestedFileOpNodes`: Represents a set of analysis file dependencies *without* any source file dependencies. * `NestedFileOpNodesWithSources`: Represents a set of analysis file dependencies that *includes* immediate source file dependencies. This separation allows a more precise representation of dependencies, distinguishing between files depended on during analysis and those depended on during execution. Source dependencies, which correspond to `InputFileConfiguredTarget` instances, are declared during analysis but are only truly depended upon by the actions owned by the corresponding configured targets, not the configured targets themselves. This distinction optimizes storage by allowing both configured targets and actions to share the same underlying file dependency graph structure. * **Consolidated `FileOpNodeOrFuture`:** The `FileSystemOperationNode` class (and related classes) have been renamed to `FileOpNode` and moved into the newly added `FileOpNodeOrFuture` interface. This consolidation improves code readability by centralizing the hierarchical structure of file operation nodes. * **Renamed `FileOpNode`:** The old name, `FileSystemOperationNode`, was becoming cumbersome. The new name, `FileOpNode`, is more concise. PiperOrigin-RevId: 717556933 Change-Id: I7376f1d5ffb30e520af0e15140d00159f2a78d6d --- .../skyframe/AbstractNestedFileOpNodes.java | 155 ++++++++++++ .../google/devtools/build/lib/skyframe/BUILD | 5 +- .../lib/skyframe/DirectoryListingKey.java | 2 +- .../devtools/build/lib/skyframe/FileKey.java | 3 +- .../lib/skyframe/FileOpNodeOrFuture.java | 51 ++++ .../lib/skyframe/FileSystemOperationNode.java | 22 -- .../NestedFileSystemOperationNodes.java | 72 ------ .../lib/skyframe/serialization/analysis/BUILD | 17 ++ .../analysis/FileDependencyDeserializer.java | 53 +++- .../analysis/FileDependencySerializer.java | 78 ++++-- .../analysis/FileOpNodeMemoizingLookup.java | 203 +++++++++++++++ .../InvalidationDataInfoOrFuture.java | 11 +- .../analysis/NestedDependencies.java | 39 ++- .../lib/skyframe/serialization/analysis/BUILD | 17 ++ .../FileOpNodeMemoizingLookupTest.java | 237 ++++++++++++++++++ 15 files changed, 830 insertions(+), 135 deletions(-) create mode 100644 src/main/java/com/google/devtools/build/lib/skyframe/AbstractNestedFileOpNodes.java create mode 100644 src/main/java/com/google/devtools/build/lib/skyframe/FileOpNodeOrFuture.java delete mode 100644 src/main/java/com/google/devtools/build/lib/skyframe/FileSystemOperationNode.java delete mode 100644 src/main/java/com/google/devtools/build/lib/skyframe/NestedFileSystemOperationNodes.java create mode 100644 src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/FileOpNodeMemoizingLookup.java create mode 100644 src/test/java/com/google/devtools/build/lib/skyframe/serialization/analysis/FileOpNodeMemoizingLookupTest.java diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AbstractNestedFileOpNodes.java b/src/main/java/com/google/devtools/build/lib/skyframe/AbstractNestedFileOpNodes.java new file mode 100644 index 00000000000000..cfc915c49a5422 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/skyframe/AbstractNestedFileOpNodes.java @@ -0,0 +1,155 @@ +// Copyright 2025 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.skyframe; + +import static com.google.common.base.Preconditions.checkArgument; +import static com.google.devtools.build.lib.skyframe.FileOpNodeOrFuture.EmptyFileOpNode.EMPTY_FILE_OP_NODE; + +import com.google.devtools.build.lib.skyframe.FileOpNodeOrFuture.FileOpNode; +import com.google.devtools.build.lib.skyframe.FileOpNodeOrFuture.FileOpNodeOrEmpty; +import java.util.Collection; +import javax.annotation.Nullable; + +/** + * Represents a collection of {@link FileOpNode}s, allowing for nested structures to represent + * complex file dependencies. + * + *

This class serves as a container for multiple {@link FileOpNode} instances, enabling the + * representation of file operation dependencies in a hierarchical manner. It differentiates between + * analysis dependencies (for example, BUILD and .bzl files) and "source" dependencies, used during + * execution (for example, .cpp, .h or .java files). It keeps them together to optimize storage. + * + *

Source vs. Analysis Dependencies: + * + *

+ * + *

Why combine them?
+ * Logically, source and analysis dependencies could be tracked separately with different {@link + * FileOpNode}s. However, this would duplicate the dependency graph structure in persistent storage, + * which is expensive. This class keeps them together, trading off a bit of complexity for reduced + * storage overhead. The structure is written only once, and the interpretation of dependencies must + * be handled by the client. + * + *

Subclasses: + * + *

+ */ +public abstract sealed class AbstractNestedFileOpNodes implements FileOpNodeOrFuture.FileOpNode + permits AbstractNestedFileOpNodes.NestedFileOpNodes, + AbstractNestedFileOpNodes.NestedFileOpNodesWithSources { + private final FileOpNode[] analysisDependencies; + + /** + * Opaque storage for use by serialization. + * + *

{@link FileOpNode}, {@link FileKey} and {@link DirectoryListingKey} are mutually dependent + * via {@link FileOpNode}. This type is opaque to avoid forcing {@link FileKey} and {@link + * DirectoryListingKey} to depend on serialization implementation code. + * + *

The serialization implementation initializes this field with double-checked locking so it is + * marked volatile. + */ + private volatile Object serializationScratch; + + /** + * Effectively, a factory method for {@link NestedFileOpNodes}, but formally a factory method for + * {@link FileOpNodeOrEmpty}. + * + *

Returns {@link EMPTY_FILE_OP_NODE} if {@code analysisDependencies} is empty. When {@code + * analysisDependencies} contains only one node, returns the node directly instead of wrapping it. + * Otherwise, returns a {@link NestedFileOpNodes} instance wrapping {@code analysisDependencies}. + */ + public static FileOpNodeOrEmpty from(Collection analysisDependencies) { + if (analysisDependencies.isEmpty()) { + return EMPTY_FILE_OP_NODE; + } + if (analysisDependencies.size() == 1) { + return analysisDependencies.iterator().next(); + } + return new NestedFileOpNodes(analysisDependencies.toArray(FileOpNode[]::new)); + } + + /** + * Creates {@link NestedFileOpNodesWithSources} with reductions similar to {@link + * #from(Collection)}. + */ + public static FileOpNodeOrEmpty from( + Collection analysisDependencies, Collection sources) { + if (sources.isEmpty()) { + return from(analysisDependencies); + } + // It's unclear if `analysisDependencies` can ever be empty here in practice, but it's + // permitted. It should be rare enough that defining a special type for it isn't worth it. + return new NestedFileOpNodesWithSources( + analysisDependencies.toArray(FileOpNode[]::new), sources.toArray(FileKey[]::new)); + } + + private AbstractNestedFileOpNodes(FileOpNode[] analysisDependencies) { + this.analysisDependencies = analysisDependencies; + } + + public int analysisDependenciesCount() { + return analysisDependencies.length; + } + + public FileOpNode getAnalysisDependency(int index) { + return analysisDependencies[index]; + } + + @Nullable + public Object getSerializationScratch() { + return serializationScratch; + } + + public void setSerializationScratch(Object value) { + this.serializationScratch = value; + } + + /** A set of {@link FileOpNode}s with no immediate source dependencies. */ + public static final class NestedFileOpNodes extends AbstractNestedFileOpNodes { + private NestedFileOpNodes(FileOpNode[] analysisDependencies) { + super(analysisDependencies); + checkArgument(analysisDependencies.length > 0); + } + } + + /** A set of analysis and source file dependencies. */ + public static final class NestedFileOpNodesWithSources extends AbstractNestedFileOpNodes { + private final FileKey[] sources; // never empty + + private NestedFileOpNodesWithSources(FileOpNode[] nodes, FileKey[] sources) { + super(nodes); + this.sources = sources; + } + + public int sourceCount() { + return sources.length; + } + + public FileKey getSource(int i) { + return sources[i]; + } + } +} diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD index 82058f9bd52a0f..b93eca59a7c5e6 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD +++ b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD @@ -1547,13 +1547,14 @@ java_library( java_library( name = "filesystem_keys", srcs = [ + "AbstractNestedFileOpNodes.java", "DirectoryListingKey.java", "FileKey.java", - "FileSystemOperationNode.java", - "NestedFileSystemOperationNodes.java", + "FileOpNodeOrFuture.java", ], deps = [ ":sky_functions", + "//src/main/java/com/google/devtools/build/lib/concurrent:settable_future_keyed_value", "//src/main/java/com/google/devtools/build/lib/skyframe/serialization:visible-for-serialization", "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec", "//src/main/java/com/google/devtools/build/lib/vfs", diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/DirectoryListingKey.java b/src/main/java/com/google/devtools/build/lib/skyframe/DirectoryListingKey.java index 485aad5ae10027..6ffe41922edcf5 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/DirectoryListingKey.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/DirectoryListingKey.java @@ -25,7 +25,7 @@ /** Key for {@link DirectoryListingFunction}. */ @AutoCodec public final class DirectoryListingKey extends AbstractSkyKey - implements FileSystemOperationNode { + implements FileOpNodeOrFuture.FileOpNode { private static final SkyKeyInterner interner = SkyKey.newInterner(); diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/FileKey.java b/src/main/java/com/google/devtools/build/lib/skyframe/FileKey.java index 47c4b008215a74..8d7ba67293538c 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/FileKey.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/FileKey.java @@ -24,7 +24,8 @@ /** Key for {@link FileFunction}. */ @AutoCodec -public final class FileKey extends AbstractSkyKey implements FileSystemOperationNode { +public final class FileKey extends AbstractSkyKey + implements FileOpNodeOrFuture.FileOpNode { private static final SkyKeyInterner interner = SkyKey.newInterner(); public static FileKey create(RootedPath arg) { diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/FileOpNodeOrFuture.java b/src/main/java/com/google/devtools/build/lib/skyframe/FileOpNodeOrFuture.java new file mode 100644 index 00000000000000..0aa02dba0660bf --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/skyframe/FileOpNodeOrFuture.java @@ -0,0 +1,51 @@ +// Copyright 2024 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.skyframe; + +import com.google.devtools.build.lib.concurrent.SettableFutureKeyedValue; +import com.google.devtools.build.skyframe.SkyKey; +import java.util.function.BiConsumer; + +/** + * A possibly empty nested set of file system operations. + * + *

This value represents the set of file system operation dependencies of a given Skyframe entry, + * computed by Skyframe graph traversal. + */ +@SuppressWarnings("InterfaceWithOnlyStatics") // sealed hierarchy root +public sealed interface FileOpNodeOrFuture + permits FileOpNodeOrFuture.FileOpNodeOrEmpty, FileOpNodeOrFuture.FutureFileOpNode { + + /** A possibly empty set of file system dependencies. */ + sealed interface FileOpNodeOrEmpty extends FileOpNodeOrFuture + permits EmptyFileOpNode, FileOpNode {} + + /** A non-empty set of filesystem operations. */ + sealed interface FileOpNode extends FileOpNodeOrEmpty + permits FileKey, DirectoryListingKey, AbstractNestedFileOpNodes {} + + /** Empty set of filesystem dependencies. */ + enum EmptyFileOpNode implements FileOpNodeOrEmpty { + EMPTY_FILE_OP_NODE; + } + + /** The in-flight computation of a {@link FileOpNodeOrEmpty}. */ + static final class FutureFileOpNode + extends SettableFutureKeyedValue + implements FileOpNodeOrFuture { + public FutureFileOpNode(SkyKey key, BiConsumer consumer) { + super(key, consumer); + } + } +} diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/FileSystemOperationNode.java b/src/main/java/com/google/devtools/build/lib/skyframe/FileSystemOperationNode.java deleted file mode 100644 index caed5761e81a83..00000000000000 --- a/src/main/java/com/google/devtools/build/lib/skyframe/FileSystemOperationNode.java +++ /dev/null @@ -1,22 +0,0 @@ -// Copyright 2024 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.skyframe; - -/** - * Represents either a single file system operation or a nested set thereof. - * - *

If the set is unary, it is represented without a wrapper. - */ -public sealed interface FileSystemOperationNode - permits FileKey, DirectoryListingKey, NestedFileSystemOperationNodes {} diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/NestedFileSystemOperationNodes.java b/src/main/java/com/google/devtools/build/lib/skyframe/NestedFileSystemOperationNodes.java deleted file mode 100644 index e22aedb4d440f5..00000000000000 --- a/src/main/java/com/google/devtools/build/lib/skyframe/NestedFileSystemOperationNodes.java +++ /dev/null @@ -1,72 +0,0 @@ -// Copyright 2024 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.skyframe; - -import static com.google.common.base.Preconditions.checkArgument; - -import java.util.Collection; -import javax.annotation.Nullable; - -/** Represents multiple {@link FileSystemOperationNode}s with nestable composition. */ -public final class NestedFileSystemOperationNodes implements FileSystemOperationNode { - private final FileSystemOperationNode[] nodes; - - /** - * Opaque storage for use by serialization. - * - *

{@link FileSystemOperationNode}, {@link FileKey} and {@link DirectoryListingKey} are - * mutually dependent via {@link FileSystemOperationNode}. This type is opaque to avoid forcing - * {@link FileKey} and {@link DirectoryListingKey} to depend on serialization implementation code. - * - *

The serialization implementation initializes this field with double-checked locking so it is - * marked volatile. - */ - private volatile Object serializationScratch; - - /** - * Effectively, a factory method for {@link NestedFileSystemOperationNodes}, but formally a - * factory method for {@link FileSystemOperationNode}. - * - *

When there is only one node, returns the node directly instead of creating a useless - * wrapper. - */ - public static FileSystemOperationNode from(Collection nodes) { - checkArgument(!nodes.isEmpty()); - if (nodes.size() == 1) { - return nodes.iterator().next(); - } - return new NestedFileSystemOperationNodes(nodes.toArray(FileSystemOperationNode[]::new)); - } - - private NestedFileSystemOperationNodes(FileSystemOperationNode[] nodes) { - this.nodes = nodes; - } - - public int count() { - return nodes.length; - } - - public FileSystemOperationNode get(int index) { - return nodes[index]; - } - - @Nullable - public Object getSerializationScratch() { - return serializationScratch; - } - - public void setSerializationScratch(Object value) { - this.serializationScratch = value; - } -} diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/BUILD index 343095cb6a8bbb..4fd945b296a658 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/BUILD +++ b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/BUILD @@ -81,6 +81,23 @@ java_library( ], ) +java_library( + name = "file_op_node_map", + srcs = ["FileOpNodeMemoizingLookup.java"], + deps = [ + ":value_or_future_map", + "//src/main/java/com/google/devtools/build/lib/actions:action_lookup_key", + "//src/main/java/com/google/devtools/build/lib/actions:artifacts", + "//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster", + "//src/main/java/com/google/devtools/build/lib/skyframe:filesystem_keys", + "//src/main/java/com/google/devtools/build/lib/skyframe:rule_configured_target_value", + "//src/main/java/com/google/devtools/build/lib/vfs", + "//src/main/java/com/google/devtools/build/skyframe", + "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects", + "//third_party:guava", + ], +) + java_library( name = "file_dependency_serializer", srcs = ["FileDependencySerializer.java"], diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/FileDependencyDeserializer.java b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/FileDependencyDeserializer.java index c0e470712414ea..dc35c7285bf7a4 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/FileDependencyDeserializer.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/FileDependencyDeserializer.java @@ -532,9 +532,12 @@ public ListenableFuture apply(byte[] bytes) { int fileCount = codedIn.readInt32(); int listingCount = codedIn.readInt32(); int nestedCount = codedIn.readInt32(); + int sourceCount = codedIn.readInt32(); var elements = new FileSystemDependencies[fileCount + listingCount + nestedCount]; - var countdown = new PendingElementCountdown(elements); + var sources = + sourceCount > 0 ? new FileDependencies[sourceCount] : NestedDependencies.EMPTY_SOURCES; + var countdown = new PendingElementCountdown(elements, sources); for (int i = 0; i < fileCount; i++) { String key = codedIn.readString(); @@ -576,6 +579,19 @@ public ListenableFuture apply(byte[] bytes) { break; } } + + for (int i = 0; i < sourceCount; i++) { + String key = codedIn.readString(); + switch (getFileDependencies(key)) { + case FileDependencies dependencies: + sources[i] = dependencies; + break; + case FutureFileDependencies future: + countdown.registerPendingElement(); + Futures.addCallback(future, new WaitingForSource(i, countdown), directExecutor()); + break; + } + } countdown.notifyInitializationDone(); return countdown; } catch (IOException e) { @@ -591,9 +607,11 @@ public ListenableFuture apply(byte[] bytes) { */ private static class PendingElementCountdown extends QuiescingFuture { private final FileSystemDependencies[] elements; + private final FileDependencies[] sources; - private PendingElementCountdown(FileSystemDependencies[] elements) { + private PendingElementCountdown(FileSystemDependencies[] elements, FileDependencies[] sources) { this.elements = elements; + this.sources = sources; } private void registerPendingElement() { @@ -609,13 +627,18 @@ private void setPendingElement(int index, FileSystemDependencies value) { decrement(); } - private void notifyElementFailure(Throwable e) { + private void setSource(int index, FileDependencies value) { + sources[index] = value; + decrement(); + } + + private void notifyFailure(Throwable e) { notifyException(e); } @Override protected NestedDependencies getValue() { - return new NestedDependencies(elements); + return new NestedDependencies(elements, sources); } } @@ -640,7 +663,27 @@ public void onSuccess(FileSystemDependencies dependencies) { @Override public void onFailure(Throwable t) { - countdown.notifyElementFailure(t); + countdown.notifyFailure(t); + } + } + + private static class WaitingForSource implements FutureCallback { + private final int index; + private final PendingElementCountdown countdown; + + private WaitingForSource(int index, PendingElementCountdown countdown) { + this.index = index; + this.countdown = countdown; + } + + @Override + public void onSuccess(FileDependencies dependencies) { + countdown.setSource(index, dependencies); + } + + @Override + public void onFailure(Throwable t) { + countdown.notifyFailure(t); } } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/FileDependencySerializer.java b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/FileDependencySerializer.java index 4925e30fe63826..1b431871714863 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/FileDependencySerializer.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/FileDependencySerializer.java @@ -38,10 +38,12 @@ import com.google.devtools.build.lib.actions.FileStateValue; import com.google.devtools.build.lib.actions.FileValue; import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider.BundledFileSystem; +import com.google.devtools.build.lib.skyframe.AbstractNestedFileOpNodes; +import com.google.devtools.build.lib.skyframe.AbstractNestedFileOpNodes.NestedFileOpNodes; +import com.google.devtools.build.lib.skyframe.AbstractNestedFileOpNodes.NestedFileOpNodesWithSources; import com.google.devtools.build.lib.skyframe.DirectoryListingKey; import com.google.devtools.build.lib.skyframe.FileKey; -import com.google.devtools.build.lib.skyframe.FileSystemOperationNode; -import com.google.devtools.build.lib.skyframe.NestedFileSystemOperationNodes; +import com.google.devtools.build.lib.skyframe.FileOpNodeOrFuture.FileOpNode; import com.google.devtools.build.lib.skyframe.serialization.FingerprintValueService; import com.google.devtools.build.lib.skyframe.serialization.KeyBytesProvider; import com.google.devtools.build.lib.skyframe.serialization.PackedFingerprint; @@ -80,7 +82,7 @@ import javax.annotation.Nullable; /** - * Records {@link FileKey}, {@link DirectoryListingKey} or {@link NestedFileSystemOperationNodes} + * Records {@link FileKey}, {@link DirectoryListingKey} or {@link AbstractNestedFileOpNodes} * invalidation to a remote {@link FingerprintValueService}. */ final class FileDependencySerializer { @@ -124,13 +126,13 @@ final class FileDependencySerializer { *

See comments at {@link FileInvalidationData} and {@link DirectoryListingInvalidationData} * for more details about the data being persisted. */ - InvalidationDataInfoOrFuture registerDependency(FileSystemOperationNode node) { + InvalidationDataInfoOrFuture registerDependency(FileOpNode node) { switch (node) { case FileKey file: return registerDependency(file); case DirectoryListingKey listing: return registerDependency(listing); - case NestedFileSystemOperationNodes nested: + case AbstractNestedFileOpNodes nested: return registerDependency(nested); } } @@ -148,7 +150,7 @@ ListingDataInfoOrFuture registerDependency(DirectoryListingKey key) { * *

Uploads the result to the {@link #fingerprintValueService}. */ - NodeDataInfoOrFuture registerDependency(NestedFileSystemOperationNodes node) { + NodeDataInfoOrFuture registerDependency(AbstractNestedFileOpNodes node) { var reference = (NodeDataInfoOrFuture) node.getSerializationScratch(); if (reference != null) { return reference; @@ -533,26 +535,36 @@ public ListingInvalidationDataInfo apply(FileDataInfo info) { } NodeDataInfoOrFuture populateFutureNodeDataInfo(FutureNodeDataInfo future) { - NestedFileSystemOperationNodes node = future.key(); + AbstractNestedFileOpNodes node = future.key(); var dependencyHandler = new NodeDependencyHandler(); // Loops through all node dependencies, registering them with the dependencyHandler. The // dependencyHandler triggers recursive registration, keeping track of immediate results and // any futures. - for (int i = 0; i < node.count(); i++) { - switch (node.get(i)) { + for (int i = 0; i < node.analysisDependenciesCount(); i++) { + switch (node.getAnalysisDependency(i)) { case FileKey fileKey: dependencyHandler.addFileKey(fileKey); break; case DirectoryListingKey listingKey: dependencyHandler.addListingKey(listingKey); break; - case NestedFileSystemOperationNodes nestedKeys: + case AbstractNestedFileOpNodes nestedKeys: dependencyHandler.addNodeKey(nestedKeys); break; } } + switch (node) { + case NestedFileOpNodes plainNodes: + break; + case NestedFileOpNodesWithSources withSources: + for (int i = 0; i < withSources.sourceCount(); i++) { + dependencyHandler.addSourceFile(withSources.getSource(i)); + } + break; + } + var allFutures = dependencyHandler.getCombinedFutures(); if (allFutures.isEmpty()) { NodeDataInfo result; @@ -580,12 +592,14 @@ private class NodeDependencyHandler implements Callable { private final TreeSet listingKeys = new TreeSet<>(); private final TreeMap nodeDependencies = new TreeMap<>(); + private final TreeSet sourceFileKeys = new TreeSet<>(); private final ArrayList> writeStatuses = new ArrayList<>(); private final ArrayList futureFileDataInfo = new ArrayList<>(); private final ArrayList futureListingDataInfo = new ArrayList<>(); private final ArrayList futureNodeDataInfo = new ArrayList<>(); + private final ArrayList futureSourceFileInfo = new ArrayList<>(); @Override public NodeDataInfo call() throws ExecutionException { @@ -598,8 +612,11 @@ public NodeDataInfo call() throws ExecutionException { for (FutureNodeDataInfo futureInfo : futureNodeDataInfo) { addNodeInfo(Futures.getDone(futureInfo)); } + for (FutureFileDataInfo futureInfo : futureSourceFileInfo) { + addSourceFileInfo(Futures.getDone(futureInfo)); + } - if (fileKeys.isEmpty() && listingKeys.isEmpty()) { + if (fileKeys.isEmpty() && listingKeys.isEmpty() && sourceFileKeys.isEmpty()) { if (nodeDependencies.isEmpty()) { return CONSTANT_NODE; // None of the dependencies are relevant to invalidation. } @@ -607,7 +624,7 @@ public NodeDataInfo call() throws ExecutionException { // least 2 children. The following may reduce child count. // 1. TreeSet deduplication. // 2. Constant references. - // 3. NestedFileSystemOperationNodes with the same fingerprints. + // 3. NestedFileOpNodes with the same fingerprints. if (nodeDependencies.size() == 1) { // It ended up as a node wrapping another node. Discards the wrapper. // @@ -667,7 +684,7 @@ private void addListingInfo(ListingDataInfo info) { } } - private void addNodeKey(NestedFileSystemOperationNodes nestedKeys) { + private void addNodeKey(AbstractNestedFileOpNodes nestedKeys) { switch (registerDependency(nestedKeys)) { case NodeDataInfo info: addNodeInfo(info); @@ -689,11 +706,34 @@ private void addNodeInfo(NodeDataInfo info) { } } + private void addSourceFile(FileKey sourceFile) { + switch (registerDependency(sourceFile)) { + case FileDataInfo info: + addSourceFileInfo(info); + break; + case FutureFileDataInfo futureInfo: + futureSourceFileInfo.add(futureInfo); + break; + } + } + + private void addSourceFileInfo(FileDataInfo info) { + switch (info) { + case CONSTANT_FILE: + break; + case FileInvalidationDataInfo fileInfo: + sourceFileKeys.add(fileInfo.cacheKey()); + addWriteStatusUnlessSuccess(fileInfo.writeStatus(), writeStatuses); + break; + } + } + private ImmutableList> getCombinedFutures() { return ImmutableList.>builder() .addAll(futureFileDataInfo) .addAll(futureListingDataInfo) .addAll(futureNodeDataInfo) + .addAll(futureSourceFileInfo) .build(); } @@ -702,16 +742,18 @@ private ImmutableList> getCombinedFutures() { * *

Logically, a node is a set of string file or listing keys, as described at {@link * FileInvalidationData} and {@link DirectoryListingInvalidationData}, respectively, and a set - * of {@link NestedFileSystemOperationNodes} fingerprints. Its byte representation is specified - * as follows. + * of {@link NestedFileOpNodes} fingerprints. Its byte representation is specified as follows. * *

    *
  1. The count of file keys, as a proto-encoded int. *
  2. The count of listing keys, as a proto-encoded int. *
  3. The count of nested nodes, as a proto-encoded int. + *
  4. The count of source file keys, as a proto-encoded int. *
  5. Sorted and deduplicated, proto-encoded strings of the file keys. *
  6. Sorted and deduplicated, proto-encoded strings of the listing keys. - *
  7. The fingerprints of the {@link NestedFileSystemOperationNodes} byte representations. + *
  8. Sorted and deduplicated, fingerprints of the {@link NestedFileOpNodes} byte + * representations. + *
  9. Sorted and deduplicated, proto-encoded strings of the source keys. *
* *

More compact formats are possible, but this reduces the complexity of the deserializer. @@ -723,6 +765,7 @@ private byte[] computeNodeBytes() { codedOut.writeInt32NoTag(fileKeys.size()); codedOut.writeInt32NoTag(listingKeys.size()); codedOut.writeInt32NoTag(nodeDependencies.size()); + codedOut.writeInt32NoTag(sourceFileKeys.size()); for (String key : fileKeys) { codedOut.writeStringNoTag(key); } @@ -732,6 +775,9 @@ private byte[] computeNodeBytes() { for (PackedFingerprint fp : nodeDependencies.keySet()) { fp.writeTo(codedOut); } + for (String key : sourceFileKeys) { + codedOut.writeStringNoTag(key); + } codedOut.flush(); bytesOut.flush(); return bytesOut.toByteArray(); diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/FileOpNodeMemoizingLookup.java b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/FileOpNodeMemoizingLookup.java new file mode 100644 index 00000000000000..151c6019cf9ed5 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/FileOpNodeMemoizingLookup.java @@ -0,0 +1,203 @@ +// Copyright 2025 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.skyframe.serialization.analysis; + +import static com.google.common.base.Preconditions.checkNotNull; +import static com.google.common.util.concurrent.MoreExecutors.directExecutor; +import static com.google.devtools.build.lib.skyframe.FileOpNodeOrFuture.EmptyFileOpNode.EMPTY_FILE_OP_NODE; + +import com.google.common.util.concurrent.Futures; +import com.google.devtools.build.lib.actions.ActionLookupKey; +import com.google.devtools.build.lib.actions.Artifact.SourceArtifact; +import com.google.devtools.build.lib.analysis.configuredtargets.InputFileConfiguredTarget; +import com.google.devtools.build.lib.skyframe.AbstractNestedFileOpNodes; +import com.google.devtools.build.lib.skyframe.FileKey; +import com.google.devtools.build.lib.skyframe.FileOpNodeOrFuture; +import com.google.devtools.build.lib.skyframe.FileOpNodeOrFuture.FileOpNode; +import com.google.devtools.build.lib.skyframe.FileOpNodeOrFuture.FileOpNodeOrEmpty; +import com.google.devtools.build.lib.skyframe.FileOpNodeOrFuture.FutureFileOpNode; +import com.google.devtools.build.lib.skyframe.NonRuleConfiguredTargetValue; +import com.google.devtools.build.lib.vfs.RootedPath; +import com.google.devtools.build.skyframe.InMemoryGraph; +import com.google.devtools.build.skyframe.SkyKey; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.concurrent.Callable; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ExecutionException; + +/** + * Computes a mapping from {@link ActionLookupKey}s to {@link FileOpNodeOrFuture}s, representing the + * complete set of file system operation dependencies required to evaluate each key. + * + *

This class tracks file dependencies for a particular build. It uses the file and source + * partitioning in {@link AbstractNestedFileOpNodes} to provide a view of file dependencies for + * configured targets and actions. For configured targets, only the analysis dependencies (BUILD, + * .bzl files) are relevant. For actions, the source (.h, .cpp, .java) files must also be + * considered. + * + *

Approximation for Efficiency: To avoid the excessive overhead of storing precise file + * dependencies per action, an over-approximation is used. This may lead to occasional spurious + * cache misses but guarantees no false cache hits. The approximation includes all source + * dependencies declared by the configured target that were visited during the build. + * + *

Not all actions of a configured target are executed, and include scanning may eliminate + * dependencies, so the actual set of source files visited by a build may be a subset of the + * declared ones. This will never skip an actual action file dependency of the build. While this is + * correct, it's possible that different builds at the same version will have slightly different + * representations of the sets of sources. + * + *

Why Approximation?
+ * Storing the exact file dependencies for each action individually would be too expensive. It would + * negate the benefits of the compact nested representation used for configured target dependencies. + * The chosen approximation balances accuracy with performance. + * + *

Different Sources in Multiple Builds
+ * Suppose there are multiple builds that share configured targets, but request different actions + * from those configured targets. The configured target data is deterministic and shared, but the + * invalidation information for source files could differ. When invalidating the configured target, + * the source files are ignored, so even if a second build overwrites the configured target of the + * first, invalidation of the configured target still works exactly the same way. For actions, + * overwriting of the configured target doesn't affect correctness either because each action + * directly references the invalidation data created by its respective build. + */ +final class FileOpNodeMemoizingLookup { + private final InMemoryGraph graph; + + private final ValueOrFutureMap + nodes = + new ValueOrFutureMap<>( + new ConcurrentHashMap<>(), + FutureFileOpNode::new, + this::populateFutureFileOpNode, + FutureFileOpNode.class); + + FileOpNodeMemoizingLookup(InMemoryGraph graph) { + this.graph = graph; + } + + FileOpNodeOrFuture computeNode(ActionLookupKey key) { + return nodes.getValueOrFuture(key); + } + + private FileOpNodeOrFuture populateFutureFileOpNode(FutureFileOpNode ownedFuture) { + var builder = new FileOpNodeBuilder(); + + accumulateTransitiveFileSystemOperations(ownedFuture.key(), builder); + + if (!builder.hasFutures()) { + // This can be empty for certain functions, e.g., PRECOMPUTED, IGNORED_PACKAGE_PREFIXES and + // PARSED_FLAGS. + return ownedFuture.completeWith(builder.call()); + } + return ownedFuture.completeWith( + Futures.whenAllSucceed(builder.futureNodes).call(builder, directExecutor())); + } + + private void accumulateTransitiveFileSystemOperations(SkyKey key, FileOpNodeBuilder builder) { + for (SkyKey dep : checkNotNull(graph.getIfPresent(key), key).getDirectDeps()) { + switch (dep) { + case FileOpNode immediateNode: + builder.addNode(immediateNode); + break; + default: + addNodeForKey(dep, builder); + break; + } + } + } + + private void addNodeForKey(SkyKey key, FileOpNodeBuilder builder) { + if (key instanceof ActionLookupKey actionLookupKey) { + var nodeEntry = checkNotNull(graph.getIfPresent(key), key); + // If the corresponding value is an InputFileConfiguredTarget, it indicates an execution time + // file dependency. + if ((checkNotNull(nodeEntry.getValue(), actionLookupKey) + instanceof NonRuleConfiguredTargetValue nonRuleConfiguredTargetValue) + && (nonRuleConfiguredTargetValue.getConfiguredTarget() + instanceof InputFileConfiguredTarget inputFileConfiguredTarget)) { + // The source artifact's file becomes an execution time dependency of actions owned by + // configured targets with this InputFileConfiguredTarget as a dependency. + SourceArtifact source = inputFileConfiguredTarget.getArtifact(); + var fileKey = + FileKey.create(RootedPath.toRootedPath(source.getRoot().getRoot(), source.getPath())); + if (graph.getIfPresent(fileKey) != null) { + // If the file value is not present in the graph, it means that no action executed + // actually depended on that file. + // + // TODO: b/364831651 - for greater determinism, consider performing additional Skyframe + // evaluations for these unused dependencies. + builder.addSource(fileKey); + } + } + } + + // TODO: b/364831651 - This adds all traversed SkyKeys to `nodes`. Consider if certain types + // should be excluded from memoization. + switch (nodes.getValueOrFuture(key)) { + case EMPTY_FILE_OP_NODE: + break; + case FileOpNode node: + builder.addNode(node); + break; + case FutureFileOpNode future: + builder.addFuture(future); + break; + } + } + + private static class FileOpNodeBuilder implements Callable { + private final HashSet nodes = new HashSet<>(); + + private final HashSet sourceFiles = new HashSet<>(); + + private final ArrayList futureNodes = new ArrayList<>(); + + /** Called only after all futures in {@link #futureNodes} succeed. */ + @Override + public FileOpNodeOrEmpty call() { + for (FutureFileOpNode future : futureNodes) { + try { + addNode(Futures.getDone(future)); + } catch (ExecutionException e) { + throw new IllegalStateException( + "unexpected exception, should only be called after success", e); + } + } + return AbstractNestedFileOpNodes.from(nodes, sourceFiles); + } + + private boolean hasFutures() { + return !futureNodes.isEmpty(); + } + + private void addNode(FileOpNodeOrEmpty nodeOrEmpty) { + switch (nodeOrEmpty) { + case EMPTY_FILE_OP_NODE: + break; + case FileOpNode node: + nodes.add(node); + break; + } + } + + private void addSource(FileKey sourceFile) { + sourceFiles.add(sourceFile); + } + + private void addFuture(FutureFileOpNode future) { + futureNodes.add(future); + } + } +} diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/InvalidationDataInfoOrFuture.java b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/InvalidationDataInfoOrFuture.java index 681e9508b7e7e2..2f2bddbafcbb33 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/InvalidationDataInfoOrFuture.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/InvalidationDataInfoOrFuture.java @@ -15,9 +15,9 @@ import com.google.common.util.concurrent.ListenableFuture; import com.google.devtools.build.lib.concurrent.SettableFutureKeyedValue; +import com.google.devtools.build.lib.skyframe.AbstractNestedFileOpNodes; import com.google.devtools.build.lib.skyframe.DirectoryListingKey; import com.google.devtools.build.lib.skyframe.FileKey; -import com.google.devtools.build.lib.skyframe.NestedFileSystemOperationNodes; import com.google.devtools.build.lib.skyframe.serialization.PackedFingerprint; import com.google.devtools.build.lib.vfs.RootedPath; import java.util.function.BiConsumer; @@ -160,7 +160,7 @@ enum ConstantNodeData implements NodeDataInfo { CONSTANT_NODE; } - /** Information about remotely persisted {@link NestedFileSystemOperationNodes}. */ + /** Information about remotely persisted {@link AbstractNestedFileOpNodes}. */ static final class NodeInvalidationDataInfo extends BaseInvalidationDataInfo implements NodeDataInfo { NodeInvalidationDataInfo(PackedFingerprint key, ListenableFuture writeStatus) { @@ -169,14 +169,13 @@ static final class NodeInvalidationDataInfo extends BaseInvalidationDataInfo + extends SettableFutureKeyedValue implements NodeDataInfoOrFuture { - FutureNodeDataInfo(NestedFileSystemOperationNodes key) { + FutureNodeDataInfo(AbstractNestedFileOpNodes key) { super(key, FutureNodeDataInfo::setNodeDataInfo); } - private static void setNodeDataInfo(NestedFileSystemOperationNodes key, NodeDataInfo value) { + private static void setNodeDataInfo(AbstractNestedFileOpNodes key, NodeDataInfo value) { key.setSerializationScratch(value); } } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/NestedDependencies.java b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/NestedDependencies.java index 082caad3aeff5e..448402a011eced 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/NestedDependencies.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis/NestedDependencies.java @@ -22,28 +22,47 @@ * A representation of a recursively composable set of {@link FileSystemDependencies}. * *

This corresponds to a previously serialized {@link - * com.google.devtools.build.lib.skyframe.NestedFileSystemOperationNodes} instance, but this + * com.google.devtools.build.lib.skyframe.AbstractNestedFileOpNodes} instance, but this * implementation is mostly decoupled from Bazel code. */ final class NestedDependencies implements FileSystemDependencies, FileDependencyDeserializer.NestedDependenciesOrFuture { - private final FileSystemDependencies[] elements; + // While formally possible, we don't anticipate analysisDependencies being empty often. `sources` + // could be frequently empty. + static final FileDependencies[] EMPTY_SOURCES = new FileDependencies[0]; - NestedDependencies(FileSystemDependencies[] elements) { - checkArgument(elements.length >= 1, "expected at least length 1, was %s", elements.length); - this.elements = elements; + private final FileSystemDependencies[] analysisDependencies; + private final FileDependencies[] sources; + + NestedDependencies(FileSystemDependencies[] analysisDependencies, FileDependencies[] sources) { + checkArgument( + analysisDependencies.length >= 1 || sources.length >= 1, + "analysisDependencies and sources both empty"); + this.analysisDependencies = analysisDependencies; + this.sources = sources; + } + + int analysisDependenciesCount() { + return analysisDependencies.length; + } + + FileSystemDependencies getAnalysisDependency(int index) { + return analysisDependencies[index]; } - int count() { - return elements.length; + int sourceCount() { + return sources.length; } - FileSystemDependencies getElement(int index) { - return elements[index]; + FileDependencies getSource(int index) { + return sources[index]; } @Override public String toString() { - return toStringHelper(this).add("elements", Arrays.asList(elements)).toString(); + return toStringHelper(this) + .add("analysisDependencies", Arrays.asList(analysisDependencies)) + .add("sources", Arrays.asList(sources)) + .toString(); } } diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/serialization/analysis/BUILD b/src/test/java/com/google/devtools/build/lib/skyframe/serialization/analysis/BUILD index d60b48415f73d9..e66ca50dd2bae7 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/serialization/analysis/BUILD +++ b/src/test/java/com/google/devtools/build/lib/skyframe/serialization/analysis/BUILD @@ -65,3 +65,20 @@ java_test( "//third_party:truth", ], ) + +java_test( + name = "FileOpNodeMemoizingLookupTest", + srcs = ["FileOpNodeMemoizingLookupTest.java"], + deps = [ + "//src/main/java/com/google/devtools/build/lib/actions:action_lookup_data", + "//src/main/java/com/google/devtools/build/lib/actions:action_lookup_key", + "//src/main/java/com/google/devtools/build/lib/skyframe:filesystem_keys", + "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/analysis:file_op_node_map", + "//src/main/java/com/google/devtools/build/skyframe", + "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects", + "//src/test/java/com/google/devtools/build/lib/buildtool/util", + "//third_party:guava", + "//third_party:junit4", + "//third_party:truth", + ], +) diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/serialization/analysis/FileOpNodeMemoizingLookupTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/serialization/analysis/FileOpNodeMemoizingLookupTest.java new file mode 100644 index 00000000000000..4453116235820c --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/skyframe/serialization/analysis/FileOpNodeMemoizingLookupTest.java @@ -0,0 +1,237 @@ +// Copyright 2025 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.skyframe.serialization.analysis; + +import static com.google.common.truth.Truth.assertWithMessage; +import static com.google.common.util.concurrent.Futures.immediateVoidFuture; +import static com.google.common.util.concurrent.MoreExecutors.directExecutor; +import static com.google.devtools.build.lib.skyframe.FileOpNodeOrFuture.EmptyFileOpNode.EMPTY_FILE_OP_NODE; + +import com.google.common.base.Function; +import com.google.common.collect.ImmutableSet; +import com.google.common.util.concurrent.Futures; +import com.google.common.util.concurrent.ListenableFuture; +import com.google.devtools.build.lib.actions.ActionLookupData; +import com.google.devtools.build.lib.actions.ActionLookupKey; +import com.google.devtools.build.lib.buildtool.util.BuildIntegrationTestCase; +import com.google.devtools.build.lib.skyframe.AbstractNestedFileOpNodes.NestedFileOpNodes; +import com.google.devtools.build.lib.skyframe.AbstractNestedFileOpNodes.NestedFileOpNodesWithSources; +import com.google.devtools.build.lib.skyframe.DirectoryListingKey; +import com.google.devtools.build.lib.skyframe.FileKey; +import com.google.devtools.build.lib.skyframe.FileOpNodeOrFuture.FileOpNode; +import com.google.devtools.build.lib.skyframe.FileOpNodeOrFuture.FileOpNodeOrEmpty; +import com.google.devtools.build.lib.skyframe.FileOpNodeOrFuture.FutureFileOpNode; +import com.google.devtools.build.skyframe.InMemoryGraph; +import com.google.devtools.build.skyframe.SkyKey; +import java.util.ArrayList; +import java.util.HashSet; +import java.util.Set; +import java.util.concurrent.ConcurrentLinkedQueue; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ForkJoinPool; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public final class FileOpNodeMemoizingLookupTest extends BuildIntegrationTestCase { + // TODO: b/364831651 - consider adding test cases covering other scenarios, like symlinks. + + private static final int CONCURRENCY = 4; + + @Test + public void fileOpNodes_areConsistent() throws Exception { + // This test case contains a glob to exercise DirectoryListingKey. + write("hello/x.txt", "x"); + write( + "hello/BUILD", + """ + genrule( + name = "target", + srcs = glob(["*.txt"]), + outs = ["out"], + cmd = "cat $(SRCS) > $@", + ) + """); + + buildTarget("//hello:target"); + + InMemoryGraph graph = getSkyframeExecutor().getEvaluator().getInMemoryGraph(); + + var fileOpDataMap = new FileOpNodeMemoizingLookup(graph); + + var actionLookups = new ArrayList(); + var actions = new ArrayList(); + + for (SkyKey key : graph.getDoneValues().keySet()) { + if (key instanceof ActionLookupKey lookupKey) { + actionLookups.add(lookupKey); + } + if (key instanceof ActionLookupData lookupData) { + actions.add(lookupData); + } + } + + var futures = new ConcurrentLinkedQueue>(); + var pool = new ForkJoinPool(CONCURRENCY); + var allAdded = new CountDownLatch(actionLookups.size() + actions.size()); + + for (ActionLookupKey lookupKey : actionLookups) { + pool.execute( + () -> { + futures.add(verifyFileOpNodeForActionLookupKey(graph, fileOpDataMap, lookupKey)); + allAdded.countDown(); + }); + } + for (ActionLookupData lookupData : actions) { + pool.execute( + () -> { + futures.add(verifyFileOpNodeForActionLookupData(graph, fileOpDataMap, lookupData)); + allAdded.countDown(); + }); + } + + allAdded.await(); + // Should not raise any exceptions. + var unused = Futures.whenAllSucceed(futures).call(() -> null, directExecutor()).get(); + } + + private static ListenableFuture verifyFileOpNodeForActionLookupKey( + InMemoryGraph graph, FileOpNodeMemoizingLookup fileOpDataMap, ActionLookupKey lookupKey) { + // For action lookup values, verifies that the file dependencies are an exact match for the ones + // in the transitive closure. + Function verify = + node -> { + var nodes = new HashSet(); + var sources = new HashSet(); + flattenNodeOrEmpty(node, nodes, sources, new HashSet<>()); + assertWithMessage("for key=%s", lookupKey) + .that(nodes) + .isEqualTo(collectTransitiveFileOpNodes(graph, lookupKey)); + return null; + }; + switch (fileOpDataMap.computeNode(lookupKey)) { + case FileOpNodeOrEmpty nodeOrEmpty: + var unusedNull = verify.apply(nodeOrEmpty); + return immediateVoidFuture(); + case FutureFileOpNode future: + return Futures.transform(future, verify, directExecutor()); + } + } + + private static ListenableFuture verifyFileOpNodeForActionLookupData( + InMemoryGraph graph, FileOpNodeMemoizingLookup fileOpDataMap, ActionLookupData lookupData) { + // For actions, verifies that the union of the files and sources of the file op data of the + // action's owner is a superset of the file dependencies of the action. There's a small + // overapproximation here. + Function verify = + node -> { + var nodes = new HashSet(); + var sources = new HashSet(); + flattenNodeOrEmpty(node, nodes, sources, new HashSet<>()); + ImmutableSet realFileDeps = collectTransitiveFileOpNodes(graph, lookupData); + + var assertBuilder = assertWithMessage("for key=%s", lookupData); + assertBuilder.that(nodes).containsNoneIn(sources); // Sources are distinct from nodes. + // All sources are contained in the real file deps. + assertBuilder.that(realFileDeps).containsAtLeastElementsIn(sources); + + nodes.addAll(sources); + // Sources may be an overapproximation by design. In this particular case, it happens to + // be an exact match, but that could conceivably change with code changes. + assertBuilder.that(nodes).containsAtLeastElementsIn(realFileDeps); + return null; + }; + // Note, that this looks up the incrementality data for the action by its ActionLookupKey. + switch (fileOpDataMap.computeNode(lookupData.getActionLookupKey())) { + case FileOpNodeOrEmpty nodeOrEmpty: + var unusedNull = verify.apply(nodeOrEmpty); + return immediateVoidFuture(); + case FutureFileOpNode future: + return Futures.transform(future, verify, directExecutor()); + } + } + + /** + * Flattens the given node or empty node into the given sets of nodes and sources. + * + *

The given sets are modified in place. + */ + private static void flattenNodeOrEmpty( + FileOpNodeOrEmpty maybeNode, + Set nodes, + Set sources, + Set visited) { + switch (maybeNode) { + case EMPTY_FILE_OP_NODE: + return; + case FileOpNode node: + flattenNode(node, nodes, sources, visited); + return; + } + } + + private static void flattenNode( + FileOpNode node, Set nodes, Set sources, Set visited) { + if (!visited.add(node)) { + return; + } + switch (node) { + case FileKey file: + nodes.add(file); + break; + case DirectoryListingKey directory: + nodes.add(directory); + break; + case NestedFileOpNodes nested: + for (int i = 0; i < nested.analysisDependenciesCount(); i++) { + flattenNode(nested.getAnalysisDependency(i), nodes, sources, visited); + } + break; + case NestedFileOpNodesWithSources withSources: + for (int i = 0; i < withSources.analysisDependenciesCount(); i++) { + flattenNode(withSources.getAnalysisDependency(i), nodes, sources, visited); + } + for (int i = 0; i < withSources.sourceCount(); i++) { + sources.add(withSources.getSource(i)); + } + break; + } + } + + private static ImmutableSet collectTransitiveFileOpNodes( + InMemoryGraph graph, SkyKey key) { + var visited = new HashSet(); + var nodes = new HashSet(); + collectTransitiveFileOpNodes(graph, key, visited, nodes); + return ImmutableSet.copyOf(nodes); + } + + private static void collectTransitiveFileOpNodes( + InMemoryGraph graph, SkyKey key, Set visited, Set nodes) { + if (!visited.add(key)) { + return; + } + if (key instanceof FileOpNode fileOp) { + // The FileOpNodeMemoizingLookup doesn't recurse beyond FileKey or DirectoryListingKeys. The + // inner details + // of those entries are handled by FileDependencySerializer. + nodes.add(fileOp); + return; + } + for (SkyKey dep : graph.getIfPresent(key).getDirectDeps()) { + collectTransitiveFileOpNodes(graph, dep, visited, nodes); + } + } +}