Add `--gc_churning_threshold_if_multiple_top_level_targets` option which overrides `--gc_churning_threshold` if there are multiple top level targets.
PiperOrigin-RevId: 781121537
Change-Id: Icbfce5c11b8a71f112040d620b86d0db892948ed
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/GcChurningDetector.java b/src/main/java/com/google/devtools/build/lib/runtime/GcChurningDetector.java
index 5912e80..b1c5336 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/GcChurningDetector.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/GcChurningDetector.java
@@ -45,7 +45,8 @@
private static final GoogleLogger logger = GoogleLogger.forEnclosingClass();
private static final Duration MIN_INVOCATION_WALL_TIME_DURATION = Duration.ofMinutes(1);
- private final int thresholdPercentage;
+ private volatile int thresholdPercentage;
+ private final int thresholdPercentageIfMultipleTopLevelTargets;
private Duration cumulativeFullGcDuration = Duration.ZERO;
private final Clock clock;
private final Instant start;
@@ -53,8 +54,14 @@
private final BugReporter bugReporter;
@VisibleForTesting
- GcChurningDetector(int thresholdPercentage, Clock clock, BugReporter bugReporter) {
+ GcChurningDetector(
+ int thresholdPercentage,
+ int thresholdPercentageIfMultipleTopLevelTargets,
+ Clock clock,
+ BugReporter bugReporter) {
this.thresholdPercentage = thresholdPercentage;
+ this.thresholdPercentageIfMultipleTopLevelTargets =
+ thresholdPercentageIfMultipleTopLevelTargets;
this.clock = clock;
this.start = clock.now();
this.bugReporter = bugReporter;
@@ -62,7 +69,19 @@
static GcChurningDetector createForCommand(MemoryPressureOptions options) {
return new GcChurningDetector(
- options.gcChurningThreshold, BlazeClock.instance(), BugReporter.defaultInstance());
+ options.gcChurningThreshold,
+ options.gcChurningThresholdIfMultipleTopLevelTargets.orElse(options.gcChurningThreshold),
+ BlazeClock.instance(),
+ BugReporter.defaultInstance());
+ }
+
+ void targetParsingComplete(int numTopLevelTargets) {
+ if (numTopLevelTargets > 1) {
+ thresholdPercentage = thresholdPercentageIfMultipleTopLevelTargets;
+ logger.atInfo().log(
+ "Switched to thresholdPercentage of %s because there were %s top-level targets",
+ thresholdPercentage, numTopLevelTargets);
+ }
}
// This is called from MemoryPressureListener on a single memory-pressure-listener-0 thread, so it
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/MemoryPressureListener.java b/src/main/java/com/google/devtools/build/lib/runtime/MemoryPressureListener.java
index a08e598..9ae7be9 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/MemoryPressureListener.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/MemoryPressureListener.java
@@ -173,6 +173,13 @@
this.gcChurnDetector.set(gcChurningDetector);
}
+ void targetParsingComplete(int numTopLevelTargets) {
+ GcChurningDetector gcChurningDetector = gcChurnDetector.get();
+ if (gcChurningDetector != null) {
+ gcChurningDetector.targetParsingComplete(numTopLevelTargets);
+ }
+ }
+
void populateStats(MemoryPressureStats.Builder memoryPressureStatsBuilder) {
GcChurningDetector gcChurningDetector = gcChurnDetector.get();
if (gcChurningDetector != null) {
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/MemoryPressureModule.java b/src/main/java/com/google/devtools/build/lib/runtime/MemoryPressureModule.java
index 69d32b6..7fbdfe7 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/MemoryPressureModule.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/MemoryPressureModule.java
@@ -17,6 +17,7 @@
import com.google.common.collect.ImmutableList;
import com.google.common.eventbus.EventBus;
import com.google.common.eventbus.Subscribe;
+import com.google.devtools.build.lib.pkgcache.TargetParsingCompleteEvent;
import com.google.devtools.build.lib.runtime.MemoryPressure.MemoryPressureStats;
import com.google.devtools.build.lib.skyframe.HighWaterMarkLimiter;
import com.google.devtools.common.options.OptionsBase;
@@ -53,6 +54,11 @@
eventBus.register(highWaterMarkLimiter);
}
+ @Subscribe
+ public void targetParsingComplete(TargetParsingCompleteEvent event) {
+ memoryPressureListener.targetParsingComplete(event.getTargets().size());
+ }
+
@Override
public void afterCommand() {
postStats();
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/MemoryPressureOptions.java b/src/main/java/com/google/devtools/build/lib/runtime/MemoryPressureOptions.java
index c257851..3964ce0 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/MemoryPressureOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/MemoryPressureOptions.java
@@ -17,6 +17,7 @@
import com.google.devtools.common.options.Converter;
import com.google.devtools.common.options.Converters.CommaSeparatedOptionListConverter;
import com.google.devtools.common.options.Converters.DurationConverter;
+import com.google.devtools.common.options.Converters.OptionalPercentageConverter;
import com.google.devtools.common.options.Converters.PercentageConverter;
import com.google.devtools.common.options.Converters.RangeConverter;
import com.google.devtools.common.options.Converters.RegexPatternConverter;
@@ -27,6 +28,7 @@
import com.google.devtools.common.options.OptionsParsingException;
import com.google.devtools.common.options.RegexPatternOption;
import java.time.Duration;
+import java.util.OptionalInt;
/** Options for responding to memory pressure. */
public final class MemoryPressureOptions extends OptionsBase {
@@ -119,6 +121,21 @@
+ " never give up for this reason.")
public int gcChurningThreshold;
+ @Option(
+ name = "gc_churning_threshold_if_multiple_top_level_targets",
+ defaultValue = OptionalPercentageConverter.UNSET,
+ documentationCategory = OptionDocumentationCategory.EXECUTION_STRATEGY,
+ effectTags = {OptionEffectTag.HOST_MACHINE_RESOURCE_OPTIMIZATIONS},
+ converter = OptionalPercentageConverter.class,
+ help =
+ "If set to a value in [0, 100] and this is a command that takes top-level targets (e.g."
+ + " build but not query) and there are multiple such top-level targets, overrides"
+ + " --gc_churning_threshold. Useful to configure more aggressive OOMing behavior"
+ + " (i.e. a lower value than --gc_churning_threshold) when they are multiple"
+ + " top-level targets so that the invoker of Bazel can split and retry while still"
+ + " having less aggressive behavior when there is a single top-level target.")
+ public OptionalInt gcChurningThresholdIfMultipleTopLevelTargets;
+
// NOTE: 2024-06-11, this matches both known patterns of:
// jdk.internal.vm.Filler[Array|Element[]] but does not match the thread
// related jdk.internal.vm entries which we do not currently want to count.
diff --git a/src/main/java/com/google/devtools/common/options/Converters.java b/src/main/java/com/google/devtools/common/options/Converters.java
index 4bfc8c5..f20b1fb 100644
--- a/src/main/java/com/google/devtools/common/options/Converters.java
+++ b/src/main/java/com/google/devtools/common/options/Converters.java
@@ -27,6 +27,7 @@
import java.util.Iterator;
import java.util.List;
import java.util.Map;
+import java.util.OptionalInt;
import java.util.logging.Level;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
@@ -700,6 +701,24 @@
}
}
+ /** Same as {@link PercentageConverter} but also supports being unset. */
+ public static class OptionalPercentageConverter extends Converter.Contextless<OptionalInt> {
+ public static final String UNSET = "-1";
+ private static final PercentageConverter PERCENTAGE_CONVERTER = new PercentageConverter();
+
+ @Override
+ public String getTypeDescription() {
+ return "an integer";
+ }
+
+ @Override
+ public OptionalInt convert(String input) throws OptionsParsingException {
+ return input.equals(UNSET)
+ ? OptionalInt.empty()
+ : OptionalInt.of(PERCENTAGE_CONVERTER.convert(input));
+ }
+ }
+
/**
* A {@link Converter} for {@link com.github.benmanes.caffeine.cache.CaffeineSpec}. The spec may
* be empty, in which case this converter returns null.
diff --git a/src/test/java/com/google/devtools/build/lib/runtime/GcChurningDetectorTest.java b/src/test/java/com/google/devtools/build/lib/runtime/GcChurningDetectorTest.java
index 534239b..d6e1fd6 100644
--- a/src/test/java/com/google/devtools/build/lib/runtime/GcChurningDetectorTest.java
+++ b/src/test/java/com/google/devtools/build/lib/runtime/GcChurningDetectorTest.java
@@ -44,7 +44,11 @@
ManualClock fakeClock = new ManualClock();
GcChurningDetector underTest =
- new GcChurningDetector(/* thresholdPercentage= */ 100, fakeClock, mockBugReporter);
+ new GcChurningDetector(
+ /* thresholdPercentage= */ 100,
+ /* thresholdPercentageIfMultipleTopLevelTargets= */ 100,
+ fakeClock,
+ mockBugReporter);
fakeClock.advance(Duration.ofMillis(50L));
underTest.handle(fullGcEvent(Duration.ofMillis(10L)));
@@ -78,7 +82,11 @@
ManualClock fakeClock = new ManualClock();
GcChurningDetector underTest =
- new GcChurningDetector(/* thresholdPercentage= */ 100, fakeClock, mockBugReporter);
+ new GcChurningDetector(
+ /* thresholdPercentage= */ 100,
+ /* thresholdPercentageIfMultipleTopLevelTargets= */ 100,
+ fakeClock,
+ mockBugReporter);
fakeClock.advance(Duration.ofNanos(456L));
underTest.handle(fullGcEvent(Duration.ofNanos(123L)));
@@ -105,7 +113,11 @@
ManualClock fakeClock = new ManualClock();
GcChurningDetector underTest =
- new GcChurningDetector(/* thresholdPercentage= */ 50, fakeClock, mockBugReporter);
+ new GcChurningDetector(
+ /* thresholdPercentage= */ 50,
+ /* thresholdPercentageIfMultipleTopLevelTargets= */ 50,
+ fakeClock,
+ mockBugReporter);
fakeClock.advance(Duration.ofMinutes(3L));
underTest.handle(fullGcEvent(Duration.ofMinutes(1L)));
@@ -121,7 +133,11 @@
ManualClock fakeClock = new ManualClock();
GcChurningDetector underTest =
- new GcChurningDetector(/* thresholdPercentage= */ 50, fakeClock, mockBugReporter);
+ new GcChurningDetector(
+ /* thresholdPercentage= */ 50,
+ /* thresholdPercentageIfMultipleTopLevelTargets= */ 50,
+ fakeClock,
+ mockBugReporter);
fakeClock.advance(Duration.ofSeconds(30L));
underTest.handle(fullGcEvent(Duration.ofSeconds(15L)));
@@ -136,6 +152,48 @@
verifyOom();
}
+ @Test
+ public void thresholdPercentageIfMultipleTopLevelTargets_onlySingleTarget() {
+ ManualClock fakeClock = new ManualClock();
+
+ GcChurningDetector underTest =
+ new GcChurningDetector(
+ /* thresholdPercentage= */ 100,
+ /* thresholdPercentageIfMultipleTopLevelTargets= */ 50,
+ fakeClock,
+ mockBugReporter);
+
+ fakeClock.advance(Duration.ofSeconds(60L));
+ underTest.handle(fullGcEvent(Duration.ofSeconds(30L)));
+ verifyNoOom();
+
+ underTest.targetParsingComplete(1);
+ fakeClock.advance(Duration.ofSeconds(30L));
+ underTest.handle(fullGcEvent(Duration.ofSeconds(20L)));
+ verifyNoOom();
+ }
+
+ @Test
+ public void thresholdPercentageIfMultipleTopLevelTargets() {
+ ManualClock fakeClock = new ManualClock();
+
+ GcChurningDetector underTest =
+ new GcChurningDetector(
+ /* thresholdPercentage= */ 100,
+ /* thresholdPercentageIfMultipleTopLevelTargets= */ 50,
+ fakeClock,
+ mockBugReporter);
+
+ fakeClock.advance(Duration.ofSeconds(60L));
+ underTest.handle(fullGcEvent(Duration.ofSeconds(40L)));
+ verifyNoOom();
+
+ underTest.targetParsingComplete(2);
+ fakeClock.advance(Duration.ofSeconds(30L));
+ underTest.handle(fullGcEvent(Duration.ofSeconds(20L)));
+ verifyOom();
+ }
+
private void verifyNoOom() {
verifyNoInteractions(mockBugReporter);
}
diff --git a/src/test/java/com/google/devtools/build/lib/runtime/MemoryPressureListenerTest.java b/src/test/java/com/google/devtools/build/lib/runtime/MemoryPressureListenerTest.java
index 4e59593..4bd422d 100644
--- a/src/test/java/com/google/devtools/build/lib/runtime/MemoryPressureListenerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/runtime/MemoryPressureListenerTest.java
@@ -319,4 +319,17 @@
verify(mockGcThrashingDetector).handle(eq(event));
verify(mockGcChurningDetector).handle(eq(event));
}
+
+ @Test
+ public void forwardsTargetParsingComplete() {
+ MemoryPressureListener underTest =
+ MemoryPressureListener.createFromBeans(
+ ImmutableList.of(mockUselessBean, mockBean), directExecutor());
+
+ GcChurningDetector mockGcChurningDetector = mock(GcChurningDetector.class);
+ underTest.initForInvocation(eventBus, mock(GcThrashingDetector.class), mockGcChurningDetector);
+ underTest.targetParsingComplete(42);
+
+ verify(mockGcChurningDetector).targetParsingComplete(eq(42));
+ }
}
