Address mem leak in NSOS in the case of changing config & target.
When we run blaze with NSOS, and alternating 2 different targets with 2 different configs (which clears the analysis cache), there's observable memory leak:
- The content of ArtifactnestedSetFunction#artifactToValueOrException is still retained
- NodeEntries of ArtifactNestedSetValue is retained, and in turn retains all the transitive ArtifactNestedSetKeys.
Changes:
- Clear the map in ArtifactNestedSetFunction when analysis cache is discarded when there's a change in config & target.
- Clear the Nodes of ArtifactNestedSetValue in Skyframe when analysis cache is discarded.
RELNOTES: None
PiperOrigin-RevId: 299332103
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactNestedSetFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactNestedSetFunction.java
index b1605bc..a256569 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactNestedSetFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ArtifactNestedSetFunction.java
@@ -68,8 +68,15 @@
* therefore not populating artifactSkyKeyToValueOrException with X2's member Artifacts. Hence if
* we clear artifactSkyKeyToValueOrException between build 0 and 1, X2's member artifacts'
* SkyValues would not be available in the map.
+ *
+ * <p>We can't make this a:
+ *
+ * <p>- Weak-keyd map since ActionExecutionValue holds a reference to Artifact.
+ *
+ * <p>- Weak-valued map since there's nothing else holding on to ValueOrException and the entry
+ * will GCed immediately.
*/
- private final ConcurrentMap<SkyKey, ValueOrException2<IOException, ActionExecutionException>>
+ private ConcurrentMap<SkyKey, ValueOrException2<IOException, ActionExecutionException>>
artifactSkyKeyToValueOrException;
/**
@@ -134,6 +141,10 @@
return singleton;
}
+ public void resetArtifactSkyKeyToValueOrException() {
+ artifactSkyKeyToValueOrException = Maps.newConcurrentMap();
+ }
+
Map<SkyKey, ValueOrException2<IOException, ActionExecutionException>>
getArtifactSkyKeyToValueOrException() {
return artifactSkyKeyToValueOrException;
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 bf6873c..7ced0f9 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
@@ -816,7 +816,7 @@
@Override
public void handleAnalysisInvalidatingChange() {
super.handleAnalysisInvalidatingChange();
- memoizingEvaluator.delete(ANALYSIS_KEY_PREDICATE);
+ memoizingEvaluator.delete(ANALYSIS_INVALIDATING_PREDICATE);
}
/**
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 9c37cf0..234763b 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
@@ -222,8 +222,9 @@
private static final Logger logger = Logger.getLogger(SkyframeExecutor.class.getName());
// We delete any value that can hold an action -- all subclasses of ActionLookupKey.
- protected static final Predicate<SkyKey> ANALYSIS_KEY_PREDICATE =
- k -> k instanceof ActionLookupValue.ActionLookupKey;
+ // Also remove ArtifactNestedSetValues to prevent memory leak (b/143940221).
+ protected static final Predicate<SkyKey> ANALYSIS_INVALIDATING_PREDICATE =
+ k -> (k instanceof ArtifactNestedSetKey || k instanceof ActionLookupValue.ActionLookupKey);
private final EvaluatorSupplier evaluatorSupplier;
protected MemoizingEvaluator memoizingEvaluator;
@@ -840,6 +841,7 @@
clearTrimmingCache();
skyframeBuildView.clearInvalidatedConfiguredTargets();
skyframeBuildView.clearLegacyData();
+ ArtifactNestedSetFunction.getInstance().resetArtifactSkyKeyToValueOrException();
}
/** Activates retroactive trimming (idempotently, so has no effect if already active). */
