Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[7.4.0] Compact execution log improvements #23713

Merged
merged 12 commits into from
Sep 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 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 @@ -157,6 +157,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 @@ -498,6 +499,7 @@ 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",
"//src/main/java/net/starlark/java/eval",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.util.Collection;
import java.util.LinkedHashMap;
import java.util.Map;
import javax.annotation.Nullable;

Expand Down Expand Up @@ -119,4 +120,13 @@ public boolean isBuildRunfileLinks(PathFragment runfilesDir) {
public RunfilesSupplier withOverriddenRunfilesDir(PathFragment newRunfilesDir) {
throw new UnsupportedOperationException();
}

@Override
public Map<Artifact, RunfilesTree> getRunfilesTreesForLogging() {
Map<Artifact, RunfilesTree> result = new LinkedHashMap<>();
for (RunfilesSupplier supplier : suppliers) {
result.putAll(supplier.getRunfilesTreesForLogging());
}
return result;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -63,4 +63,9 @@ public boolean isBuildRunfileLinks(PathFragment runfilesDir) {
public RunfilesSupplier withOverriddenRunfilesDir(PathFragment newRunfilesDir) {
throw new UnsupportedOperationException();
}

@Override
public Map<Artifact, RunfilesTree> getRunfilesTreesForLogging() {
return ImmutableMap.of();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
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;
Expand Down Expand Up @@ -66,4 +67,44 @@ public interface RunfilesSupplier extends StarlarkValue {
* #getRunfilesDirs} returns a set of size 1.
*/
RunfilesSupplier withOverriddenRunfilesDir(PathFragment newRunfilesDir);

/**
* Returns information about the runfiles trees in this supplier for logging purposes.
*
* <p>The field names are chosen to minimize the diff with the actual RunfilesTree class in Bazel
* 8, which simplifies cherry-picks.
*
* @param getArtifactsAtCanonicalLocationsForLogging 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.
* @param getEmptyFilenamesForLogging 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.
* @param getSymlinksForLogging Returns the set of custom symlink entries.
* @param getRootSymlinksForLogging Returns the set of root symlinks.
* @param getRepoMappingManifestForLogging Returns the repo mapping manifest if it exists.
* @param isLegacyExternalRunfiles Whether this runfiles tree materializes external runfiles also
* at their legacy locations.
* @param isMappingCached Whether this runfiles tree is likely to be used by multiple spawns.
*/
record RunfilesTree(
PathFragment getExecPath,
NestedSet<Artifact> getArtifactsAtCanonicalLocationsForLogging,
Iterable<PathFragment> getEmptyFilenamesForLogging,
NestedSet<SymlinkEntry> getSymlinksForLogging,
NestedSet<SymlinkEntry> getRootSymlinksForLogging,
@Nullable Artifact getRepoMappingManifestForLogging,
boolean isLegacyExternalRunfiles,
boolean isMappingCached) {}

/**
* A map from runfiles middleman artifacts to the {@link RunfilesTree} they represent.
*
* <p>This is used for the purposes of the execution log only and matches the architecture of
* Bazel 8, in which a runfiles tree can be obtained from its middleman via {@link
* InputMetadataProvider}.
*/
Map<Artifact, RunfilesTree> getRunfilesTreesForLogging();
}
Empty file.
Original file line number Diff line number Diff line change
Expand Up @@ -250,10 +250,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 @@ -264,13 +271,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 @@ -387,6 +388,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 @@ -563,6 +563,22 @@ public RunfilesSupplier withOverriddenRunfilesDir(PathFragment newRunfilesDir) {
runfiles,
repoMappingManifest,
runfileSymlinksMode,
buildRunfileLinks);
buildRunfileLinks,
runfilesMiddleman);
}

@Override
public Map<Artifact, RunfilesTree> getRunfilesTreesForLogging() {
return ImmutableMap.of(
getRunfilesMiddleman(),
new RunfilesTree(
getRunfilesDirectoryExecPath(),
runfiles.getArtifacts(),
runfiles.getEmptyFilenames(),
runfiles.getSymlinks(),
runfiles.getRootSymlinks(),
getRepoMappingManifest(),
runfiles.isLegacyExternalRunfiles(),
/* isMappingCached= */ false));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ public final class SingleRunfilesSupplier implements RunfilesSupplier {
@Nullable private final Artifact repoMappingManifest;
private final RunfileSymlinksMode runfileSymlinksMode;
private final boolean buildRunfileLinks;
@Nullable private final Artifact runfilesMiddleman;

/**
* Same as {@link SingleRunfilesSupplier#SingleRunfilesSupplier(PathFragment, Runfiles, boolean,
Expand All @@ -54,14 +55,16 @@ public static SingleRunfilesSupplier createCaching(
Runfiles runfiles,
@Nullable Artifact repoMappingManifest,
RunfileSymlinksMode runfileSymlinksMode,
boolean buildRunfileLinks) {
boolean buildRunfileLinks,
@Nullable Artifact runfilesMiddleman) {
return new SingleRunfilesSupplier(
runfilesDir,
runfiles,
/* runfilesCachingEnabled= */ true,
repoMappingManifest,
runfileSymlinksMode,
buildRunfileLinks);
buildRunfileLinks,
runfilesMiddleman);
}

/**
Expand All @@ -78,14 +81,16 @@ public SingleRunfilesSupplier(
Runfiles runfiles,
@Nullable Artifact repoMappingManifest,
RunfileSymlinksMode runfileSymlinksMode,
boolean buildRunfileLinks) {
boolean buildRunfileLinks,
@Nullable Artifact runfilesMiddleman) {
this(
runfilesDir,
runfiles,
/* runfilesCachingEnabled= */ false,
repoMappingManifest,
runfileSymlinksMode,
buildRunfileLinks);
buildRunfileLinks,
runfilesMiddleman);
}

private SingleRunfilesSupplier(
Expand All @@ -94,7 +99,8 @@ private SingleRunfilesSupplier(
boolean runfilesCachingEnabled,
@Nullable Artifact repoMappingManifest,
RunfileSymlinksMode runfileSymlinksMode,
boolean buildRunfileLinks) {
boolean buildRunfileLinks,
@Nullable Artifact runfilesMiddleman) {
this(
runfilesDir,
runfiles,
Expand All @@ -105,7 +111,8 @@ private SingleRunfilesSupplier(
/* eventHandler= */ null, /* location= */ null, repoMappingManifest),
repoMappingManifest,
runfileSymlinksMode,
buildRunfileLinks);
buildRunfileLinks,
runfilesMiddleman);
}

private SingleRunfilesSupplier(
Expand All @@ -114,14 +121,16 @@ private SingleRunfilesSupplier(
Supplier<Map<PathFragment, Artifact>> runfilesInputs,
@Nullable Artifact repoMappingManifest,
RunfileSymlinksMode runfileSymlinksMode,
boolean buildRunfileLinks) {
boolean buildRunfileLinks,
@Nullable Artifact runfilesMiddleman) {
checkArgument(!runfilesDir.isAbsolute());
this.runfilesDir = checkNotNull(runfilesDir);
this.runfiles = checkNotNull(runfiles);
this.runfilesInputs = checkNotNull(runfilesInputs);
this.repoMappingManifest = repoMappingManifest;
this.runfileSymlinksMode = runfileSymlinksMode;
this.buildRunfileLinks = buildRunfileLinks;
this.runfilesMiddleman = runfilesMiddleman;
}

@Override
Expand Down Expand Up @@ -163,7 +172,28 @@ public SingleRunfilesSupplier withOverriddenRunfilesDir(PathFragment newRunfiles
runfilesInputs,
repoMappingManifest,
runfileSymlinksMode,
buildRunfileLinks);
buildRunfileLinks,
runfilesMiddleman);
}

@Override
public Map<Artifact, RunfilesTree> getRunfilesTreesForLogging() {
if (runfilesMiddleman == null) {
// This can only happen with the "new_runfiles_supplier" Python builtin function, which is
// unused in builtin rules as well as in the latest rules_python version.
return ImmutableMap.of();
}
return ImmutableMap.of(
runfilesMiddleman,
new RunfilesTree(
runfilesDir,
runfiles.getArtifacts(),
runfiles.getEmptyFilenames(),
runfiles.getSymlinks(),
runfiles.getRootSymlinks(),
repoMappingManifest,
runfiles.isLegacyExternalRunfiles(),
runfilesInputs instanceof RunfilesCacher));
}

/** Softly caches the result of {@link Runfiles#getRunfilesInputs}. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,8 @@ private TestParams createTestAction(int shards)
runfilesSupport.getRunfiles(),
runfilesSupport.getRepoMappingManifest(),
runfilesSupport.getRunfileSymlinksMode(),
runfilesSupport.isBuildRunfileLinks());
runfilesSupport.isBuildRunfileLinks(),
runfilesSupport.getRunfilesMiddleman());
} else {
testRunfilesSupplier = runfilesSupport;
}
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: 2 additions & 1 deletion src/main/java/com/google/devtools/build/lib/exec/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,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_supplier",
"//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 @@ -297,8 +298,8 @@ java_library(
],
deps = [
"//src/main/java/com/google/devtools/build/lib/profiler",
"//src/main/java/com/google/devtools/build/lib/util/io",
"//src/main/java/com/google/devtools/build/lib/util:pair",
"//src/main/java/com/google/devtools/build/lib/util/io",
"//src/main/protobuf:spawn_java_proto",
"//third_party:guava",
"//third_party:jsr305",
Expand Down
Loading
Loading