Avoid all configured targets depending on all build infos
Instead, throw a MissingDepException from CachingAnalysisEnvironment if
the build info is missing, and restart the analysis of that configured
target in the ConfiguredTargetFunction if so.
This removes several Skyframe edges per configured target, since most of
them never attempt to access the build info. For builds with a few
hundred thousand targets, that's several million edges.
The build info Skyframe nodes are the currently the nodes with the
largest number of reverse deps, which creates a bottleneck in
InMemoryNodeEntry.addReverseDepAndCheckIfDone during analysis, causing
significant lock contention. In the builds we have looked at, this was
the second most common source of lock contention.
However, we have not measured any significant reduction in end-to-end
build times.
There is already an injected key for the build info factories, but it is
never actually used, which looks like an incomplete refactoring. This
change finishes that refactoring with the nice benefit of removing the
build info factories from the SkyframeExecutor (doing so is a
prerequisite for allowing multiple concurrent executions in the same
SkyframeExecutor).
PiperOrigin-RevId: 269555110
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/CachingAnalysisEnvironment.java b/src/main/java/com/google/devtools/build/lib/analysis/CachingAnalysisEnvironment.java
index da4c147..91d537b 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/CachingAnalysisEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/CachingAnalysisEnvironment.java
@@ -27,7 +27,6 @@
import com.google.devtools.build.lib.actions.ArtifactRoot;
import com.google.devtools.build.lib.actions.MiddlemanFactory;
import com.google.devtools.build.lib.analysis.buildinfo.BuildInfoCollection;
-import com.google.devtools.build.lib.analysis.buildinfo.BuildInfoFactory;
import com.google.devtools.build.lib.analysis.buildinfo.BuildInfoFactory.BuildInfoKey;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.events.ExtendedEventHandler;
@@ -359,28 +358,21 @@
@Override
public Artifact getStableWorkspaceStatusArtifact() throws InterruptedException {
- return ((WorkspaceStatusValue) skyframeEnv.getValue(WorkspaceStatusValue.BUILD_INFO_KEY))
- .getStableArtifact();
+ return getWorkspaceStatusValue().getStableArtifact();
}
@Override
public Artifact getVolatileWorkspaceStatusArtifact() throws InterruptedException {
- return ((WorkspaceStatusValue) skyframeEnv.getValue(WorkspaceStatusValue.BUILD_INFO_KEY))
- .getVolatileArtifact();
+ return getWorkspaceStatusValue().getVolatileArtifact();
}
- // See SkyframeBuildView#getWorkspaceStatusValues for the code that this method is attempting to
- // verify.
- private NullPointerException collectDebugInfoAndCrash(BuildInfoKey key, BuildConfiguration config)
- throws InterruptedException {
- String debugInfo = key + " " + config;
- Preconditions.checkState(skyframeEnv.valuesMissing(), debugInfo);
- Map<BuildInfoKey, BuildInfoFactory> buildInfoFactories = Preconditions.checkNotNull(
- PrecomputedValue.BUILD_INFO_FACTORIES.get(skyframeEnv), debugInfo);
- BuildInfoFactory buildInfoFactory =
- Preconditions.checkNotNull(buildInfoFactories.get(key), debugInfo);
- Preconditions.checkState(buildInfoFactory.isEnabled(config), debugInfo);
- throw new NullPointerException("BuildInfoCollectionValue shouldn't have been null");
+ private WorkspaceStatusValue getWorkspaceStatusValue() throws InterruptedException {
+ WorkspaceStatusValue workspaceStatusValue =
+ ((WorkspaceStatusValue) skyframeEnv.getValue(WorkspaceStatusValue.BUILD_INFO_KEY));
+ if (workspaceStatusValue == null) {
+ throw new MissingDepException("Restart due to missing build info");
+ }
+ return workspaceStatusValue;
}
@Override
@@ -389,14 +381,24 @@
BuildInfoCollectionValue collectionValue =
(BuildInfoCollectionValue) skyframeEnv.getValue(BuildInfoCollectionValue.key(key, config));
if (collectionValue == null) {
- throw collectDebugInfoAndCrash(key, config);
+ throw new MissingDepException(
+ String.format("Restart due to missing BuildInfoCollectionValue (%s %s)", key, config));
}
BuildInfoCollection collection = collectionValue.getCollection();
- return stamp ? collection.getStampedBuildInfo() : collection.getRedactedBuildInfo();
+ return stamp ? collection.getStampedBuildInfo() : collection.getRedactedBuildInfo();
}
@Override
public ActionLookupValue.ActionLookupKey getOwner() {
return owner;
}
+
+ /** Thrown in case of a missing build info key. */
+ // TODO(ulfjack): It would be better for this to be a checked exception, which requires updating
+ // all callers to pass the exception through.
+ public static class MissingDepException extends RuntimeException {
+ MissingDepException(String msg) {
+ super(msg);
+ }
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java
index 9a2d31c..1753bdc 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java
@@ -26,6 +26,7 @@
import com.google.common.collect.ImmutableSortedSet;
import com.google.devtools.build.lib.actions.ActionEnvironment;
import com.google.devtools.build.lib.analysis.buildinfo.BuildInfoFactory;
+import com.google.devtools.build.lib.analysis.buildinfo.BuildInfoFactory.BuildInfoKey;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration.Fragment;
import com.google.devtools.build.lib.analysis.config.BuildOptions;
@@ -776,11 +777,12 @@
return nativeAspectClassMap.get(key);
}
- /**
- * Returns a list of build info factories that are needed for the supported languages.
- */
- public ImmutableList<BuildInfoFactory> getBuildInfoFactories() {
- return buildInfoFactories;
+ public Map<BuildInfoKey, BuildInfoFactory> getBuildInfoFactoriesAsMap() {
+ ImmutableMap.Builder<BuildInfoKey, BuildInfoFactory> factoryMapBuilder = ImmutableMap.builder();
+ for (BuildInfoFactory factory : buildInfoFactories) {
+ factoryMapBuilder.put(factory.getKey(), factory);
+ }
+ return factoryMapBuilder.build();
}
/**
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/AnalysisPhaseRunner.java b/src/main/java/com/google/devtools/build/lib/buildtool/AnalysisPhaseRunner.java
index bb82cf2..89af14f 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/AnalysisPhaseRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/AnalysisPhaseRunner.java
@@ -14,6 +14,7 @@
package com.google.devtools.build.lib.buildtool;
import com.google.common.base.Stopwatch;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.flogger.GoogleLogger;
import com.google.devtools.build.lib.actions.BuildFailedException;
@@ -42,6 +43,7 @@
import com.google.devtools.build.lib.runtime.BlazeModule;
import com.google.devtools.build.lib.runtime.CommandEnvironment;
import com.google.devtools.build.lib.skyframe.BuildConfigurationValue;
+import com.google.devtools.build.lib.skyframe.PrecomputedValue;
import com.google.devtools.build.lib.skyframe.TargetPatternPhaseValue;
import com.google.devtools.build.lib.util.AbruptExitException;
import com.google.devtools.build.lib.util.RegexFilter;
@@ -105,6 +107,18 @@
AnalysisResult analysisResult = null;
if (request.getBuildOptions().performAnalysisPhase) {
Profiler.instance().markPhase(ProfilePhase.ANALYZE);
+
+ // The build info factories are immutable during the life time of this server. However, we
+ // sometimes clean the graph, which requires re-injecting the value, which requires a hook to
+ // do so afterwards, and there is no such hook at the server / workspace level right now. For
+ // simplicity, we keep the code here for now.
+ env.getSkyframeExecutor()
+ .injectExtraPrecomputedValues(
+ ImmutableList.of(
+ PrecomputedValue.injected(
+ PrecomputedValue.BUILD_INFO_FACTORIES,
+ env.getRuntime().getRuleClassProvider().getBuildInfoFactoriesAsMap())));
+
try (SilentCloseable c = Profiler.instance().profile("runAnalysisPhase")) {
analysisResult =
runAnalysisPhase(request, loadingResult, buildOptions, request.getMultiCpus());
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/WorkspaceBuilder.java b/src/main/java/com/google/devtools/build/lib/runtime/WorkspaceBuilder.java
index a86a406..fd45083 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/WorkspaceBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/WorkspaceBuilder.java
@@ -78,7 +78,6 @@
directories,
runtime.getActionKeyContext(),
workspaceStatusActionFactory,
- ruleClassProvider.getBuildInfoFactories(),
diffAwarenessFactories.build(),
skyFunctions.build(),
customDirtinessCheckers.build(),
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
index d1f2d63..f6f3554 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
@@ -23,6 +23,7 @@
import com.google.devtools.build.lib.analysis.AliasProvider;
import com.google.devtools.build.lib.analysis.AspectResolver;
import com.google.devtools.build.lib.analysis.CachingAnalysisEnvironment;
+import com.google.devtools.build.lib.analysis.CachingAnalysisEnvironment.MissingDepException;
import com.google.devtools.build.lib.analysis.ConfiguredAspect;
import com.google.devtools.build.lib.analysis.ConfiguredAspectFactory;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
@@ -650,6 +651,9 @@
aspectConfiguration,
view.getHostConfiguration(aspectConfiguration),
key);
+ } catch (MissingDepException e) {
+ Preconditions.checkState(env.valuesMissing());
+ return null;
} finally {
CurrentRuleTracker.endConfiguredAspect();
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BuildInfoCollectionFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/BuildInfoCollectionFunction.java
index 3425c32..5b73b60 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/BuildInfoCollectionFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/BuildInfoCollectionFunction.java
@@ -13,8 +13,8 @@
// limitations under the License.
package com.google.devtools.build.lib.skyframe;
+import com.google.common.base.Preconditions;
import com.google.common.base.Supplier;
-import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.actions.ActionKeyContext;
import com.google.devtools.build.lib.actions.Actions;
@@ -28,6 +28,7 @@
import com.google.devtools.build.lib.analysis.buildinfo.BuildInfoFactory.BuildInfoContext;
import com.google.devtools.build.lib.analysis.buildinfo.BuildInfoFactory.BuildInfoKey;
import com.google.devtools.build.lib.analysis.buildinfo.BuildInfoFactory.BuildInfoType;
+import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.skyframe.BuildInfoCollectionValue.BuildInfoKeyAndConfig;
import com.google.devtools.build.lib.vfs.PathFragment;
@@ -45,15 +46,11 @@
private final ActionKeyContext actionKeyContext;
// Supplier only because the artifact factory has not yet been created at constructor time.
private final Supplier<ArtifactFactory> artifactFactory;
- private final ImmutableMap<BuildInfoKey, BuildInfoFactory> buildInfoFactories;
BuildInfoCollectionFunction(
- ActionKeyContext actionKeyContext,
- Supplier<ArtifactFactory> artifactFactory,
- ImmutableMap<BuildInfoKey, BuildInfoFactory> buildInfoFactories) {
+ ActionKeyContext actionKeyContext, Supplier<ArtifactFactory> artifactFactory) {
this.actionKeyContext = actionKeyContext;
this.artifactFactory = artifactFactory;
- this.buildInfoFactories = buildInfoFactories;
}
@Override
@@ -74,6 +71,13 @@
RepositoryName repositoryName = RepositoryName.createFromValidStrippedName(
nameValue.getName());
+ BuildConfiguration config =
+ ((BuildConfigurationValue) result.get(keyAndConfig.getConfigKey())).getConfiguration();
+ Map<BuildInfoKey, BuildInfoFactory> buildInfoFactories =
+ PrecomputedValue.BUILD_INFO_FACTORIES.get(env);
+ BuildInfoFactory buildInfoFactory = buildInfoFactories.get(keyAndConfig.getInfoKey());
+ Preconditions.checkState(buildInfoFactory.isEnabled(config));
+
final ArtifactFactory factory = artifactFactory.get();
BuildInfoContext context =
new BuildInfoContext() {
@@ -86,15 +90,12 @@
}
};
BuildInfoCollection collection =
- buildInfoFactories
- .get(keyAndConfig.getInfoKey())
- .create(
- context,
- ((BuildConfigurationValue) result.get(keyAndConfig.getConfigKey()))
- .getConfiguration(),
- infoArtifactValue.getStableArtifact(),
- infoArtifactValue.getVolatileArtifact(),
- repositoryName);
+ buildInfoFactory.create(
+ context,
+ config,
+ infoArtifactValue.getStableArtifact(),
+ infoArtifactValue.getVolatileArtifact(),
+ repositoryName);
GeneratingActions generatingActions;
try {
generatingActions =
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
index 80c50fe..d0d3345 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
@@ -27,6 +27,7 @@
import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
import com.google.devtools.build.lib.analysis.AspectResolver;
import com.google.devtools.build.lib.analysis.CachingAnalysisEnvironment;
+import com.google.devtools.build.lib.analysis.CachingAnalysisEnvironment.MissingDepException;
import com.google.devtools.build.lib.analysis.ConfiguredAspect;
import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
@@ -904,6 +905,9 @@
depValueMap,
configConditions,
toolchainContext);
+ } catch (MissingDepException e) {
+ Preconditions.checkState(env.valuesMissing(), e.getMessage());
+ return null;
} catch (ActionConflictException e) {
throw new ConfiguredTargetFunctionException(e);
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java
index 7a4bd2f..3aaf99f 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java
@@ -36,7 +36,6 @@
import com.google.devtools.build.lib.analysis.BlazeDirectories;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.WorkspaceStatusAction.Factory;
-import com.google.devtools.build.lib.analysis.buildinfo.BuildInfoFactory;
import com.google.devtools.build.lib.analysis.config.BuildOptions;
import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget;
import com.google.devtools.build.lib.cmdline.LabelConstants;
@@ -162,7 +161,6 @@
BlazeDirectories directories,
ActionKeyContext actionKeyContext,
Factory workspaceStatusActionFactory,
- ImmutableList<BuildInfoFactory> buildInfoFactories,
Iterable<? extends DiffAwareness.Factory> diffAwarenessFactories,
ImmutableMap<SkyFunctionName, SkyFunction> extraSkyFunctions,
Iterable<SkyValueDirtinessChecker> customDirtinessCheckers,
@@ -182,7 +180,6 @@
directories,
actionKeyContext,
workspaceStatusActionFactory,
- buildInfoFactories,
extraSkyFunctions,
ExternalFileAction.DEPEND_ON_EXTERNAL_PKG_FOR_EXTERNAL_REPO_PATHS,
hardcodedBlacklistedPackagePrefixes,
@@ -959,7 +956,6 @@
protected FileSystem fileSystem;
protected BlazeDirectories directories;
protected ActionKeyContext actionKeyContext;
- protected ImmutableList<BuildInfoFactory> buildInfoFactories;
protected BuildOptions defaultBuildOptions;
private ImmutableSet<PathFragment> hardcodedBlacklistedPackagePrefixes;
private PathFragment additionalBlacklistedPackagePrefixesFile;
@@ -985,7 +981,6 @@
Preconditions.checkNotNull(fileSystem);
Preconditions.checkNotNull(directories);
Preconditions.checkNotNull(actionKeyContext);
- Preconditions.checkNotNull(buildInfoFactories);
Preconditions.checkNotNull(defaultBuildOptions);
Preconditions.checkNotNull(hardcodedBlacklistedPackagePrefixes);
Preconditions.checkNotNull(additionalBlacklistedPackagePrefixesFile);
@@ -1002,7 +997,6 @@
directories,
actionKeyContext,
workspaceStatusActionFactory,
- buildInfoFactories,
diffAwarenessFactories,
extraSkyFunctions,
customDirtinessCheckers,
@@ -1038,11 +1032,6 @@
return this;
}
- public Builder setBuildInfoFactories(ImmutableList<BuildInfoFactory> buildInfoFactories) {
- this.buildInfoFactories = buildInfoFactories;
- return this;
- }
-
public Builder setDefaultBuildOptions(BuildOptions defaultBuildOptions) {
this.defaultBuildOptions = defaultBuildOptions;
return this;
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorFactory.java b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorFactory.java
index 33f5a61..ed2f463 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutorFactory.java
@@ -13,12 +13,10 @@
// limitations under the License.
package com.google.devtools.build.lib.skyframe;
-import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.actions.ActionKeyContext;
import com.google.devtools.build.lib.analysis.BlazeDirectories;
import com.google.devtools.build.lib.analysis.WorkspaceStatusAction.Factory;
-import com.google.devtools.build.lib.analysis.buildinfo.BuildInfoFactory;
import com.google.devtools.build.lib.analysis.config.BuildOptions;
import com.google.devtools.build.lib.packages.PackageFactory;
import com.google.devtools.build.lib.rules.repository.ManagedDirectoriesKnowledge;
@@ -45,7 +43,6 @@
BlazeDirectories directories,
ActionKeyContext actionKeyContext,
Factory workspaceStatusActionFactory,
- ImmutableList<BuildInfoFactory> buildInfoFactories,
Iterable<? extends DiffAwareness.Factory> diffAwarenessFactories,
ImmutableMap<SkyFunctionName, SkyFunction> extraSkyFunctions,
Iterable<SkyValueDirtinessChecker> customDirtinessCheckers,
@@ -55,7 +52,6 @@
.setFileSystem(fileSystem)
.setDirectories(directories)
.setActionKeyContext(actionKeyContext)
- .setBuildInfoFactories(buildInfoFactories)
.setDefaultBuildOptions(defaultBuildOptions)
.setWorkspaceStatusActionFactory(workspaceStatusActionFactory)
.setDiffAwarenessFactories(diffAwarenessFactories)
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
index c19a52a..f5c1bc4 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
@@ -44,8 +44,6 @@
import com.google.devtools.build.lib.analysis.DependencyResolver.DependencyKind;
import com.google.devtools.build.lib.analysis.ResolvedToolchainContext;
import com.google.devtools.build.lib.analysis.ViewCreationFailedException;
-import com.google.devtools.build.lib.analysis.buildinfo.BuildInfoFactory;
-import com.google.devtools.build.lib.analysis.buildinfo.BuildInfoFactory.BuildInfoKey;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.analysis.config.BuildConfigurationCollection;
import com.google.devtools.build.lib.analysis.config.BuildOptions;
@@ -692,43 +690,12 @@
return artifactFactory;
}
- /**
- * Because we don't know what build-info artifacts this configured target may request, we
- * conservatively register a dep on all of them.
- */
- // TODO(bazel-team): Allow analysis to return null so the value builder can exit and wait for a
- // restart deps are not present.
- private static boolean getWorkspaceStatusValues(
- Environment env,
- BuildConfiguration config,
- ImmutableMap<BuildInfoKey, BuildInfoFactory> buildInfoFactories)
- throws InterruptedException {
- env.getValue(WorkspaceStatusValue.BUILD_INFO_KEY);
- // These factories may each create their own build info artifacts, all depending on the basic
- // build-info.txt and build-changelist.txt.
- List<SkyKey> depKeys = Lists.newArrayList();
- for (BuildInfoKey key : buildInfoFactories.keySet()) {
- if (buildInfoFactories.get(key).isEnabled(config)) {
- depKeys.add(BuildInfoCollectionValue.key(key, config));
- }
- }
- env.getValues(depKeys);
- return !env.valuesMissing();
- }
-
- /** Returns null if any build-info values are not ready. */
- @Nullable
CachingAnalysisEnvironment createAnalysisEnvironment(
ActionLookupValue.ActionLookupKey owner,
boolean isSystemEnv,
ExtendedEventHandler eventHandler,
Environment env,
- BuildConfiguration config)
- throws InterruptedException {
- if (config != null
- && !getWorkspaceStatusValues(env, config, skyframeExecutor.getBuildInfoFactories())) {
- return null;
- }
+ BuildConfiguration config) {
boolean extendedSanityChecks = config != null && config.extendedSanityChecks();
boolean allowAnalysisFailures = config != null && config.allowAnalysisFailures();
return new CachingAnalysisEnvironment(
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
index ad330bf..c00e171 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
@@ -74,8 +74,6 @@
import com.google.devtools.build.lib.analysis.TopLevelArtifactContext;
import com.google.devtools.build.lib.analysis.WorkspaceStatusAction;
import com.google.devtools.build.lib.analysis.WorkspaceStatusAction.Factory;
-import com.google.devtools.build.lib.analysis.buildinfo.BuildInfoFactory;
-import com.google.devtools.build.lib.analysis.buildinfo.BuildInfoFactory.BuildInfoKey;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.analysis.config.BuildConfigurationCollection;
import com.google.devtools.build.lib.analysis.config.BuildOptions;
@@ -284,8 +282,6 @@
protected final AtomicReference<TimestampGranularityMonitor> tsgm = new AtomicReference<>();
protected final AtomicReference<Map<String, String>> clientEnv = new AtomicReference<>();
- private final ImmutableMap<BuildInfoKey, BuildInfoFactory> buildInfoFactories;
-
// Under normal circumstances, the artifact factory persists for the life of a Blaze server, but
// since it is not yet created when we create the value builders, we have to use a supplier,
// initialized when the build view is created.
@@ -389,7 +385,6 @@
BlazeDirectories directories,
ActionKeyContext actionKeyContext,
Factory workspaceStatusActionFactory,
- ImmutableList<BuildInfoFactory> buildInfoFactories,
ImmutableMap<SkyFunctionName, SkyFunction> extraSkyFunctions,
ExternalFileAction externalFileAction,
ImmutableSet<PathFragment> hardcodedBlacklistedPackagePrefixes,
@@ -427,11 +422,6 @@
this.fileSystem = fileSystem;
this.directories = Preconditions.checkNotNull(directories);
this.actionKeyContext = Preconditions.checkNotNull(actionKeyContext);
- ImmutableMap.Builder<BuildInfoKey, BuildInfoFactory> factoryMapBuilder = ImmutableMap.builder();
- for (BuildInfoFactory factory : buildInfoFactories) {
- factoryMapBuilder.put(factory.getKey(), factory);
- }
- this.buildInfoFactories = factoryMapBuilder.build();
this.extraSkyFunctions = extraSkyFunctions;
this.hardcodedBlacklistedPackagePrefixes = hardcodedBlacklistedPackagePrefixes;
this.additionalBlacklistedPackagePrefixesFile = additionalBlacklistedPackagePrefixesFile;
@@ -595,8 +585,7 @@
() -> !skyframeActionExecutor.actionFileSystemType().inMemoryFileSystem()));
map.put(
SkyFunctions.BUILD_INFO_COLLECTION,
- new BuildInfoCollectionFunction(
- actionKeyContext, artifactFactory::get, buildInfoFactories));
+ new BuildInfoCollectionFunction(actionKeyContext, artifactFactory::get));
map.put(SkyFunctions.BUILD_INFO, new WorkspaceStatusFunction(this::makeWorkspaceStatusAction));
map.put(SkyFunctions.COVERAGE_REPORT, new CoverageReportFunction(actionKeyContext));
ActionExecutionFunction actionExecutionFunction =
@@ -1148,10 +1137,6 @@
return CacheBuilder.newBuilder().build();
}
- public ImmutableMap<BuildInfoKey, BuildInfoFactory> getBuildInfoFactories() {
- return buildInfoFactories;
- }
-
private void setShowLoadingProgress(boolean showLoadingProgressValue) {
showLoadingProgress.set(showLoadingProgressValue);
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutorFactory.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutorFactory.java
index e7c7106..e6338ed 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutorFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutorFactory.java
@@ -13,12 +13,10 @@
// limitations under the License.
package com.google.devtools.build.lib.skyframe;
-import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.actions.ActionKeyContext;
import com.google.devtools.build.lib.analysis.BlazeDirectories;
import com.google.devtools.build.lib.analysis.WorkspaceStatusAction.Factory;
-import com.google.devtools.build.lib.analysis.buildinfo.BuildInfoFactory;
import com.google.devtools.build.lib.packages.PackageFactory;
import com.google.devtools.build.lib.rules.repository.ManagedDirectoriesKnowledge;
import com.google.devtools.build.lib.util.AbruptExitException;
@@ -38,7 +36,6 @@
* @param fileSystem the Blaze file system
* @param directories Blaze directories
* @param workspaceStatusActionFactory a factory for creating WorkspaceStatusAction objects
- * @param buildInfoFactories list of BuildInfoFactories
* @return an instance of the SkyframeExecutor
* @throws AbruptExitException if the executor cannot be created
*/
@@ -48,7 +45,6 @@
BlazeDirectories directories,
ActionKeyContext actionKeyContext,
Factory workspaceStatusActionFactory,
- ImmutableList<BuildInfoFactory> buildInfoFactories,
Iterable<? extends DiffAwareness.Factory> diffAwarenessFactories,
ImmutableMap<SkyFunctionName, SkyFunction> extraSkyFunctions,
Iterable<SkyValueDirtinessChecker> customDirtinessCheckers,
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
index 489e557..614cfb0 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
@@ -40,7 +40,6 @@
import com.google.devtools.build.lib.analysis.RuleDefinition;
import com.google.devtools.build.lib.analysis.ServerDirectories;
import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
-import com.google.devtools.build.lib.analysis.buildinfo.BuildInfoFactory;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.analysis.config.BuildConfigurationCollection;
import com.google.devtools.build.lib.analysis.config.BuildOptions;
@@ -185,14 +184,12 @@
useRuleClassProvider(analysisMock.createRuleClassProvider());
}
- protected SkyframeExecutor createSkyframeExecutor(
- PackageFactory pkgFactory, ImmutableList<BuildInfoFactory> buildInfoFactories) {
+ protected SkyframeExecutor createSkyframeExecutor(PackageFactory pkgFactory) {
return BazelSkyframeExecutorConstants.newBazelSkyframeExecutorBuilder()
.setPkgFactory(pkgFactory)
.setFileSystem(fileSystem)
.setDirectories(directories)
.setActionKeyContext(actionKeyContext)
- .setBuildInfoFactories(buildInfoFactories)
.setDefaultBuildOptions(
DefaultBuildOptionsForTesting.getDefaultBuildOptionsForTest(ruleClassProvider))
.setWorkspaceStatusActionFactory(workspaceStatusActionFactory)
@@ -211,8 +208,7 @@
.getPackageFactoryBuilderForTesting(directories)
.build(ruleClassProvider, fileSystem);
useConfiguration();
- skyframeExecutor =
- createSkyframeExecutor(pkgFactory, ruleClassProvider.getBuildInfoFactories());
+ skyframeExecutor = createSkyframeExecutor(pkgFactory);
reinitializeSkyframeExecutor();
packageManager = skyframeExecutor.getPackageManager();
buildView = new BuildViewForTesting(directories, ruleClassProvider, skyframeExecutor, null);
@@ -241,7 +237,10 @@
ImmutableMap.<RepositoryName, PathFragment>of()),
PrecomputedValue.injected(
RepositoryDelegatorFunction.DEPENDENCY_FOR_UNCONDITIONAL_FETCHING,
- RepositoryDelegatorFunction.DONT_FETCH_UNCONDITIONALLY)));
+ RepositoryDelegatorFunction.DONT_FETCH_UNCONDITIONALLY),
+ PrecomputedValue.injected(
+ PrecomputedValue.BUILD_INFO_FACTORIES,
+ ruleClassProvider.getBuildInfoFactoriesAsMap())));
}
/** Resets the SkyframeExecutor, as if a clean had been executed. */
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
index d96fbbc..d548c09 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
@@ -267,7 +267,10 @@
Optional.<RootedPath>absent()),
PrecomputedValue.injected(
RepositoryDelegatorFunction.DEPENDENCY_FOR_UNCONDITIONAL_FETCHING,
- RepositoryDelegatorFunction.DONT_FETCH_UNCONDITIONALLY));
+ RepositoryDelegatorFunction.DONT_FETCH_UNCONDITIONALLY),
+ PrecomputedValue.injected(
+ PrecomputedValue.BUILD_INFO_FACTORIES,
+ ruleClassProvider.getBuildInfoFactoriesAsMap()));
PackageFactory.BuilderForTesting pkgFactoryBuilder =
analysisMock
.getPackageFactoryBuilderForTesting(directories)
@@ -284,7 +287,6 @@
.setFileSystem(fileSystem)
.setDirectories(directories)
.setActionKeyContext(actionKeyContext)
- .setBuildInfoFactories(ruleClassProvider.getBuildInfoFactories())
.setDefaultBuildOptions(
DefaultBuildOptionsForTesting.getDefaultBuildOptionsForTest(ruleClassProvider))
.setWorkspaceStatusActionFactory(workspaceStatusActionFactory)
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/ConfigurationTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/ConfigurationTestCase.java
index b714fe8..c26dae0 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/ConfigurationTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/ConfigurationTestCase.java
@@ -141,7 +141,6 @@
.setFileSystem(fileSystem)
.setDirectories(directories)
.setActionKeyContext(actionKeyContext)
- .setBuildInfoFactories(ruleClassProvider.getBuildInfoFactories())
.setDefaultBuildOptions(
DefaultBuildOptionsForTesting.getDefaultBuildOptionsForTest(ruleClassProvider))
.setWorkspaceStatusActionFactory(workspaceStatusActionFactory)
@@ -161,7 +160,10 @@
ImmutableMap.<RepositoryName, PathFragment>of()),
PrecomputedValue.injected(
RepositoryDelegatorFunction.DEPENDENCY_FOR_UNCONDITIONAL_FETCHING,
- RepositoryDelegatorFunction.DONT_FETCH_UNCONDITIONALLY)));
+ RepositoryDelegatorFunction.DONT_FETCH_UNCONDITIONALLY),
+ PrecomputedValue.injected(
+ PrecomputedValue.BUILD_INFO_FACTORIES,
+ ruleClassProvider.getBuildInfoFactoriesAsMap())));
PackageCacheOptions packageCacheOptions = Options.getDefaults(PackageCacheOptions.class);
packageCacheOptions.showLoadingProgress = true;
packageCacheOptions.globbingThreads = 7;
diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/PackageLoadingTestCase.java b/src/test/java/com/google/devtools/build/lib/packages/util/PackageLoadingTestCase.java
index 8691dd7..7fea1b1 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/util/PackageLoadingTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/util/PackageLoadingTestCase.java
@@ -117,7 +117,6 @@
.setFileSystem(fileSystem)
.setDirectories(directories)
.setActionKeyContext(actionKeyContext)
- .setBuildInfoFactories(ruleClassProvider.getBuildInfoFactories())
.setDefaultBuildOptions(
DefaultBuildOptionsForTesting.getDefaultBuildOptionsForTest(ruleClassProvider))
.build();
diff --git a/src/test/java/com/google/devtools/build/lib/pkgcache/BuildFileModificationTest.java b/src/test/java/com/google/devtools/build/lib/pkgcache/BuildFileModificationTest.java
index 1580ca3..85b40cf 100644
--- a/src/test/java/com/google/devtools/build/lib/pkgcache/BuildFileModificationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/pkgcache/BuildFileModificationTest.java
@@ -92,7 +92,6 @@
.setFileSystem(fileSystem)
.setDirectories(directories)
.setActionKeyContext(actionKeyContext)
- .setBuildInfoFactories(ruleClassProvider.getBuildInfoFactories())
.setDefaultBuildOptions(
DefaultBuildOptionsForTesting.getDefaultBuildOptionsForTest(ruleClassProvider))
.setExtraSkyFunctions(analysisMock.getSkyFunctions(directories))
diff --git a/src/test/java/com/google/devtools/build/lib/pkgcache/IncrementalLoadingTest.java b/src/test/java/com/google/devtools/build/lib/pkgcache/IncrementalLoadingTest.java
index 7ab05be..628546c 100644
--- a/src/test/java/com/google/devtools/build/lib/pkgcache/IncrementalLoadingTest.java
+++ b/src/test/java/com/google/devtools/build/lib/pkgcache/IncrementalLoadingTest.java
@@ -461,7 +461,6 @@
.setFileSystem(fs)
.setDirectories(directories)
.setActionKeyContext(actionKeyContext)
- .setBuildInfoFactories(loadingMock.createRuleClassProvider().getBuildInfoFactories())
.setDefaultBuildOptions(
DefaultBuildOptionsForTesting.getDefaultBuildOptionsForTest(ruleClassProvider))
.setDiffAwarenessFactories(ImmutableList.of(new ManualDiffAwarenessFactory()))
diff --git a/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java b/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java
index 94c9e24..b73a6763 100644
--- a/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/pkgcache/LoadingPhaseRunnerTest.java
@@ -1219,7 +1219,6 @@
.setFileSystem(fs)
.setDirectories(directories)
.setActionKeyContext(actionKeyContext)
- .setBuildInfoFactories(ruleClassProvider.getBuildInfoFactories())
.setDefaultBuildOptions(defaultBuildOptions)
.setExtraSkyFunctions(analysisMock.getSkyFunctions(directories))
.build();
diff --git a/src/test/java/com/google/devtools/build/lib/pkgcache/PackageCacheTest.java b/src/test/java/com/google/devtools/build/lib/pkgcache/PackageCacheTest.java
index ddf67e9..71ba585 100644
--- a/src/test/java/com/google/devtools/build/lib/pkgcache/PackageCacheTest.java
+++ b/src/test/java/com/google/devtools/build/lib/pkgcache/PackageCacheTest.java
@@ -105,7 +105,6 @@
.setFileSystem(fileSystem)
.setDirectories(directories)
.setActionKeyContext(actionKeyContext)
- .setBuildInfoFactories(ruleClassProvider.getBuildInfoFactories())
.setDefaultBuildOptions(defaultBuildOptions)
.setExtraSkyFunctions(analysisMock.getSkyFunctions(directories))
.build();
diff --git a/src/test/java/com/google/devtools/build/lib/query2/engine/SkyframeQueryHelper.java b/src/test/java/com/google/devtools/build/lib/query2/engine/SkyframeQueryHelper.java
index ac97e80..9ff2e78 100644
--- a/src/test/java/com/google/devtools/build/lib/query2/engine/SkyframeQueryHelper.java
+++ b/src/test/java/com/google/devtools/build/lib/query2/engine/SkyframeQueryHelper.java
@@ -307,7 +307,6 @@
.setFileSystem(fileSystem)
.setDirectories(directories)
.setActionKeyContext(actionKeyContext)
- .setBuildInfoFactories(ruleClassProvider.getBuildInfoFactories())
.setDefaultBuildOptions(getDefaultBuildOptions(ruleClassProvider))
.setAdditionalBlacklistedPackagePrefixesFile(additionalBlacklistedPackagePrefixesFile)
.setExtraSkyFunctions(analysisMock.getSkyFunctions(directories))
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/AbstractCollectPackagesUnderDirectoryTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/AbstractCollectPackagesUnderDirectoryTest.java
index 7299e53..e57cb26 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/AbstractCollectPackagesUnderDirectoryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/AbstractCollectPackagesUnderDirectoryTest.java
@@ -288,7 +288,6 @@
directories,
new ActionKeyContext(),
/*workspaceStatusActionFactory=*/ null,
- ruleClassProvider.getBuildInfoFactories(),
/*diffAwarenessFactories=*/ ImmutableList.of(),
getExtraSkyFunctions(),
/*customDirtinessCheckers=*/ ImmutableList.of(),
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternsFunctionSmartNegationTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternsFunctionSmartNegationTest.java
index 0a6b74f..65a2306 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternsFunctionSmartNegationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternsFunctionSmartNegationTest.java
@@ -92,8 +92,6 @@
.setFileSystem(fileSystem)
.setDirectories(directories)
.setActionKeyContext(new ActionKeyContext())
- .setBuildInfoFactories(
- AnalysisMock.get().createRuleClassProvider().getBuildInfoFactories())
.setDefaultBuildOptions(
DefaultBuildOptionsForTesting.getDefaultBuildOptionsForTest(ruleClassProvider))
.setExtraSkyFunctions(AnalysisMock.get().getSkyFunctions(directories))
