Allow callers to preserve either loading or analysis objects when discardAnalysisCache runs.
PiperOrigin-RevId: 244070039
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 cd14cb5..8fb6870 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
@@ -575,8 +575,12 @@
}
@Override
- protected boolean discardPackagesWhenDiscardingAnalysisObjects() {
- return !trackIncrementalState;
+ public void clearAnalysisCache(
+ Collection<ConfiguredTarget> topLevelTargets, Collection<AspectValue> topLevelAspects) {
+ discardPreExecutionCache(
+ topLevelTargets,
+ topLevelAspects,
+ trackIncrementalState ? DiscardType.ANALYSIS_REFS_ONLY : DiscardType.ALL);
}
@Override
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 1d18244..0298f62 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
@@ -888,8 +888,6 @@
return true;
}
- protected abstract boolean discardPackagesWhenDiscardingAnalysisObjects();
-
/**
* If not null, this is the only source root in the build, corresponding to the single element in
* a single-element package path. Such a single-source-root build need not plant the execroot
@@ -909,6 +907,13 @@
@VisibleForTesting
protected abstract Injectable injectable();
+ /**
+ * Types that are created during loading, use significant space, and are definitely not needed
+ * during execution unless explicitly named.
+ *
+ * <p>Some keys, like globs, may be re-evaluated during execution, so these types should only be
+ * discarded if reverse deps are not being tracked!
+ */
private static final ImmutableSet<SkyFunctionName> LOADING_TYPES =
ImmutableSet.of(
SkyFunctions.PACKAGE,
@@ -916,6 +921,21 @@
SkyFunctions.AST_FILE_LOOKUP,
SkyFunctions.GLOB);
+ /** Data that should be discarded in {@link #discardPreExecutionCache}. */
+ protected enum DiscardType {
+ ALL,
+ ANALYSIS_REFS_ONLY,
+ LOADING_NODES_ONLY;
+
+ boolean discardsAnalysis() {
+ return this != LOADING_NODES_ONLY;
+ }
+
+ boolean discardsLoading() {
+ return this != ANALYSIS_REFS_ONLY;
+ }
+ }
+
/**
* Save memory by removing references to configured targets and aspects in Skyframe.
*
@@ -926,21 +946,26 @@
* phase. Instead, their analysis-time data is cleared while preserving the generating action info
* needed for execution. The next build will delete the nodes (and recreate them if necessary).
*
- * <p>If {@link #discardPackagesWhenDiscardingAnalysisObjects} is true, then also delete
- * loading-phase nodes (as determined by {@link #LOADING_TYPES}) from the graph.
+ * <p>{@code discardType} can be used to specify which data to discard.
*/
- private void discardAnalysisCache(
- Collection<ConfiguredTarget> topLevelTargets, Collection<AspectValue> topLevelAspects) {
- topLevelTargets = ImmutableSet.copyOf(topLevelTargets);
- topLevelAspects = ImmutableSet.copyOf(topLevelAspects);
+ protected void discardPreExecutionCache(
+ Collection<ConfiguredTarget> topLevelTargets,
+ Collection<AspectValue> topLevelAspects,
+ DiscardType discardType) {
+ if (discardType.discardsAnalysis()) {
+ topLevelTargets = ImmutableSet.copyOf(topLevelTargets);
+ topLevelAspects = ImmutableSet.copyOf(topLevelAspects);
+ }
// This is to prevent throwing away Packages we may need during execution.
ImmutableSet.Builder<PackageIdentifier> packageSetBuilder = ImmutableSet.builder();
- packageSetBuilder.addAll(
- Collections2.transform(
- topLevelTargets, target -> target.getLabel().getPackageIdentifier()));
- packageSetBuilder.addAll(
- Collections2.transform(
- topLevelAspects, aspect -> aspect.getLabel().getPackageIdentifier()));
+ if (discardType.discardsLoading()) {
+ packageSetBuilder.addAll(
+ Collections2.transform(
+ topLevelTargets, target -> target.getLabel().getPackageIdentifier()));
+ packageSetBuilder.addAll(
+ Collections2.transform(
+ topLevelAspects, aspect -> aspect.getLabel().getPackageIdentifier()));
+ }
ImmutableSet<PackageIdentifier> topLevelPackages = packageSetBuilder.build();
try (AutoProfiler p = AutoProfiler.logged("discarding analysis cache", logger)) {
lastAnalysisDiscarded = true;
@@ -954,37 +979,42 @@
}
SkyKey key = keyAndEntry.getKey();
SkyFunctionName functionName = key.functionName();
- // Keep packages for top-level targets and aspects in memory to get the target from later.
- if (functionName.equals(SkyFunctions.PACKAGE)
- && topLevelPackages.contains(key.argument())) {
- continue;
+ if (discardType.discardsLoading()) {
+ // Keep packages for top-level targets and aspects in memory to get the target from later.
+ if (functionName.equals(SkyFunctions.PACKAGE)
+ && topLevelPackages.contains(key.argument())) {
+ continue;
+ }
+ if (LOADING_TYPES.contains(functionName)) {
+ it.remove();
+ continue;
+ }
}
- if (discardPackagesWhenDiscardingAnalysisObjects()
- && LOADING_TYPES.contains(functionName)) {
- it.remove();
- continue;
- }
- if (functionName.equals(SkyFunctions.CONFIGURED_TARGET)) {
- ConfiguredTargetValue ctValue;
- try {
- ctValue = (ConfiguredTargetValue) entry.getValue();
- } catch (InterruptedException e) {
- throw new IllegalStateException("No interruption in in-memory retrieval: " + entry, e);
- }
- // 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;
- try {
- aspectValue = (AspectValue) entry.getValue();
- } catch (InterruptedException e) {
- throw new IllegalStateException("No interruption in in-memory retrieval: " + entry, e);
- }
- // value may be null if target was not successfully analyzed.
- if (aspectValue != null) {
- aspectValue.clear(!topLevelAspects.contains(aspectValue));
+ if (discardType.discardsAnalysis()) {
+ if (functionName.equals(SkyFunctions.CONFIGURED_TARGET)) {
+ ConfiguredTargetValue ctValue;
+ try {
+ ctValue = (ConfiguredTargetValue) entry.getValue();
+ } catch (InterruptedException e) {
+ throw new IllegalStateException(
+ "No interruption in in-memory retrieval: " + entry, e);
+ }
+ // 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;
+ try {
+ aspectValue = (AspectValue) entry.getValue();
+ } catch (InterruptedException e) {
+ throw new IllegalStateException(
+ "No interruption in in-memory retrieval: " + entry, e);
+ }
+ // value may be null if target was not successfully analyzed.
+ if (aspectValue != null) {
+ aspectValue.clear(!topLevelAspects.contains(aspectValue));
+ }
}
}
}
@@ -993,12 +1023,11 @@
/**
* Saves memory by clearing analysis objects from Skyframe. Clears their data without deleting
- * them (they will be deleted on the next build).
+ * them (they will be deleted on the next build). May also delete loading-phase objects from the
+ * graph.
*/
- public void clearAnalysisCache(
- Collection<ConfiguredTarget> topLevelTargets, Collection<AspectValue> topLevelAspects) {
- discardAnalysisCache(topLevelTargets, topLevelAspects);
- }
+ public abstract void clearAnalysisCache(
+ Collection<ConfiguredTarget> topLevelTargets, Collection<AspectValue> topLevelAspects);
protected abstract void dropConfiguredTargetsNow(final ExtendedEventHandler eventHandler);
diff --git a/src/test/shell/integration/discard_analysis_cache_test.sh b/src/test/shell/integration/discard_analysis_cache_test.sh
index f1b601c..4c3cb9f 100755
--- a/src/test/shell/integration/discard_analysis_cache_test.sh
+++ b/src/test/shell/integration/discard_analysis_cache_test.sh
@@ -80,17 +80,20 @@
function test_compile_helloworld() {
write_hello_world_files
- bazel run --discard_analysis_cache hello:hello >&$TEST_log \
+ bazel run --noexperimental_ui --discard_analysis_cache hello:hello >&$TEST_log \
|| fail "Build failed"
+ expect_log "Loading package: hello"
expect_log 'hello!'
- bazel run --discard_analysis_cache hello:hello >&$TEST_log \
+ bazel run --noexperimental_ui --discard_analysis_cache hello:hello >&$TEST_log \
|| fail "Build failed"
+ expect_not_log "Loading package: hello"
expect_log 'hello!'
# Check that further incremental builds work fine.
- bazel run hello:hello >&$TEST_log \
+ bazel run --noexperimental_ui hello:hello >&$TEST_log \
|| fail "Build failed"
+ expect_not_log "Loading package: hello"
expect_log 'hello!'
}
