Move ownership of SkyframeBuildView to SkyframeExecutor.
Also move ownership of ArtifactFactory to SkyframeBuildView; simplify the
code.
--
MOS_MIGRATED_REVID=103722228
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
index 92290ac..eb8ec0f 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
@@ -220,8 +220,6 @@
private final ConfiguredRuleClassProvider ruleClassProvider;
- private final ArtifactFactory artifactFactory;
-
/**
* A factory class to create the coverage report action. May be null.
*/
@@ -266,23 +264,9 @@
this.packageManager = skyframeExecutor.getPackageManager();
this.binTools = binTools;
this.coverageReportActionFactory = coverageReportActionFactory;
- this.artifactFactory = new ArtifactFactory(directories.getExecRoot());
this.ruleClassProvider = ruleClassProvider;
this.skyframeExecutor = Preconditions.checkNotNull(skyframeExecutor);
- this.skyframeBuildView =
- new SkyframeBuildView(
- new ConfiguredTargetFactory(ruleClassProvider),
- artifactFactory,
- skyframeExecutor,
- new Runnable() {
- @Override
- public void run() {
- clear();
- }
- },
- binTools,
- ruleClassProvider);
- skyframeExecutor.setSkyframeBuildView(skyframeBuildView);
+ this.skyframeBuildView = skyframeExecutor.getSkyframeBuildView();
}
/** Returns the action graph. */
@@ -312,16 +296,8 @@
skyframeBuildView.setTopLevelHostConfiguration(configurations.getHostConfiguration());
}
- /**
- * Clear the graphs of ConfiguredTargets and Artifacts.
- */
- @VisibleForTesting
- public void clear() {
- artifactFactory.clear();
- }
-
public ArtifactFactory getArtifactFactory() {
- return artifactFactory;
+ return skyframeBuildView.getArtifactFactory();
}
@VisibleForTesting
@@ -627,7 +603,6 @@
skyframeExecutor.dropConfiguredTargets();
skyframeCacheWasInvalidated = true;
- clear();
}
skyframeAnalysisWasDiscarded = false;
this.configurations = configurations;
@@ -741,7 +716,7 @@
actionsWrapper = coverageReportActionFactory.createCoverageReportActionsWrapper(
allTargetsToTest,
baselineCoverageArtifacts,
- artifactFactory,
+ getArtifactFactory(),
CoverageReportValue.ARTIFACT_OWNER);
if (actionsWrapper != null) {
ImmutableList <Action> actions = actionsWrapper.getActions();
@@ -952,7 +927,7 @@
realPackageRoots.put(entry.getKey(), root);
}
// Source Artifact roots:
- artifactFactory.setPackageRoots(realPackageRoots);
+ getArtifactFactory().setPackageRoots(realPackageRoots);
// Derived Artifact roots:
ImmutableList.Builder<Root> roots = ImmutableList.builder();
@@ -965,7 +940,7 @@
roots.addAll(cfg.getRoots());
}
- artifactFactory.setDerivedArtifactRoots(roots.build());
+ getArtifactFactory().setDerivedArtifactRoots(roots.build());
}
/**
@@ -995,7 +970,7 @@
BuildConfigurationCollection configurations) throws InterruptedException {
BuildConfiguration config = target.getConfiguration();
CachingAnalysisEnvironment analysisEnvironment =
- new CachingAnalysisEnvironment(artifactFactory,
+ new CachingAnalysisEnvironment(getArtifactFactory(),
new ConfiguredTargetKey(target.getLabel(), config),
/*isSystemEnv=*/false, config.extendedSanityChecks(), eventHandler,
/*skyframeEnv=*/null, config.isActionsEnabled(), binTools);
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 bef594b..4c885a3 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
@@ -103,7 +103,7 @@
this.errorEventListener = errorEventListener;
this.skyframeEnv = env;
this.allowRegisteringActions = allowRegisteringActions;
- this.binTools = binTools;
+ this.binTools = Preconditions.checkNotNull(binTools);
middlemanFactory = new MiddlemanFactory(artifactFactory, this);
artifacts = new HashMap<>();
}
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
index 368812f..6584af6 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
@@ -621,7 +621,6 @@
/** Removes skyframe cache and other caches that must be kept synchronized with skyframe. */
private void clearSkyframeRelevantCaches() {
skyframeExecutor.resetEvaluator();
- view.clear();
}
/**
@@ -1598,6 +1597,7 @@
pkgFactory,
timestampMonitor,
directories,
+ binTools,
workspaceStatusActionFactory,
ruleClassProvider.getBuildInfoFactories(),
immutableDirectories,
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 6461e3f..9f67714 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
@@ -31,6 +31,7 @@
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.config.BinTools;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.events.Reporter;
import com.google.devtools.build.lib.packages.Package;
@@ -100,6 +101,7 @@
PackageFactory pkgFactory,
TimestampGranularityMonitor tsgm,
BlazeDirectories directories,
+ BinTools binTools,
Factory workspaceStatusActionFactory,
ImmutableList<BuildInfoFactory> buildInfoFactories,
Set<Path> immutableDirectories,
@@ -115,6 +117,7 @@
pkgFactory,
tsgm,
directories,
+ binTools,
workspaceStatusActionFactory,
buildInfoFactories,
immutableDirectories,
@@ -132,6 +135,7 @@
PackageFactory pkgFactory,
TimestampGranularityMonitor tsgm,
BlazeDirectories directories,
+ BinTools binTools,
Factory workspaceStatusActionFactory,
ImmutableList<BuildInfoFactory> buildInfoFactories,
Set<Path> immutableDirectories,
@@ -148,6 +152,7 @@
pkgFactory,
tsgm,
directories,
+ binTools,
workspaceStatusActionFactory,
buildInfoFactories,
immutableDirectories,
@@ -163,7 +168,7 @@
@VisibleForTesting
public static SequencedSkyframeExecutor create(Reporter reporter, PackageFactory pkgFactory,
- TimestampGranularityMonitor tsgm, BlazeDirectories directories,
+ TimestampGranularityMonitor tsgm, BlazeDirectories directories, BinTools binTools,
WorkspaceStatusAction.Factory workspaceStatusActionFactory,
ImmutableList<BuildInfoFactory> buildInfoFactories,
Set<Path> immutableDirectories,
@@ -173,6 +178,7 @@
pkgFactory,
tsgm,
directories,
+ binTools,
workspaceStatusActionFactory,
buildInfoFactories,
immutableDirectories,
@@ -498,9 +504,8 @@
@Override
public void dropConfiguredTargets() {
- if (skyframeBuildView != null) {
- skyframeBuildView.clearInvalidatedConfiguredTargets();
- }
+ skyframeBuildView.clearInvalidatedConfiguredTargets();
+ skyframeBuildView.clearLegacyData();
memoizingEvaluator.delete(
// We delete any value that can hold an action -- all subclasses of ActionLookupValue -- as
// well as ActionExecutionValues, since they do not depend on ActionLookupValues.
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 b39707f..df68c74 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
@@ -19,6 +19,7 @@
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.BinTools;
import com.google.devtools.build.lib.events.Reporter;
import com.google.devtools.build.lib.packages.PackageFactory;
import com.google.devtools.build.lib.packages.Preprocessor;
@@ -41,6 +42,7 @@
PackageFactory pkgFactory,
TimestampGranularityMonitor tsgm,
BlazeDirectories directories,
+ BinTools binTools,
Factory workspaceStatusActionFactory,
ImmutableList<BuildInfoFactory> buildInfoFactories,
Set<Path> immutableDirectories,
@@ -55,6 +57,7 @@
pkgFactory,
tsgm,
directories,
+ binTools,
workspaceStatusActionFactory,
buildInfoFactories,
immutableDirectories,
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 718fd50..0c6d78e 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
@@ -34,8 +34,10 @@
import com.google.devtools.build.lib.analysis.AnalysisEnvironment;
import com.google.devtools.build.lib.analysis.AnalysisFailureEvent;
import com.google.devtools.build.lib.analysis.Aspect;
+import com.google.devtools.build.lib.analysis.BlazeDirectories;
import com.google.devtools.build.lib.analysis.CachingAnalysisEnvironment;
import com.google.devtools.build.lib.analysis.ConfiguredAspectFactory;
+import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.ConfiguredTargetFactory;
import com.google.devtools.build.lib.analysis.LabelAndConfiguration;
@@ -88,7 +90,6 @@
private final ConfiguredTargetFactory factory;
private final ArtifactFactory artifactFactory;
private final SkyframeExecutor skyframeExecutor;
- private final Runnable legacyDataCleaner;
private final BinTools binTools;
private boolean enableAnalysis = false;
@@ -115,16 +116,14 @@
private Map<Set<Class<? extends BuildConfiguration.Fragment>>, BuildConfiguration>
hostConfigurationCache = Maps.newConcurrentMap();
- public SkyframeBuildView(ConfiguredTargetFactory factory, ArtifactFactory artifactFactory,
- SkyframeExecutor skyframeExecutor, Runnable legacyDataCleaner, BinTools binTools,
- RuleClassProvider ruleClassProvider) {
- this.factory = factory;
- this.artifactFactory = artifactFactory;
+ public SkyframeBuildView(BlazeDirectories directories,
+ SkyframeExecutor skyframeExecutor, BinTools binTools,
+ ConfiguredRuleClassProvider ruleClassProvider) {
+ this.factory = new ConfiguredTargetFactory(ruleClassProvider);
+ this.artifactFactory = new ArtifactFactory(directories.getExecRoot());
this.skyframeExecutor = skyframeExecutor;
- this.legacyDataCleaner = legacyDataCleaner;
this.binTools = binTools;
this.ruleClassProvider = ruleClassProvider;
- skyframeExecutor.setArtifactFactoryAndBinTools(artifactFactory, binTools);
}
public void resetEvaluatedConfiguredTargetKeysSet() {
@@ -375,7 +374,7 @@
}
}
- ArtifactFactory getArtifactFactory() {
+ public ArtifactFactory getArtifactFactory() {
return artifactFactory;
}
@@ -480,12 +479,12 @@
}
/**
- * Workaround to clear all legacy data, like the action graph and the artifact factory. We need
+ * Workaround to clear all legacy data, like the artifact factory. We need
* to clear them to avoid conflicts.
* TODO(bazel-team): Remove this workaround. [skyframe-execution]
*/
void clearLegacyData() {
- legacyDataCleaner.run();
+ artifactFactory.clear();
}
/**
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 443a444..bc624d1 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
@@ -49,6 +49,7 @@
import com.google.devtools.build.lib.analysis.AspectWithParameters;
import com.google.devtools.build.lib.analysis.BlazeDirectories;
import com.google.devtools.build.lib.analysis.BuildView.Options;
+import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.DependencyResolver.Dependency;
import com.google.devtools.build.lib.analysis.RuleConfiguredTarget;
@@ -229,7 +230,7 @@
private final Set<Path> immutableDirectories;
- private BinTools binTools = null;
+ private final BinTools binTools;
private boolean needToInjectEmbeddedArtifacts = true;
private boolean needToInjectPrecomputedValuesForAnalysis = true;
protected int modifiedFiles;
@@ -256,6 +257,7 @@
PackageFactory pkgFactory,
TimestampGranularityMonitor tsgm,
BlazeDirectories directories,
+ BinTools binTools,
Factory workspaceStatusActionFactory,
ImmutableList<BuildInfoFactory> buildInfoFactories,
Set<Path> immutableDirectories,
@@ -288,6 +290,14 @@
this.extraSkyFunctions = extraSkyFunctions;
this.extraPrecomputedValues = extraPrecomputedValues;
this.errorOnExternalFiles = errorOnExternalFiles;
+ this.binTools = binTools;
+
+ this.skyframeBuildView = new SkyframeBuildView(
+ directories,
+ this,
+ binTools,
+ (ConfiguredRuleClassProvider) pkgFactory.getRuleClassProvider());
+ this.artifactFactory.set(skyframeBuildView.getArtifactFactory());
}
private ImmutableMap<SkyFunctionName, SkyFunction> skyFunctions(
@@ -479,9 +489,7 @@
public void resetEvaluator() {
init();
emittedEventState.clear();
- if (skyframeBuildView != null) {
- skyframeBuildView.clearLegacyData();
- }
+ skyframeBuildView.clearLegacyData();
reinjectConstantValuesLazily();
}
@@ -890,13 +898,8 @@
}
}
- /**
- * Specifies the current {@link SkyframeBuildView} instance. This should only be set once over the
- * lifetime of the Blaze server, except in tests.
- */
- public void setSkyframeBuildView(SkyframeBuildView skyframeBuildView) {
- this.skyframeBuildView = skyframeBuildView;
- this.artifactFactory.set(skyframeBuildView.getArtifactFactory());
+ public SkyframeBuildView getSkyframeBuildView() {
+ return skyframeBuildView;
}
/**
@@ -1062,11 +1065,6 @@
public ImmutableMap<Dependency, ConfiguredTarget> getConfiguredTargetMap(
BuildConfiguration originalConfig, Iterable<Dependency> keys, boolean useOriginalConfig) {
checkActive();
- if (skyframeBuildView == null) {
- // If build view has not yet been initialized, no configured targets can have been created.
- // This is most likely to happen after a failed loading phase.
- return ImmutableMap.of();
- }
Map<Dependency, BuildConfiguration> configs;
if (originalConfig != null) {
@@ -1620,15 +1618,6 @@
this.statusReporterRef.set(statusReporter);
}
- /**
- * This should be called at most once in the lifetime of the SkyframeExecutor (except for
- * tests), and it should be called before the execution phase.
- */
- void setArtifactFactoryAndBinTools(ArtifactFactory artifactFactory, BinTools binTools) {
- this.artifactFactory.set(artifactFactory);
- this.binTools = binTools;
- }
-
public void prepareExecution(boolean checkOutputFiles,
@Nullable Range<Long> lastExecutionTimeRange) throws AbruptExitException,
InterruptedException {
@@ -1704,9 +1693,7 @@
if (ignoreInvalidations) {
return;
}
- if (skyframeBuildView != null) {
- skyframeBuildView.getInvalidationReceiver().invalidated(skyKey, state);
- }
+ skyframeBuildView.getInvalidationReceiver().invalidated(skyKey, state);
}
@Override
@@ -1714,9 +1701,7 @@
if (ignoreInvalidations) {
return;
}
- if (skyframeBuildView != null) {
- skyframeBuildView.getInvalidationReceiver().enqueueing(skyKey);
- }
+ skyframeBuildView.getInvalidationReceiver().enqueueing(skyKey);
if (executionProgressReceiver != null) {
executionProgressReceiver.enqueueing(skyKey);
}
@@ -1730,9 +1715,7 @@
if (ignoreInvalidations) {
return;
}
- if (skyframeBuildView != null) {
- skyframeBuildView.getInvalidationReceiver().evaluated(skyKey, valueSupplier, state);
- }
+ skyframeBuildView.getInvalidationReceiver().evaluated(skyKey, valueSupplier, state);
if (executionProgressReceiver != null) {
executionProgressReceiver.evaluated(skyKey, valueSupplier, state);
}
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 7710fb0..f5e3ae5 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
@@ -19,6 +19,7 @@
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.BinTools;
import com.google.devtools.build.lib.events.Reporter;
import com.google.devtools.build.lib.packages.PackageFactory;
import com.google.devtools.build.lib.packages.Preprocessor;
@@ -43,6 +44,7 @@
* @param pkgFactory the package factory
* @param tsgm timestamp granularity monitor
* @param directories Blaze directories
+ * @param binTools the embedded tools
* @param workspaceStatusActionFactory a factory for creating WorkspaceStatusAction objects
* @param buildInfoFactories list of BuildInfoFactories
* @param diffAwarenessFactories
@@ -59,6 +61,7 @@
PackageFactory pkgFactory,
TimestampGranularityMonitor tsgm,
BlazeDirectories directories,
+ BinTools binTools,
Factory workspaceStatusActionFactory,
ImmutableList<BuildInfoFactory> buildInfoFactories,
Set<Path> immutableDirectories,
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 3d5cfb4..1ba2d1d 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
@@ -162,12 +162,14 @@
throws Exception {
this.ruleClassProvider = ruleClassProvider;
PackageFactory pkgFactory = new PackageFactory(ruleClassProvider);
+ BinTools binTools = BinTools.forUnitTesting(directories, TestConstants.EMBEDDED_TOOLS);
skyframeExecutor =
SequencedSkyframeExecutor.create(
reporter,
pkgFactory,
new TimestampGranularityMonitor(BlazeClock.instance()),
directories,
+ binTools,
workspaceStatusActionFactory,
ruleClassProvider.getBuildInfoFactories(),
ImmutableSet.<Path>of(),
@@ -183,7 +185,7 @@
packageManager = skyframeExecutor.getPackageManager();
loadingPhaseRunner = new LoadingPhaseRunner(packageManager, pkgFactory.getRuleClassNames());
buildView = new BuildView(directories, ruleClassProvider, skyframeExecutor,
- BinTools.forUnitTesting(directories, TestConstants.EMBEDDED_TOOLS), null);
+ binTools, null);
useConfiguration();
}
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 38b7355..f8718a4 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
@@ -197,6 +197,7 @@
new PackageFactory(ruleClassProvider, getEnvironmentExtensions()),
new TimestampGranularityMonitor(BlazeClock.instance()),
directories,
+ binTools,
workspaceStatusActionFactory,
ruleClassProvider.getBuildInfoFactories(),
ImmutableSet.<Path>of(),
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 fe946f3..9fd44f3 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
@@ -22,6 +22,7 @@
import com.google.devtools.build.lib.actions.Root;
import com.google.devtools.build.lib.analysis.BlazeDirectories;
import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
+import com.google.devtools.build.lib.analysis.config.BinTools;
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;
@@ -98,6 +99,7 @@
pkgFactory,
new TimestampGranularityMonitor(BlazeClock.instance()),
directories,
+ BinTools.forUnitTesting(directories, TestConstants.EMBEDDED_TOOLS),
workspaceStatusActionFactory,
ruleClassProvider.getBuildInfoFactories(),
ImmutableSet.<Path>of(),
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 4c45b97..c25aeea 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
@@ -76,6 +76,7 @@
new PackageFactory(ruleClassProvider, getEnvironmentExtensions()),
new TimestampGranularityMonitor(BlazeClock.instance()),
new BlazeDirectories(outputBase, outputBase, rootDirectory),
+ null, /* BinTools */
null, /* workspaceStatusActionFactory */
ruleClassProvider.getBuildInfoFactories(),
ImmutableSet.<Path>of(),
