Skip to content

Commit

Permalink
Optimize representation of runfiles in compact execution log
Browse files Browse the repository at this point in the history
Runfiles trees are now represented with a custom `RunfilesTree` message in the compact execution log. This allows using `InputSet`s to representing all artifacts staged at canonical locations, with only symlinks and root symlinks stored flattened and with explicit runfiles paths.

Since runfile paths can collide, this change makes it necessary to preserve the order of elements in an `InputSet`. The previous representation as repeated ID fields for each type (file, symlink, directory) made this impossible, so the representation has been modified to reference all direct entry IDs in a single repeated field. Since this also reduces the potential for type mismatches between the ID field type and the referenced message type, all other typed IDs are replaced with untyped ID fields. By slightly tweaking the way IDs are generated for nested entries and not emitting IDs for entries that are never referenced (e.g. `Spawn`s), IDs are now consecutive, which simplifies the (possibly concurrent) bookkeeping for consumers by allowing them to use an array to store the entries.

Progress on #18643.

RELNOTES: The compact execution log now stores runfiles in a more compact representation that should reduce the memory overhead and log output size, in particular for test spawns. This change required breaking changes to the (experimental) log format.

Closes #23321.

PiperOrigin-RevId: 676773599
Change-Id: I010653681ffa44557142bf25009e9178b5d68515
  • Loading branch information
fmeum authored and iancha1992 committed Sep 20, 2024
1 parent 55f41a1 commit 4fbc3f4
Show file tree
Hide file tree
Showing 23 changed files with 2,136 additions and 292 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.actions.cache.OutputMetadataStore;
import com.google.devtools.build.lib.analysis.SymlinkEntry;
import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue.RunfileSymlinksMode;
import com.google.devtools.build.lib.bugreport.BugReporter;
import com.google.devtools.build.lib.clock.Clock;
Expand Down Expand Up @@ -88,6 +89,37 @@ public boolean isBuildRunfileLinks() {
public String getWorkspaceName() {
return wrapped.getWorkspaceName();
}

@Override
public NestedSet<Artifact> getArtifactsAtCanonicalLocationsForLogging() {
return wrapped.getArtifactsAtCanonicalLocationsForLogging();
}

@Override
public Iterable<PathFragment> getEmptyFilenamesForLogging() {
return wrapped.getEmptyFilenamesForLogging();
}

@Override
public NestedSet<SymlinkEntry> getSymlinksForLogging() {
return wrapped.getSymlinksForLogging();
}

@Override
public NestedSet<SymlinkEntry> getRootSymlinksForLogging() {
return wrapped.getRootSymlinksForLogging();
}

@Nullable
@Override
public Artifact getRepoMappingManifestForLogging() {
return wrapped.getRepoMappingManifestForLogging();
}

@Override
public boolean isLegacyExternalRunfiles() {
return wrapped.isLegacyExternalRunfiles();
}
}

/**
Expand Down
3 changes: 3 additions & 0 deletions src/main/java/com/google/devtools/build/lib/actions/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ java_library(
":thread_state_receiver",
"//src/main/java/com/google/devtools/build/lib/analysis:config/build_configuration",
"//src/main/java/com/google/devtools/build/lib/analysis:config/core_options",
"//src/main/java/com/google/devtools/build/lib/analysis:symlink_entry",
"//src/main/java/com/google/devtools/build/lib/analysis/platform",
"//src/main/java/com/google/devtools/build/lib/bugreport",
"//src/main/java/com/google/devtools/build/lib/buildeventstream",
Expand Down Expand Up @@ -553,8 +554,10 @@ java_library(
deps = [
":artifacts",
"//src/main/java/com/google/devtools/build/lib/analysis:config/build_configuration",
"//src/main/java/com/google/devtools/build/lib/analysis:symlink_entry",
"//src/main/java/com/google/devtools/build/lib/collect/nestedset",
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
"//third_party:jsr305",
],
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,12 @@

package com.google.devtools.build.lib.actions;

import com.google.devtools.build.lib.analysis.SymlinkEntry;
import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue.RunfileSymlinksMode;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.util.Map;
import javax.annotation.Nullable;

/** Lazy wrapper for a single runfiles tree. */
// TODO(bazel-team): Ideally we could refer to Runfiles objects directly here, but current package
Expand Down Expand Up @@ -45,4 +47,32 @@ public interface RunfilesTree {

/** Returns the name of the workspace that the build is occurring in. */
String getWorkspaceName();

/**
* Returns artifacts the runfiles tree contain symlinks to at their canonical locations.
*
* <p>This does <b>not</b> include artifacts that only the symlinks and root symlinks point to.
*/
NestedSet<Artifact> getArtifactsAtCanonicalLocationsForLogging();

/**
* Returns the set of names of implicit empty files to materialize.
*
* <p>If this runfiles tree does not implicitly add empty files, implementations should have a
* dedicated fast path that returns an empty set without traversing the tree.
*/
Iterable<PathFragment> getEmptyFilenamesForLogging();

/** Returns the set of custom symlink entries. */
NestedSet<SymlinkEntry> getSymlinksForLogging();

/** Returns the set of root symlinks. */
NestedSet<SymlinkEntry> getRootSymlinksForLogging();

/** Returns the repo mapping manifest if it exists. */
@Nullable
Artifact getRepoMappingManifestForLogging();

/** Whether this runfiles tree materializes external runfiles also at their legacy locations. */
boolean isLegacyExternalRunfiles();
}
23 changes: 14 additions & 9 deletions src/main/java/com/google/devtools/build/lib/analysis/Runfiles.java
Original file line number Diff line number Diff line change
Expand Up @@ -245,10 +245,17 @@ public NestedSet<SymlinkEntry> getSymlinks() {

@Override
public Depset /*<String>*/ getEmptyFilenamesForStarlark() {
return Depset.of(String.class, getEmptyFilenames());
return Depset.of(
String.class,
NestedSetBuilder.wrap(
Order.STABLE_ORDER,
Iterables.transform(getEmptyFilenames(), PathFragment::getPathString)));
}

public NestedSet<String> getEmptyFilenames() {
public Iterable<PathFragment> getEmptyFilenames() {
if (emptyFilesSupplier == DUMMY_EMPTY_FILES_SUPPLIER) {
return ImmutableList.of();
}
Set<PathFragment> manifestKeys =
Streams.concat(
symlinks.toList().stream().map(SymlinkEntry::getPath),
Expand All @@ -259,13 +266,7 @@ public NestedSet<String> getEmptyFilenames() {
? artifact.getOutputDirRelativePath(false)
: artifact.getRunfilesPath()))
.collect(ImmutableSet.toImmutableSet());
Iterable<PathFragment> emptyKeys = emptyFilesSupplier.getExtraPaths(manifestKeys);
return NestedSetBuilder.<String>stableOrder()
.addAll(
Streams.stream(emptyKeys)
.map(PathFragment::toString)
.collect(ImmutableList.toImmutableList()))
.build();
return emptyFilesSupplier.getExtraPaths(manifestKeys);
}

/**
Expand Down Expand Up @@ -383,6 +384,10 @@ public Map<PathFragment, Artifact> getRunfilesInputs(
return builder.build();
}

public boolean isLegacyExternalRunfiles() {
return legacyExternalRunfiles;
}

/**
* Helper class to handle munging the paths of external artifacts.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,14 +88,16 @@ public final class RunfilesSupport {
private static final String OUTPUT_MANIFEST_BASENAME = "MANIFEST";
private static final String REPO_MAPPING_MANIFEST_EXT = ".repo_mapping";

private static class RunfilesTreeImpl implements RunfilesTree {
/** The implementation of {@link RunfilesTree}. */
@VisibleForTesting
public static class RunfilesTreeImpl implements RunfilesTree {

private static final WeakReference<Map<PathFragment, Artifact>> NOT_YET_COMPUTED =
new WeakReference<>(null);

private final PathFragment execPath;
private final Runfiles runfiles;
private final Artifact repoMappingManifest;
@Nullable private final Artifact repoMappingManifest;

/**
* The cached runfiles mapping. Possible values:
Expand All @@ -119,7 +121,7 @@ private static class RunfilesTreeImpl implements RunfilesTree {
private RunfilesTreeImpl(
PathFragment execPath,
Runfiles runfiles,
Artifact repoMappingManifest,
@Nullable Artifact repoMappingManifest,
boolean buildRunfileLinks,
boolean cacheMapping,
RunfileSymlinksMode runfileSymlinksMode) {
Expand All @@ -131,6 +133,17 @@ private RunfilesTreeImpl(
this.cachedMapping = cacheMapping ? NOT_YET_COMPUTED : null;
}

@VisibleForTesting
public RunfilesTreeImpl(PathFragment execPath, Runfiles runfiles) {
this(
execPath,
runfiles,
/* repoMappingManifest= */ null,
/* buildRunfileLinks= */ false,
/* cacheMapping= */ false,
RunfileSymlinksMode.EXTERNAL);
}

@Override
public PathFragment getExecPath() {
return execPath;
Expand Down Expand Up @@ -167,6 +180,37 @@ public NestedSet<Artifact> getArtifacts() {
return runfiles.getAllArtifacts();
}

@Override
public NestedSet<Artifact> getArtifactsAtCanonicalLocationsForLogging() {
return runfiles.getArtifacts();
}

@Override
public Iterable<PathFragment> getEmptyFilenamesForLogging() {
return runfiles.getEmptyFilenames();
}

@Override
public NestedSet<SymlinkEntry> getSymlinksForLogging() {
return runfiles.getSymlinks();
}

@Override
public NestedSet<SymlinkEntry> getRootSymlinksForLogging() {
return runfiles.getRootSymlinks();
}

@Nullable
@Override
public Artifact getRepoMappingManifestForLogging() {
return repoMappingManifest;
}

@Override
public boolean isLegacyExternalRunfiles() {
return runfiles.isLegacyExternalRunfiles();
}

@Override
public RunfileSymlinksMode getSymlinksMode() {
return runfileSymlinksMode;
Expand Down
1 change: 1 addition & 0 deletions src/main/java/com/google/devtools/build/lib/bazel/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/exec:executor_builder",
"//src/main/java/com/google/devtools/build/lib/exec:module_action_context_registry",
"//src/main/java/com/google/devtools/build/lib/exec:spawn_log_context",
"//src/main/java/com/google/devtools/build/lib/packages/semantics",
"//src/main/java/com/google/devtools/build/lib/remote/options",
"//src/main/java/com/google/devtools/build/lib/util:abrupt_exit_exception",
"//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import com.google.devtools.build.lib.exec.ExpandedSpawnLogContext.Encoding;
import com.google.devtools.build.lib.exec.ModuleActionContextRegistry;
import com.google.devtools.build.lib.exec.SpawnLogContext;
import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
import com.google.devtools.build.lib.remote.options.RemoteOptions;
import com.google.devtools.build.lib.runtime.BlazeModule;
import com.google.devtools.build.lib.runtime.CommandEnvironment;
Expand Down Expand Up @@ -104,6 +105,10 @@ private void initOutputs(CommandEnvironment env) throws IOException {
new CompactSpawnLogContext(
outputPath,
env.getExecRoot().asFragment(),
env.getWorkspaceName(),
env.getOptions()
.getOptions(BuildLanguageOptions.class)
.experimentalSiblingRepositoryLayout,
env.getOptions().getOptions(RemoteOptions.class),
env.getRuntime().getFileSystem().getDigestFunction(),
env.getXattrProvider());
Expand Down
3 changes: 1 addition & 2 deletions src/main/java/com/google/devtools/build/lib/exec/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,6 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/vfs",
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
"//third_party:guava",
"//third_party:jsr305",
],
)

Expand All @@ -277,6 +276,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/actions:runfiles_tree",
"//src/main/java/com/google/devtools/build/lib/analysis:symlink_entry",
"//src/main/java/com/google/devtools/build/lib/analysis/platform:platform_utils",
"//src/main/java/com/google/devtools/build/lib/cmdline",
"//src/main/java/com/google/devtools/build/lib/collect/nestedset",
Expand Down Expand Up @@ -306,7 +306,6 @@ java_library(
],
deps = [
"//src/main/java/com/google/devtools/build/lib/profiler",
"//src/main/java/com/google/devtools/build/lib/util:pair",
"//src/main/java/com/google/devtools/build/lib/util/io:io-proto",
"//src/main/protobuf:spawn_java_proto",
"//third_party:guava",
Expand Down
Loading

0 comments on commit 4fbc3f4

Please sign in to comment.