diff --git a/src/main/java/com/google/devtools/build/lib/actions/CompositeRunfilesSupplier.java b/src/main/java/com/google/devtools/build/lib/actions/CompositeRunfilesSupplier.java index 24aef85b5bdca3..c29b5c69500775 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/CompositeRunfilesSupplier.java +++ b/src/main/java/com/google/devtools/build/lib/actions/CompositeRunfilesSupplier.java @@ -65,6 +65,20 @@ public static RunfilesSupplier of(RunfilesSupplier supplier1, RunfilesSupplier s this.suppliers = suppliers; } + @Override + public boolean equals(Object other) { + if (!(other instanceof CompositeRunfilesSupplier)) { + return false; + } + CompositeRunfilesSupplier that = (CompositeRunfilesSupplier) other; + return suppliers.equals(that.suppliers); + } + + @Override + public int hashCode() { + return suppliers.hashCode(); + } + @Override public NestedSet getArtifacts() { NestedSetBuilder result = NestedSetBuilder.stableOrder(); diff --git a/src/main/java/com/google/devtools/build/lib/actions/EmptyRunfilesSupplier.java b/src/main/java/com/google/devtools/build/lib/actions/EmptyRunfilesSupplier.java index 5c32d6fca10573..44086cbe7fb81f 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/EmptyRunfilesSupplier.java +++ b/src/main/java/com/google/devtools/build/lib/actions/EmptyRunfilesSupplier.java @@ -24,12 +24,22 @@ import java.util.Map; /** Empty implementation of RunfilesSupplier */ -public class EmptyRunfilesSupplier implements RunfilesSupplier { +public final class EmptyRunfilesSupplier implements RunfilesSupplier { @AutoCodec public static final EmptyRunfilesSupplier INSTANCE = new EmptyRunfilesSupplier(); private EmptyRunfilesSupplier() {} + @Override + public boolean equals(Object other) { + return (other instanceof EmptyRunfilesSupplier); + } + + @Override + public int hashCode() { + return 0; + } + @Override public NestedSet getArtifacts() { return NestedSetBuilder.stableOrder().build(); diff --git a/src/main/java/com/google/devtools/build/lib/analysis/SingleRunfilesSupplier.java b/src/main/java/com/google/devtools/build/lib/analysis/SingleRunfilesSupplier.java index 56dc97d85daf5d..126c0e98548430 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/SingleRunfilesSupplier.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/SingleRunfilesSupplier.java @@ -27,6 +27,7 @@ import com.google.devtools.build.lib.vfs.PathFragment; import java.lang.ref.SoftReference; import java.util.Map; +import java.util.Objects; import java.util.function.Supplier; import javax.annotation.Nullable; @@ -47,6 +48,7 @@ public static SingleRunfilesSupplier create(RunfilesSupport runfilesSupport) { return new SingleRunfilesSupplier( runfilesSupport.getRunfilesDirectoryExecPath(), runfilesSupport.getRunfiles(), + /*runfilesCachingEnabled=*/ false, /*manifest=*/ null, runfilesSupport.isBuildRunfileLinks(), runfilesSupport.isRunfilesEnabled()); @@ -68,7 +70,7 @@ public static SingleRunfilesSupplier createCaching( return new SingleRunfilesSupplier( runfilesDir, runfiles, - new RunfilesCacher(runfiles), + /*runfilesCachingEnabled=*/ true, /*manifest=*/ null, buildRunfileLinks, runfileLinksEnabled); @@ -95,7 +97,25 @@ public SingleRunfilesSupplier( this( runfilesDir, runfiles, - () -> runfiles.getRunfilesInputs(/*eventHandler=*/ null, /*location=*/ null), + /*runfilesCachingEnabled=*/ false, + manifest, + buildRunfileLinks, + runfileLinksEnabled); + } + + private SingleRunfilesSupplier( + PathFragment runfilesDir, + Runfiles runfiles, + boolean runfilesCachingEnabled, + @Nullable Artifact manifest, + boolean buildRunfileLinks, + boolean runfileLinksEnabled) { + this( + runfilesDir, + runfiles, + runfilesCachingEnabled + ? new RunfilesCacher(runfiles) + : () -> runfiles.getRunfilesInputs(/*eventHandler=*/ null, /*location=*/ null), manifest, buildRunfileLinks, runfileLinksEnabled); @@ -117,6 +137,26 @@ private SingleRunfilesSupplier( this.runfileLinksEnabled = runfileLinksEnabled; } + @Override + public boolean equals(Object other) { + if (!(other instanceof SingleRunfilesSupplier)) { + return false; + } + + SingleRunfilesSupplier that = (SingleRunfilesSupplier) other; + // Not dependent on runfilesInputs which is only used for enabling caching. + return (Objects.equals(runfilesDir, that.runfilesDir) + && Objects.equals(runfiles, that.runfiles) + && Objects.equals(manifest, that.manifest) + && (buildRunfileLinks == that.buildRunfileLinks) + && (runfileLinksEnabled == that.runfileLinksEnabled)); + } + + @Override + public int hashCode() { + return Objects.hash(runfilesDir, runfiles, manifest, buildRunfileLinks, runfileLinksEnabled); + } + @Override public NestedSet getArtifacts() { return runfiles.getAllArtifacts(); diff --git a/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java index 5b9df88233f817..8dab93d4aaf738 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java @@ -269,6 +269,11 @@ public ArtifactPathResolver getPathResolver() { return actionExecutionContext.getPathResolver(); } + @Override + public SpawnInputExpander getSpawnInputExpander() { + return spawnInputExpander; + } + @Override public void lockOutputFiles() throws InterruptedException { if (stopConcurrentSpawns != null) { diff --git a/src/main/java/com/google/devtools/build/lib/exec/BUILD b/src/main/java/com/google/devtools/build/lib/exec/BUILD index 0fa09aed72166d..bee9f384adac2f 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/BUILD +++ b/src/main/java/com/google/devtools/build/lib/exec/BUILD @@ -275,6 +275,7 @@ java_library( "SpawnSchedulingEvent.java", ], deps = [ + ":spawn_input_expander", ":tree_deleter", "//src/main/java/com/google/devtools/build/lib/actions", "//src/main/java/com/google/devtools/build/lib/actions:artifacts", diff --git a/src/main/java/com/google/devtools/build/lib/exec/SpawnInputExpander.java b/src/main/java/com/google/devtools/build/lib/exec/SpawnInputExpander.java index 160b965993539b..44cb0210b92584 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/SpawnInputExpander.java +++ b/src/main/java/com/google/devtools/build/lib/exec/SpawnInputExpander.java @@ -33,11 +33,13 @@ import com.google.devtools.build.lib.actions.RunfilesSupplier; import com.google.devtools.build.lib.actions.Spawn; import com.google.devtools.build.lib.actions.cache.VirtualActionInput; +import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.collect.nestedset.Order; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; import java.io.IOException; +import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -95,7 +97,7 @@ public SpawnInputExpander( this.relSymlinkBehavior = relSymlinkBehavior; } - private void addMapping( + private static void addMapping( Map inputMappings, PathFragment targetLocation, ActionInput input, @@ -215,13 +217,12 @@ void addFilesetManifest( } } - private void addInputs( + private static void addInputs( Map inputMap, - Spawn spawn, + NestedSet inputFiles, ArtifactExpander artifactExpander, PathFragment baseDirectory) { - List inputs = - ActionInputHelper.expandArtifacts(spawn.getInputFiles(), artifactExpander); + List inputs = ActionInputHelper.expandArtifacts(inputFiles, artifactExpander); for (ActionInput input : inputs) { addMapping(inputMap, input.getExecPath(), input, baseDirectory); } @@ -243,7 +244,7 @@ public SortedMap getInputMapping( MetadataProvider actionInputFileCache) throws IOException, ForbiddenActionInputException { TreeMap inputMap = new TreeMap<>(); - addInputs(inputMap, spawn, artifactExpander, baseDirectory); + addInputs(inputMap, spawn.getInputFiles(), artifactExpander, baseDirectory); addRunfilesToInputs( inputMap, spawn.getRunfilesSupplier(), @@ -254,6 +255,126 @@ public SortedMap getInputMapping( return inputMap; } + /** The interface for accessing part of the input hierarchy. */ + public interface InputWalker { + SortedMap getLeavesInputMapping() + throws IOException, ForbiddenActionInputException; + + void visitNonLeaves(InputVisitor visitor) throws IOException, ForbiddenActionInputException; + } + + /** The interface for visiting part of the input hierarchy. */ + public interface InputVisitor { + /** + * Visits a part of the input hierarchy. + * + *

{@code nodeKey} can be used as key when memoizing visited parts of the hierarchy. + */ + void visit(Object nodeKey, InputWalker walker) + throws IOException, ForbiddenActionInputException; + } + + /** + * Visits the input files hierarchy in a depth first manner. + * + *

Similar to {@link #getInputMapping} but allows for early exit, by not visiting children, + * when walking through the input hierarchy. By applying memoization, the retrieval process of the + * inputs can be speeded up. + * + *

{@code baseDirectory} is prepended to every path in the input key. This is useful if the + * mapping is used in a context where the directory relative to which the keys are interpreted is + * not the same as the execroot. + */ + public void walkInputs( + Spawn spawn, + ArtifactExpander artifactExpander, + PathFragment baseDirectory, + MetadataProvider actionInputFileCache, + InputVisitor visitor) + throws IOException, ForbiddenActionInputException { + walkNestedSetInputs(baseDirectory, spawn.getInputFiles(), artifactExpander, visitor); + + RunfilesSupplier runfilesSupplier = spawn.getRunfilesSupplier(); + visitor.visit( + // The list of variables affecting the functional expressions below. + Arrays.asList( + // Assuming that artifactExpander and actionInputFileCache, different for each spawn, + // always expand the same way. + this, // For accessing addRunfilesToInputs. + runfilesSupplier, + baseDirectory), + new InputWalker() { + @Override + public SortedMap getLeavesInputMapping() + throws IOException, ForbiddenActionInputException { + TreeMap inputMap = new TreeMap<>(); + addRunfilesToInputs( + inputMap, runfilesSupplier, actionInputFileCache, artifactExpander, baseDirectory); + return inputMap; + } + + @Override + public void visitNonLeaves(InputVisitor childVisitor) {} + }); + + Map> filesetMappings = spawn.getFilesetMappings(); + // filesetMappings is assumed to be very small, so no need to implement visitNonLeaves() for + // improved runtime. + visitor.visit( + // The list of variables affecting the functional expressions below. + Arrays.asList( + this, // For accessing addFilesetManifests. + filesetMappings, + baseDirectory), + new InputWalker() { + @Override + public SortedMap getLeavesInputMapping() + throws ForbiddenRelativeSymlinkException { + TreeMap inputMap = new TreeMap<>(); + addFilesetManifests(filesetMappings, inputMap, baseDirectory); + return inputMap; + } + + @Override + public void visitNonLeaves(InputVisitor childVisitor) {} + }); + } + + /** Walks through one level of a {@link NestedSet} of {@link ActionInput}s. */ + private void walkNestedSetInputs( + PathFragment baseDirectory, + NestedSet someInputFiles, + ArtifactExpander artifactExpander, + InputVisitor visitor) + throws IOException, ForbiddenActionInputException { + visitor.visit( + // addInputs is static so no need to add 'this' as dependent key. + Arrays.asList( + // Assuming that artifactExpander, different for each spawn, always expands the same + // way. + someInputFiles.toNode(), baseDirectory), + new InputWalker() { + @Override + public SortedMap getLeavesInputMapping() { + TreeMap inputMap = new TreeMap<>(); + addInputs( + inputMap, + NestedSetBuilder.wrap(someInputFiles.getOrder(), someInputFiles.getLeaves()), + artifactExpander, + baseDirectory); + return inputMap; + } + + @Override + public void visitNonLeaves(InputVisitor childVisitor) + throws IOException, ForbiddenActionInputException { + for (NestedSet subInputs : someInputFiles.getNonLeaves()) { + walkNestedSetInputs(baseDirectory, subInputs, artifactExpander, childVisitor); + } + } + }); + } + /** * Exception signaling that an input was not a regular file: most likely a directory. This * exception is currently never thrown in practice since we do not enforce "strict" mode. diff --git a/src/main/java/com/google/devtools/build/lib/exec/SpawnRunner.java b/src/main/java/com/google/devtools/build/lib/exec/SpawnRunner.java index 5cadd596132db3..673b6f95cb0c2d 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/SpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/exec/SpawnRunner.java @@ -161,6 +161,12 @@ interface SpawnExecutionContext { // directories? Or maybe we need a separate method to return the set of directories? ArtifactExpander getArtifactExpander(); + /** A spawn input expander. */ + // TODO(moroten): This is only used for the remote cache and remote execution to optimize + // Merkle tree generation. Having both this and the getInputMapping method seems a bit + // duplicated. + SpawnInputExpander getSpawnInputExpander(); + /** The {@link ArtifactPathResolver} to use when directly writing output files. */ default ArtifactPathResolver getPathResolver() { return ArtifactPathResolver.IDENTITY; 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 1490cc8cb77558..e2e1b987c3d6f1 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/BUILD +++ b/src/main/java/com/google/devtools/build/lib/remote/BUILD @@ -66,6 +66,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/exec:module_action_context_registry", "//src/main/java/com/google/devtools/build/lib/exec:remote_local_fallback_registry", "//src/main/java/com/google/devtools/build/lib/exec:spawn_cache", + "//src/main/java/com/google/devtools/build/lib/exec:spawn_input_expander", "//src/main/java/com/google/devtools/build/lib/exec:spawn_runner", "//src/main/java/com/google/devtools/build/lib/exec:spawn_strategy_registry", "//src/main/java/com/google/devtools/build/lib/packages", @@ -94,6 +95,7 @@ java_library( "//src/main/java/com/google/devtools/common/options", "//src/main/protobuf:failure_details_java_proto", "//third_party:auth", + "//third_party:caffeine", "//third_party:flogger", "//third_party:guava", "//third_party:jsr305", 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 7722aff0fd8937..aa48ba91656eb7 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 @@ -53,6 +53,8 @@ import build.bazel.remote.execution.v2.RequestMetadata; import build.bazel.remote.execution.v2.SymlinkNode; import build.bazel.remote.execution.v2.Tree; +import com.github.benmanes.caffeine.cache.Cache; +import com.github.benmanes.caffeine.cache.Caffeine; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.base.Strings; @@ -73,6 +75,7 @@ import com.google.devtools.build.lib.actions.ExecException; import com.google.devtools.build.lib.actions.FileArtifactValue.RemoteFileArtifactValue; import com.google.devtools.build.lib.actions.ForbiddenActionInputException; +import com.google.devtools.build.lib.actions.MetadataProvider; import com.google.devtools.build.lib.actions.Spawn; import com.google.devtools.build.lib.actions.SpawnResult; import com.google.devtools.build.lib.actions.Spawns; @@ -82,6 +85,7 @@ import com.google.devtools.build.lib.buildtool.buildevent.BuildInterruptedEvent; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.Reporter; +import com.google.devtools.build.lib.exec.SpawnInputExpander.InputWalker; import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionContext; import com.google.devtools.build.lib.profiler.Profiler; import com.google.devtools.build.lib.profiler.ProfilerTask; @@ -133,6 +137,7 @@ import java.util.Objects; import java.util.SortedMap; import java.util.TreeSet; +import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.Executor; import java.util.concurrent.atomic.AtomicBoolean; import javax.annotation.Nullable; @@ -154,6 +159,7 @@ public class RemoteExecutionService { @Nullable private final RemoteExecutionClient remoteExecutor; private final ImmutableSet filesToDownload; @Nullable private final Path captureCorruptedOutputsDir; + private final Cache merkleTreeCache; private final Scheduler scheduler; @@ -184,6 +190,14 @@ public RemoteExecutionService( this.remoteOptions = remoteOptions; this.remoteCache = remoteCache; this.remoteExecutor = remoteExecutor; + + Caffeine merkleTreeCacheBuilder = Caffeine.newBuilder().softValues(); + // remoteMerkleTreesCacheSize = 0 means limitless. + if (remoteOptions.remoteMerkleTreeCacheSize != 0) { + merkleTreeCacheBuilder.maximumSize(remoteOptions.remoteMerkleTreeCacheSize); + } + this.merkleTreeCache = merkleTreeCacheBuilder.build(); + ImmutableSet.Builder filesToDownloadBuilder = ImmutableSet.builder(); for (ActionInput actionInput : filesToDownload) { filesToDownloadBuilder.add(actionInput.getExecPath()); @@ -238,7 +252,7 @@ public static class RemoteAction { private final Spawn spawn; private final SpawnExecutionContext spawnExecutionContext; private final RemoteActionExecutionContext remoteActionExecutionContext; - private final SortedMap inputMap; + private final RemotePathResolver remotePathResolver; private final MerkleTree merkleTree; private final Digest commandHash; private final Command command; @@ -249,7 +263,7 @@ public static class RemoteAction { Spawn spawn, SpawnExecutionContext spawnExecutionContext, RemoteActionExecutionContext remoteActionExecutionContext, - SortedMap inputMap, + RemotePathResolver remotePathResolver, MerkleTree merkleTree, Digest commandHash, Command command, @@ -258,7 +272,7 @@ public static class RemoteAction { this.spawn = spawn; this.spawnExecutionContext = spawnExecutionContext; this.remoteActionExecutionContext = remoteActionExecutionContext; - this.inputMap = inputMap; + this.remotePathResolver = remotePathResolver; this.merkleTree = merkleTree; this.commandHash = commandHash; this.command = command; @@ -297,8 +311,9 @@ public String getActionId() { * Returns a {@link SortedMap} which maps from input paths for remote action to {@link * ActionInput}. */ - public SortedMap getInputMap() { - return inputMap; + public SortedMap getInputMap() + throws IOException, ForbiddenActionInputException { + return remotePathResolver.getInputMapping(spawnExecutionContext); } /** @@ -362,12 +377,53 @@ public boolean mayBeExecutedRemotely(Spawn spawn) { && Spawns.mayBeExecutedRemotely(spawn); } + private MerkleTree buildInputMerkleTree(Spawn spawn, SpawnExecutionContext context) + throws IOException, ForbiddenActionInputException { + if (remoteOptions.remoteMerkleTreeCache) { + MetadataProvider metadataProvider = context.getMetadataProvider(); + ConcurrentLinkedQueue subMerkleTrees = new ConcurrentLinkedQueue<>(); + remotePathResolver.walkInputs( + spawn, + context, + (Object nodeKey, InputWalker walker) -> { + subMerkleTrees.add(buildMerkleTreeVisitor(nodeKey, walker, metadataProvider)); + }); + return MerkleTree.merge(subMerkleTrees, digestUtil); + } else { + SortedMap inputMap = remotePathResolver.getInputMapping(context); + return MerkleTree.build(inputMap, context.getMetadataProvider(), execRoot, digestUtil); + } + } + + private MerkleTree buildMerkleTreeVisitor( + Object nodeKey, InputWalker walker, MetadataProvider metadataProvider) + throws IOException, ForbiddenActionInputException { + MerkleTree result = merkleTreeCache.getIfPresent(nodeKey); + if (result == null) { + result = uncachedBuildMerkleTreeVisitor(walker, metadataProvider); + merkleTreeCache.put(nodeKey, result); + } + return result; + } + + @VisibleForTesting + public MerkleTree uncachedBuildMerkleTreeVisitor( + InputWalker walker, MetadataProvider metadataProvider) + throws IOException, ForbiddenActionInputException { + ConcurrentLinkedQueue subMerkleTrees = new ConcurrentLinkedQueue<>(); + subMerkleTrees.add( + MerkleTree.build(walker.getLeavesInputMapping(), metadataProvider, execRoot, digestUtil)); + walker.visitNonLeaves( + (Object subNodeKey, InputWalker subWalker) -> { + subMerkleTrees.add(buildMerkleTreeVisitor(subNodeKey, subWalker, metadataProvider)); + }); + return MerkleTree.merge(subMerkleTrees, digestUtil); + } + /** Creates a new {@link RemoteAction} instance from spawn. */ public RemoteAction buildRemoteAction(Spawn spawn, SpawnExecutionContext context) throws IOException, UserExecException, ForbiddenActionInputException { - SortedMap inputMap = remotePathResolver.getInputMapping(context); - final MerkleTree merkleTree = - MerkleTree.build(inputMap, context.getMetadataProvider(), execRoot, digestUtil); + final MerkleTree merkleTree = buildInputMerkleTree(spawn, context); // Get the remote platform properties. Platform platform = PlatformUtils.getPlatformProto(spawn, remoteOptions); @@ -400,7 +456,7 @@ public RemoteAction buildRemoteAction(Spawn spawn, SpawnExecutionContext context spawn, context, remoteActionExecutionContext, - inputMap, + remotePathResolver, merkleTree, commandHash, command, diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java index 3b8344cfc8f569..e40969b547c36c 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java @@ -192,7 +192,7 @@ public void store(SpawnResult result) throws ExecException, InterruptedException if (options.experimentalGuardAgainstConcurrentChanges) { try (SilentCloseable c = prof.profile("RemoteCache.checkForConcurrentModifications")) { checkForConcurrentModifications(); - } catch (IOException e) { + } catch (IOException | ForbiddenActionInputException e) { report(Event.warn(e.getMessage())); return; } @@ -204,7 +204,8 @@ public void store(SpawnResult result) throws ExecException, InterruptedException @Override public void close() {} - private void checkForConcurrentModifications() throws IOException { + private void checkForConcurrentModifications() + throws IOException, ForbiddenActionInputException { for (ActionInput input : action.getInputMap().values()) { if (input instanceof VirtualActionInput) { continue; diff --git a/src/main/java/com/google/devtools/build/lib/remote/common/BUILD b/src/main/java/com/google/devtools/build/lib/remote/common/BUILD index 1926df1cbb19d6..e6279c38a989ff 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/common/BUILD +++ b/src/main/java/com/google/devtools/build/lib/remote/common/BUILD @@ -19,6 +19,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/actions:artifacts", "//src/main/java/com/google/devtools/build/lib/actions:file_metadata", "//src/main/java/com/google/devtools/build/lib/concurrent", + "//src/main/java/com/google/devtools/build/lib/exec:spawn_input_expander", "//src/main/java/com/google/devtools/build/lib/exec:spawn_runner", "//src/main/java/com/google/devtools/build/lib/vfs", "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", diff --git a/src/main/java/com/google/devtools/build/lib/remote/common/RemotePathResolver.java b/src/main/java/com/google/devtools/build/lib/remote/common/RemotePathResolver.java index 5b063c65af226c..d2c1c2c5448a38 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/common/RemotePathResolver.java +++ b/src/main/java/com/google/devtools/build/lib/remote/common/RemotePathResolver.java @@ -18,6 +18,8 @@ import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.ActionInputHelper; import com.google.devtools.build.lib.actions.ForbiddenActionInputException; +import com.google.devtools.build.lib.actions.Spawn; +import com.google.devtools.build.lib.exec.SpawnInputExpander; import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionContext; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; @@ -43,6 +45,10 @@ public interface RemotePathResolver { SortedMap getInputMapping(SpawnExecutionContext context) throws IOException, ForbiddenActionInputException; + void walkInputs( + Spawn spawn, SpawnExecutionContext context, SpawnInputExpander.InputVisitor visitor) + throws IOException, ForbiddenActionInputException; + /** Resolves the output path relative to input root for the given {@link Path}. */ String localPathToOutputPath(Path path); @@ -99,6 +105,20 @@ public SortedMap getInputMapping(SpawnExecutionContex return context.getInputMapping(PathFragment.EMPTY_FRAGMENT); } + @Override + public void walkInputs( + Spawn spawn, SpawnExecutionContext context, SpawnInputExpander.InputVisitor visitor) + throws IOException, ForbiddenActionInputException { + context + .getSpawnInputExpander() + .walkInputs( + spawn, + context.getArtifactExpander(), + PathFragment.EMPTY_FRAGMENT, + context.getMetadataProvider(), + visitor); + } + @Override public String localPathToOutputPath(Path path) { return path.relativeTo(execRoot).getPathString(); @@ -159,6 +179,20 @@ public SortedMap getInputMapping(SpawnExecutionContex return context.getInputMapping(PathFragment.create(checkNotNull(getWorkingDirectory()))); } + @Override + public void walkInputs( + Spawn spawn, SpawnExecutionContext context, SpawnInputExpander.InputVisitor visitor) + throws IOException, ForbiddenActionInputException { + context + .getSpawnInputExpander() + .walkInputs( + spawn, + context.getArtifactExpander(), + PathFragment.create(checkNotNull(getWorkingDirectory())), + context.getMetadataProvider(), + visitor); + } + private Path getBase() { if (incompatibleRemoteOutputPathsRelativeToInputRoot) { return execRoot.getParentDirectory(); diff --git a/src/main/java/com/google/devtools/build/lib/remote/merkletree/MerkleTree.java b/src/main/java/com/google/devtools/build/lib/remote/merkletree/MerkleTree.java index 5b33c57e53cc18..f519402d77bf23 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/merkletree/MerkleTree.java +++ b/src/main/java/com/google/devtools/build/lib/remote/merkletree/MerkleTree.java @@ -18,9 +18,14 @@ import build.bazel.remote.execution.v2.DirectoryNode; import build.bazel.remote.execution.v2.FileNode; import com.google.common.base.Preconditions; +import com.google.common.collect.ArrayListMultimap; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSortedMap; +import com.google.common.collect.ImmutableSortedSet; import com.google.common.collect.Iterables; import com.google.common.collect.Maps; +import com.google.common.collect.Multimap; +import com.google.common.collect.Sets; import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.MetadataProvider; import com.google.devtools.build.lib.profiler.Profiler; @@ -30,10 +35,14 @@ import com.google.devtools.build.lib.vfs.PathFragment; import com.google.protobuf.ByteString; import java.io.IOException; +import java.util.Collection; import java.util.HashMap; import java.util.Map; +import java.util.Objects; import java.util.SortedMap; -import java.util.concurrent.atomic.AtomicLong; +import java.util.SortedSet; +import java.util.TreeMap; +import java.util.TreeSet; import javax.annotation.Nullable; /** A merkle tree representation as defined by the remote execution api. */ @@ -66,30 +75,70 @@ public ByteString getBytes() { } } - private final Map digestDirectoryMap; - private final Map digestFileMap; + private interface MerkleTreeDirectoryVisitor { + + /** + * Visits each directory in a {@code MerkleTree}. + * + *

The order of the iteration is undefined. + * + * @param dir a directory in the {@code MerkleTree}. + */ + void visitDirectory(MerkleTree dir); + } + + private Map digestDirectoryMap; + private Map digestFileMap; + @Nullable private final Directory rootProto; private final Digest rootDigest; + private final SortedSet files; + private final SortedMap directories; private final long inputFiles; private final long inputBytes; private MerkleTree( - Map digestDirectoryMap, - Map digestFileMap, + @Nullable Directory rootProto, Digest rootDigest, + SortedSet files, + SortedMap directories, long inputFiles, long inputBytes) { - this.digestDirectoryMap = digestDirectoryMap; - this.digestFileMap = digestFileMap; - this.rootDigest = rootDigest; + this.digestDirectoryMap = null; + this.digestFileMap = null; + this.rootProto = rootProto; + this.rootDigest = Preconditions.checkNotNull(rootDigest, "rootDigest"); + this.files = Preconditions.checkNotNull(files, "files"); + this.directories = Preconditions.checkNotNull(directories, "directories"); this.inputFiles = inputFiles; this.inputBytes = inputBytes; } - /** Returns the digest of the merkle tree's root. */ + /** Returns the digest of the Merkle tree's root. */ + @Nullable + public Directory getRootProto() { + return rootProto; + } + + /** Returns the protobuf representation of the Merkle tree's root. */ public Digest getRootDigest() { return rootDigest; } + private SortedSet getFiles() { + return files; + } + + private SortedMap getDirectories() { + return directories; + } + + private void visitTree(MerkleTreeDirectoryVisitor visitor) { + visitor.visitDirectory(this); + for (MerkleTree dir : getDirectories().values()) { + dir.visitTree(visitor); + } + } + /** Returns the number of files represented by this merkle tree */ public long getInputFiles() { return inputFiles; @@ -100,14 +149,42 @@ public long getInputBytes() { return inputBytes; } + private Map getDigestDirectoryMap() { + if (this.digestDirectoryMap == null) { + Map newDigestMap = Maps.newHashMap(); + visitTree( + (dir) -> { + if (dir.getRootProto() != null) { + newDigestMap.put(dir.getRootDigest(), dir.getRootProto()); + } + }); + this.digestDirectoryMap = newDigestMap; + } + return this.digestDirectoryMap; + } + + private Map getDigestFileMap() { + if (this.digestFileMap == null) { + Map newDigestMap = Maps.newHashMap(); + visitTree( + (dir) -> { + for (DirectoryTree.FileNode file : dir.getFiles()) { + newDigestMap.put(file.getDigest(), toPathOrBytes(file)); + } + }); + this.digestFileMap = newDigestMap; + } + return this.digestFileMap; + } + @Nullable public Directory getDirectoryByDigest(Digest digest) { - return digestDirectoryMap.get(digest); + return getDigestDirectoryMap().get(digest); } @Nullable public PathOrBytes getFileByDigest(Digest digest) { - return digestFileMap.get(digest); + return getDigestFileMap().get(digest); } /** @@ -115,7 +192,7 @@ public PathOrBytes getFileByDigest(Digest digest) { * Directory} protobufs and {@link ActionInput} files. */ public Iterable getAllDigests() { - return Iterables.concat(digestDirectoryMap.keySet(), digestFileMap.keySet()); + return Iterables.concat(getDigestDirectoryMap().keySet(), getDigestFileMap().keySet()); } /** @@ -161,37 +238,97 @@ private static MerkleTree build(DirectoryTree tree, DigestUtil digestUtil) { Preconditions.checkNotNull(tree); if (tree.isEmpty()) { return new MerkleTree( - ImmutableMap.of(), ImmutableMap.of(), digestUtil.compute(new byte[0]), 0, 0); + null, + digestUtil.compute(new byte[0]), + ImmutableSortedSet.of(), + ImmutableSortedMap.of(), + 0, + 0); } - Map digestDirectoryMap = - Maps.newHashMapWithExpectedSize(tree.numDirectories()); - Map digestPathMap = Maps.newHashMapWithExpectedSize(tree.numFiles()); - Map m = new HashMap<>(); - AtomicLong inputBytes = new AtomicLong(0); + Map m = new HashMap<>(); tree.visit( (dirname, files, dirs) -> { - Directory.Builder b = Directory.newBuilder(); - for (DirectoryTree.FileNode file : files) { - b.addFiles(buildProto(file)); - digestPathMap.put(file.getDigest(), toPathOrBytes(file)); - inputBytes.addAndGet(file.getDigest().getSizeBytes()); - } + SortedMap subDirs = new TreeMap<>(); for (DirectoryTree.DirectoryNode dir : dirs) { PathFragment subDirname = dirname.getRelative(dir.getPathSegment()); - Digest protoDirDigest = - Preconditions.checkNotNull(m.remove(subDirname), "protoDirDigest was null"); - b.addDirectories(buildProto(dir, protoDirDigest)); - inputBytes.addAndGet(protoDirDigest.getSizeBytes()); + MerkleTree subMerkleTree = + Preconditions.checkNotNull(m.remove(subDirname), "subMerkleTree was null"); + subDirs.put(dir.getPathSegment(), subMerkleTree); } - Directory protoDir = b.build(); - Digest protoDirDigest = digestUtil.compute(protoDir); - digestDirectoryMap.put(protoDirDigest, protoDir); - m.put(dirname, protoDirDigest); + MerkleTree mt = buildMerkleTree(new TreeSet<>(files), subDirs, digestUtil); + m.put(dirname, mt); }); - Digest rootDigest = m.get(PathFragment.EMPTY_FRAGMENT); - inputBytes.addAndGet(rootDigest.getSizeBytes()); - return new MerkleTree( - digestDirectoryMap, digestPathMap, rootDigest, tree.numFiles(), inputBytes.get()); + MerkleTree rootMerkleTree = m.get(PathFragment.EMPTY_FRAGMENT); + Preconditions.checkState( + rootMerkleTree.getInputFiles() == tree.numFiles(), + "rootMerkleTree.getInputFiles() %s != tree.numFiles() %s", + rootMerkleTree.getInputFiles(), + tree.numFiles()); + return rootMerkleTree; + } + + public static MerkleTree merge(Collection merkleTrees, DigestUtil digestUtil) { + if (merkleTrees.isEmpty()) { + return build(new DirectoryTree(ImmutableMap.of(), 0), digestUtil); + } + + MerkleTree firstMerkleTree = merkleTrees.iterator().next(); + Digest firstRootDigest = firstMerkleTree.getRootDigest(); + if (merkleTrees.stream() + .allMatch((mt) -> Objects.equals(mt.getRootDigest(), firstRootDigest))) { + // All are the same, pick the first one. + return firstMerkleTree; + } + + // Some differ, do a full merge. + SortedSet files = Sets.newTreeSet(); + for (MerkleTree merkleTree : merkleTrees) { + files.addAll(merkleTree.getFiles()); + } + + // Group all Merkle trees per path. + Multimap allDirsToMerge = ArrayListMultimap.create(); + for (MerkleTree merkleTree : merkleTrees) { + merkleTree.getDirectories().forEach(allDirsToMerge::put); + } + // Merge the Merkle trees for each path. + SortedMap directories = new TreeMap<>(); + allDirsToMerge + .asMap() + .forEach( + (baseName, dirsToMerge) -> directories.put(baseName, merge(dirsToMerge, digestUtil))); + + return buildMerkleTree(files, directories, digestUtil); + } + + private static MerkleTree buildMerkleTree( + SortedSet files, + SortedMap directories, + DigestUtil digestUtil) { + Directory.Builder b = Directory.newBuilder(); + for (DirectoryTree.FileNode file : files) { + b.addFiles(buildProto(file)); + } + for (Map.Entry nameAndDir : directories.entrySet()) { + b.addDirectories(buildProto(nameAndDir.getKey(), nameAndDir.getValue())); + } + Directory protoDir = b.build(); + Digest protoDirDigest = digestUtil.compute(protoDir); + + long inputFiles = files.size(); + for (MerkleTree dir : directories.values()) { + inputFiles += dir.getInputFiles(); + } + + long inputBytes = protoDirDigest.getSizeBytes(); + for (DirectoryTree.FileNode file : files) { + inputBytes += file.getDigest().getSizeBytes(); + } + for (MerkleTree dir : directories.values()) { + inputBytes += dir.getInputBytes(); + } + + return new MerkleTree(protoDir, protoDirDigest, files, directories, inputFiles, inputBytes); } private static FileNode buildProto(DirectoryTree.FileNode file) { @@ -202,11 +339,8 @@ private static FileNode buildProto(DirectoryTree.FileNode file) { .build(); } - private static DirectoryNode buildProto(DirectoryTree.DirectoryNode dir, Digest protoDirDigest) { - return DirectoryNode.newBuilder() - .setName(dir.getPathSegment()) - .setDigest(protoDirDigest) - .build(); + private static DirectoryNode buildProto(String baseName, MerkleTree dir) { + return DirectoryNode.newBuilder().setName(baseName).setDigest(dir.getRootDigest()).build(); } private static PathOrBytes toPathOrBytes(DirectoryTree.FileNode file) { 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 b4896a2977dc98..89d6e1c6f5120f 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 @@ -505,6 +505,29 @@ public RemoteOutputsStrategyConverter() { + " discard the remotely cached values if they don't match the expected value.") public boolean remoteVerifyDownloads; + @Option( + name = "experimental_remote_merkle_tree_cache", + defaultValue = "false", + documentationCategory = OptionDocumentationCategory.REMOTE, + effectTags = {OptionEffectTag.UNKNOWN}, + help = + "If set to true, Merkle tree calculations will be memoized to improve the remote cache " + + "hit checking speed. The memory foot print of the cache is controlled by " + + "--experimental_remote_merkle_tree_cache_size.") + public boolean remoteMerkleTreeCache; + + @Option( + name = "experimental_remote_merkle_tree_cache_size", + defaultValue = "0", + documentationCategory = OptionDocumentationCategory.REMOTE, + effectTags = {OptionEffectTag.UNKNOWN}, + help = + "The number of Merkle trees to memoize to improve the remote cache hit checking speed. " + + "Even though the cache is automatically pruned according to Java's handling of " + + "soft references, out-of-memory errors can occur if set too high. If set to 0 " + + "(default), the cache size is unlimited.") + public long remoteMerkleTreeCacheSize; + @Option( name = "remote_download_symlink_template", defaultValue = "", diff --git a/src/test/java/com/google/devtools/build/lib/exec/local/BUILD b/src/test/java/com/google/devtools/build/lib/exec/local/BUILD index 8727600ead4392..4a053aab4e606f 100644 --- a/src/test/java/com/google/devtools/build/lib/exec/local/BUILD +++ b/src/test/java/com/google/devtools/build/lib/exec/local/BUILD @@ -28,6 +28,7 @@ java_test( "//src/main/java/com/google/devtools/build/lib/actions:localhost_capacity", "//src/main/java/com/google/devtools/build/lib/exec:bin_tools", "//src/main/java/com/google/devtools/build/lib/exec:runfiles_tree_updater", + "//src/main/java/com/google/devtools/build/lib/exec:spawn_input_expander", "//src/main/java/com/google/devtools/build/lib/exec:spawn_runner", "//src/main/java/com/google/devtools/build/lib/exec/local", "//src/main/java/com/google/devtools/build/lib/exec/local:options", diff --git a/src/test/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunnerTest.java b/src/test/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunnerTest.java index f1a608c7fd02eb..e91cc0adb790c4 100644 --- a/src/test/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunnerTest.java +++ b/src/test/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunnerTest.java @@ -49,6 +49,7 @@ import com.google.devtools.build.lib.exec.BinTools; import com.google.devtools.build.lib.exec.RunfilesTreeUpdater; import com.google.devtools.build.lib.exec.SpawnExecutingEvent; +import com.google.devtools.build.lib.exec.SpawnInputExpander; import com.google.devtools.build.lib.exec.SpawnRunner.ProgressStatus; import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionContext; import com.google.devtools.build.lib.exec.SpawnSchedulingEvent; @@ -260,6 +261,11 @@ public ArtifactExpander getArtifactExpander() { throw new UnsupportedOperationException(); } + @Override + public SpawnInputExpander getSpawnInputExpander() { + throw new UnsupportedOperationException(); + } + @Override public Duration getTimeout() { return Duration.ofMillis(timeoutMillis); diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java index 03d8fc70fb9fd6..30f13bdf32bd00 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java @@ -1386,6 +1386,97 @@ public void uploadInputsIfNotPresent_deduplicateFindMissingBlobCalls() throws Ex } } + @Test + public void buildMerkleTree_withMemoization_works() throws Exception { + // Test that Merkle tree building can be memoized. + + // TODO: Would like to check that NestedSet.getNonLeaves() is only called once per node, but + // cannot Mockito.spy on NestedSet as it is final. + + // arrange + /* + * First: + * /bar/file + * /foo1/file + * Second: + * /bar/file + * /foo2/file + */ + + // arrange + // Single node NestedSets are folded, so always add a dummy file everywhere. + ActionInput dummyFile = ActionInputHelper.fromPath("dummy"); + fakeFileCache.createScratchInput(dummyFile, "dummy"); + + ActionInput barFile = ActionInputHelper.fromPath("bar/file"); + NestedSet nodeBar = + NestedSetBuilder.create(Order.STABLE_ORDER, dummyFile, barFile); + fakeFileCache.createScratchInput(barFile, "bar"); + + ActionInput foo1File = ActionInputHelper.fromPath("foo1/file"); + NestedSet nodeFoo1 = + NestedSetBuilder.create(Order.STABLE_ORDER, dummyFile, foo1File); + fakeFileCache.createScratchInput(foo1File, "foo1"); + + ActionInput foo2File = ActionInputHelper.fromPath("foo2/file"); + NestedSet nodeFoo2 = + NestedSetBuilder.create(Order.STABLE_ORDER, dummyFile, foo2File); + fakeFileCache.createScratchInput(foo2File, "foo2"); + + NestedSet nodeRoot1 = + new NestedSetBuilder(Order.STABLE_ORDER) + .add(dummyFile) + .addTransitive(nodeBar) + .addTransitive(nodeFoo1) + .build(); + NestedSet nodeRoot2 = + new NestedSetBuilder(Order.STABLE_ORDER) + .add(dummyFile) + .addTransitive(nodeBar) + .addTransitive(nodeFoo2) + .build(); + + Spawn spawn1 = + new SimpleSpawn( + new FakeOwner("foo", "bar", "//dummy:label"), + /*arguments=*/ ImmutableList.of(), + /*environment=*/ ImmutableMap.of(), + /*executionInfo=*/ ImmutableMap.of(), + /*inputs=*/ nodeRoot1, + /*outputs=*/ ImmutableSet.of(), + ResourceSet.ZERO); + Spawn spawn2 = + new SimpleSpawn( + new FakeOwner("foo", "bar", "//dummy:label"), + /*arguments=*/ ImmutableList.of(), + /*environment=*/ ImmutableMap.of(), + /*executionInfo=*/ ImmutableMap.of(), + /*inputs=*/ nodeRoot2, + /*outputs=*/ ImmutableSet.of(), + ResourceSet.ZERO); + + FakeSpawnExecutionContext context1 = newSpawnExecutionContext(spawn1); + FakeSpawnExecutionContext context2 = newSpawnExecutionContext(spawn2); + RemoteOptions remoteOptions = Options.getDefaults(RemoteOptions.class); + remoteOptions.remoteMerkleTreeCache = true; + remoteOptions.remoteMerkleTreeCacheSize = 0; + RemoteExecutionService service = spy(newRemoteExecutionService(remoteOptions)); + + // act first time + service.buildRemoteAction(spawn1, context1); + + // assert first time + // Called for: manifests, runfiles, nodeRoot1, nodeFoo1 and nodeBar. + verify(service, times(5)).uncachedBuildMerkleTreeVisitor(any(), any()); + + // act second time + service.buildRemoteAction(spawn2, context2); + + // assert second time + // Called again for: manifests, runfiles, nodeRoot2 and nodeFoo2 but not nodeBar (cached). + verify(service, times(5 + 4)).uncachedBuildMerkleTreeVisitor(any(), any()); + } + private Spawn newSpawnFromResult(RemoteActionResult result) { return newSpawnFromResult(ImmutableMap.of(), result); } diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java index 474711092d830b..4171d01fe5b763 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnCacheTest.java @@ -146,6 +146,11 @@ public ArtifactExpander getArtifactExpander() { throw new UnsupportedOperationException(); } + @Override + public SpawnInputExpander getSpawnInputExpander() { + return new SpawnInputExpander(execRoot, /*strict*/ false); + } + @Override public Duration getTimeout() { return Duration.ZERO; @@ -159,7 +164,7 @@ public FileOutErr getFileOutErr() { @Override public SortedMap getInputMapping(PathFragment baseDirectory) throws IOException, ForbiddenActionInputException { - return new SpawnInputExpander(execRoot, /*strict*/ false) + return getSpawnInputExpander() .getInputMapping(simpleSpawn, SIMPLE_ARTIFACT_EXPANDER, baseDirectory, fakeFileCache); } diff --git a/src/test/java/com/google/devtools/build/lib/remote/merkletree/MerkleTreeTest.java b/src/test/java/com/google/devtools/build/lib/remote/merkletree/MerkleTreeTest.java index 17d714828e05eb..221fdc1638a125 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/merkletree/MerkleTreeTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/merkletree/MerkleTreeTest.java @@ -37,6 +37,7 @@ import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem; import java.io.IOException; +import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.Map; @@ -138,6 +139,49 @@ public void buildMerkleTree() throws IOException { assertThat(allDigests).asList().containsAtLeastElementsIn(inputDigests); } + @Test + public void mergeMerkleTrees() throws IOException { + // arrange + SortedMap sortedInputs1 = new TreeMap<>(); + SortedMap sortedInputs2 = new TreeMap<>(); + SortedMap sortedInputsAll = new TreeMap<>(); + Map metadata = new HashMap<>(); + + addFile("srcs/foo.cc", "foo", sortedInputs1, metadata); + addFile("srcs/bar.cc", "bar", sortedInputs2, metadata); + addFile("srcs/fizz/buzz.cc", "buzz", sortedInputs1, metadata); + addFile("srcs/fizz/fizzbuzz.cc", "fizzbuzz", sortedInputs2, metadata); + sortedInputsAll.putAll(sortedInputs1); + sortedInputsAll.putAll(sortedInputs2); + + MerkleTree treeEmpty = + MerkleTree.build( + new TreeMap<>(), new StaticMetadataProvider(metadata), execRoot, digestUtil); + MerkleTree tree1 = + MerkleTree.build(sortedInputs1, new StaticMetadataProvider(metadata), execRoot, digestUtil); + MerkleTree tree2 = + MerkleTree.build(sortedInputs2, new StaticMetadataProvider(metadata), execRoot, digestUtil); + MerkleTree treeAll = + MerkleTree.build( + sortedInputsAll, new StaticMetadataProvider(metadata), execRoot, digestUtil); + + // act + MerkleTree mergedTreeEmpty = MerkleTree.merge(Arrays.asList(), digestUtil); + MerkleTree mergedTree1 = MerkleTree.merge(Arrays.asList(tree1), digestUtil); + MerkleTree mergedTree11 = MerkleTree.merge(Arrays.asList(tree1, tree1), digestUtil); + MerkleTree mergedTree12 = MerkleTree.merge(Arrays.asList(tree1, tree2), digestUtil); + + // assert + assertThat(mergedTreeEmpty.getRootDigest()).isEqualTo(treeEmpty.getRootDigest()); + assertThat(mergedTree1.getRootDigest()).isEqualTo(tree1.getRootDigest()); + assertThat(mergedTree11.getRootDigest()).isEqualTo(tree1.getRootDigest()); + assertThat(mergedTree12.getRootDigest()).isEqualTo(treeAll.getRootDigest()); + + assertThat(mergedTree1).isSameInstanceAs(tree1); + + assertThat(mergedTree12.getAllDigests()).containsExactlyElementsIn(treeAll.getAllDigests()); + } + private Artifact addFile( String path, String content, diff --git a/src/test/java/com/google/devtools/build/lib/remote/util/FakeSpawnExecutionContext.java b/src/test/java/com/google/devtools/build/lib/remote/util/FakeSpawnExecutionContext.java index 6a6ba60a9f07df..6c719bcbdb40a4 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/util/FakeSpawnExecutionContext.java +++ b/src/test/java/com/google/devtools/build/lib/remote/util/FakeSpawnExecutionContext.java @@ -123,7 +123,12 @@ public MetadataProvider getMetadataProvider() { @Override public ArtifactExpander getArtifactExpander() { - throw new UnsupportedOperationException(); + return this::artifactExpander; + } + + @Override + public SpawnInputExpander getSpawnInputExpander() { + return new SpawnInputExpander(execRoot, /*strict*/ false); } @Override @@ -139,7 +144,7 @@ public FileOutErr getFileOutErr() { @Override public SortedMap getInputMapping(PathFragment baseDirectory) throws IOException, ForbiddenActionInputException { - return new SpawnInputExpander(execRoot, /*strict*/ false) + return getSpawnInputExpander() .getInputMapping(spawn, this::artifactExpander, baseDirectory, metadataProvider); }