Clear AspectValues when discarding analysis cache, along with ConfiguredTargetValues. Also clear transitive packages for both, even for top-level targets.
This is not expected to save significant memory, but is expected to reduce the number of references to Packages, allowing them to be dropped more easily when discarding analysis cache and running in batch mode.
PiperOrigin-RevId: 151508877
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java
index df6048d..25d3873 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectValue.java
@@ -15,6 +15,7 @@
package com.google.devtools.build.lib.skyframe;
import com.google.common.base.Objects;
+import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
import com.google.devtools.build.lib.analysis.ConfiguredAspect;
@@ -305,12 +306,14 @@
}
- private final Label label;
- private final Aspect aspect;
- private final Location location;
- private final AspectKey key;
- private final ConfiguredAspect configuredAspect;
- private final NestedSet<Package> transitivePackages;
+ // These variables are only non-final because they may be clear()ed to save memory. They are null
+ // only after they are cleared.
+ @Nullable private Label label;
+ @Nullable private Aspect aspect;
+ @Nullable private Location location;
+ @Nullable private AspectKey key;
+ @Nullable private ConfiguredAspect configuredAspect;
+ @Nullable private NestedSet<Package> transitivePackages;
public AspectValue(
AspectKey key,
@@ -321,41 +324,62 @@
Iterable<ActionAnalysisMetadata> actions,
NestedSet<Package> transitivePackages) {
super(actions);
- this.aspect = aspect;
- this.location = location;
- this.label = label;
- this.key = key;
- this.configuredAspect = configuredAspect;
- this.transitivePackages = transitivePackages;
+ this.label = Preconditions.checkNotNull(label, actions);
+ this.aspect = Preconditions.checkNotNull(aspect, label);
+ this.location = Preconditions.checkNotNull(location, label);
+ this.key = Preconditions.checkNotNull(key, label);
+ this.configuredAspect = Preconditions.checkNotNull(configuredAspect, label);
+ this.transitivePackages = Preconditions.checkNotNull(transitivePackages, label);
}
public ConfiguredAspect getConfiguredAspect() {
- return configuredAspect;
+ return Preconditions.checkNotNull(configuredAspect);
}
public Label getLabel() {
- return label;
+ return Preconditions.checkNotNull(label);
}
public Location getLocation() {
- return location;
+ return Preconditions.checkNotNull(location);
}
public AspectKey getKey() {
- return key;
+ return Preconditions.checkNotNull(key);
}
public Aspect getAspect() {
- return aspect;
+ return Preconditions.checkNotNull(aspect);
}
- public NestedSet<Package> getTransitivePackages() {
- return transitivePackages;
+ void clear(boolean clearEverything) {
+ Preconditions.checkNotNull(label, this);
+ Preconditions.checkNotNull(aspect, this);
+ Preconditions.checkNotNull(location, this);
+ Preconditions.checkNotNull(key, this);
+ Preconditions.checkNotNull(configuredAspect, this);
+ Preconditions.checkNotNull(transitivePackages, this);
+ if (clearEverything) {
+ label = null;
+ aspect = null;
+ location = null;
+ key = null;
+ configuredAspect = null;
+ }
+ transitivePackages = null;
}
+ NestedSet<Package> getTransitivePackages() {
+ return Preconditions.checkNotNull(transitivePackages);
+ }
+
+ // TODO(janakr): Add a nice toString after cl/150542180 is submitted.
+
public static AspectKey createAspectKey(
- Label label, BuildConfiguration baseConfiguration,
- ImmutableList<AspectKey> baseKeys, AspectDescriptor aspectDescriptor,
+ Label label,
+ BuildConfiguration baseConfiguration,
+ ImmutableList<AspectKey> baseKeys,
+ AspectDescriptor aspectDescriptor,
BuildConfiguration aspectConfiguration) {
return new AspectKey(
label, baseConfiguration,
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetValue.java
index 09cc4ed..1109570 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetValue.java
@@ -27,9 +27,7 @@
import com.google.devtools.build.lib.packages.Package;
import com.google.devtools.build.lib.util.Preconditions;
import com.google.devtools.build.skyframe.SkyKey;
-
import java.util.Map;
-
import javax.annotation.Nullable;
/**
@@ -44,14 +42,14 @@
// only after they are cleared.
@Nullable private ConfiguredTarget configuredTarget;
- private final NestedSet<Package> transitivePackages;
+ @Nullable private NestedSet<Package> transitivePackages;
ConfiguredTargetValue(ConfiguredTarget configuredTarget,
Map<Artifact, ActionAnalysisMetadata> generatingActionMap,
NestedSet<Package> transitivePackages) {
super(generatingActionMap);
- this.configuredTarget = configuredTarget;
- this.transitivePackages = transitivePackages;
+ this.configuredTarget = Preconditions.checkNotNull(configuredTarget, generatingActionMap);
+ this.transitivePackages = Preconditions.checkNotNull(transitivePackages, generatingActionMap);
}
@VisibleForTesting
@@ -67,8 +65,9 @@
}
public NestedSet<Package> getTransitivePackages() {
- return transitivePackages;
+ return Preconditions.checkNotNull(transitivePackages);
}
+
/**
* Clears configured target data from this value, leaving only the artifact->generating action
* map.
@@ -76,10 +75,18 @@
* <p>Should only be used when user specifies --discard_analysis_cache. Must be called at most
* once per value, after which {@link #getConfiguredTarget} and {@link #getActions} cannot be
* called.
+ *
+ * @param clearEverything if true, clear the {@link #configuredTarget}. If not, only the {@link
+ * #transitivePackages} field is cleared. Top-level targets need their {@link
+ * #configuredTarget} preserved, so should pass false here.
*/
- public void clear() {
+ public void clear(boolean clearEverything) {
Preconditions.checkNotNull(configuredTarget);
- configuredTarget = null;
+ Preconditions.checkNotNull(transitivePackages);
+ if (clearEverything) {
+ configuredTarget = null;
+ }
+ transitivePackages = null;
}
@VisibleForTesting
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 ed0f862..61defac 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
@@ -596,7 +596,7 @@
}
/**
- * Save memory by removing references to configured targets and actions in Skyframe.
+ * Save memory by removing references to configured targets and aspects in Skyframe.
*
* <p>These values must be recreated on subsequent builds. We do not clear the top-level target
* values, since their configured targets are needed for the target completion middleman values.
@@ -605,25 +605,35 @@
* execution phase. Instead, their data is cleared. The next build will delete the values (and
* recreate them if necessary).
*/
- private void discardAnalysisCache(Collection<ConfiguredTarget> topLevelTargets) {
+ private void discardAnalysisCache(
+ Collection<ConfiguredTarget> topLevelTargets, Collection<AspectValue> topLevelAspects) {
+ topLevelTargets = ImmutableSet.copyOf(topLevelTargets);
+ topLevelAspects = ImmutableSet.copyOf(topLevelAspects);
try (AutoProfiler p = AutoProfiler.logged("discarding analysis cache", LOG)) {
lastAnalysisDiscarded = true;
for (Map.Entry<SkyKey, SkyValue> entry : memoizingEvaluator.getValues().entrySet()) {
- if (!entry.getKey().functionName().equals(SkyFunctions.CONFIGURED_TARGET)) {
- continue;
- }
- ConfiguredTargetValue ctValue = (ConfiguredTargetValue) entry.getValue();
- // ctValue may be null if target was not successfully analyzed.
- if (ctValue != null && !topLevelTargets.contains(ctValue.getConfiguredTarget())) {
- ctValue.clear();
+ SkyFunctionName functionName = entry.getKey().functionName();
+ if (functionName.equals(SkyFunctions.CONFIGURED_TARGET)) {
+ ConfiguredTargetValue ctValue = (ConfiguredTargetValue) entry.getValue();
+ // ctValue may be null if target was not successfully analyzed.
+ if (ctValue != null) {
+ ctValue.clear(!topLevelTargets.contains(ctValue.getConfiguredTarget()));
+ }
+ } else if (functionName.equals(SkyFunctions.ASPECT)) {
+ AspectValue aspectValue = (AspectValue) entry.getValue();
+ // value may be null if target was not successfully analyzed.
+ if (aspectValue != null) {
+ aspectValue.clear(!topLevelAspects.contains(aspectValue));
+ }
}
}
}
}
@Override
- public void clearAnalysisCache(Collection<ConfiguredTarget> topLevelTargets) {
- discardAnalysisCache(topLevelTargets);
+ public void clearAnalysisCache(
+ Collection<ConfiguredTarget> topLevelTargets, Collection<AspectValue> topLevelAspects) {
+ discardAnalysisCache(topLevelTargets, topLevelAspects);
}
@Override
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 abcf596..0c41884 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
@@ -183,10 +183,11 @@
*
* @see com.google.devtools.build.lib.analysis.BuildView.Options#discardAnalysisCache
*/
- public void clearAnalysisCache(Collection<ConfiguredTarget> topLevelTargets) {
+ public void clearAnalysisCache(
+ Collection<ConfiguredTarget> topLevelTargets, Collection<AspectValue> topLevelAspects) {
// TODO(bazel-team): Consider clearing packages too to save more memory.
skyframeAnalysisWasDiscarded = true;
- skyframeExecutor.clearAnalysisCache(topLevelTargets);
+ skyframeExecutor.clearAnalysisCache(topLevelTargets, topLevelAspects);
}
/**
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 c6d088d..4c97085 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
@@ -658,7 +658,8 @@
* deletes the relevant values. If using Skyframe execution, clears their data without deleting
* them (they will be deleted on the next build).
*/
- public abstract void clearAnalysisCache(Collection<ConfiguredTarget> topLevelTargets);
+ public abstract void clearAnalysisCache(
+ Collection<ConfiguredTarget> topLevelTargets, Collection<AspectValue> topLevelAspects);
/**
* Injects the contents of the computed tools/defaults package.
