Create an option to enable GcThrashingDetector.

Also create another option so that we can experiment with and without `RetainedHeapLimiter`.

PiperOrigin-RevId: 520726326
Change-Id: I4ddfd91089132b96a9339ac60e03d634fb56c451

Author: justinhorvitz

src/main/java/com/google/devtools/build/lib/BUILD

@@ -224,14 +224,18 @@ java_library(
 java_library(
     name = "runtime/memory_pressure",
     srcs = [
+        "runtime/GcThrashingDetector.java",
         "runtime/MemoryPressureEvent.java",
         "runtime/MemoryPressureOptions.java",
         "runtime/MemoryPressureStatCollector.java",
     ],
     deps = [
+        "//src/main/java/com/google/devtools/build/lib/bugreport",
+        "//src/main/java/com/google/devtools/build/lib/clock",
         "//src/main/java/com/google/devtools/common/options",
         "//src/main/protobuf:memory_pressure_java_proto",
         "//third_party:auto_value",
+        "//third_party:flogger",
         "//third_party:guava",
     ],
 )
@@ -263,6 +267,7 @@ java_library(
             "runtime/BlazeCommandResult.java",
             "runtime/CommandDispatcher.java",
             "runtime/CommandLinePathFactory.java",
+            "runtime/GcThrashingDetector.java",
             "runtime/KeepGoingOption.java",
             "runtime/LoadingPhaseThreadsOption.java",
             "runtime/MemoryPressureEvent.java",

src/main/java/com/google/devtools/build/lib/runtime/GcThrashingDetector.java

@@ -18,11 +18,15 @@
 import static com.google.common.collect.ImmutableList.toImmutableList;
 
 import com.google.auto.value.AutoValue;
+import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.ImmutableList;
+import com.google.common.eventbus.EventBus;
+import com.google.common.eventbus.Subscribe;
 import com.google.common.flogger.GoogleLogger;
 import com.google.devtools.build.lib.bugreport.BugReporter;
 import com.google.devtools.build.lib.bugreport.Crash;
 import com.google.devtools.build.lib.bugreport.CrashContext;
+import com.google.devtools.build.lib.clock.BlazeClock;
 import com.google.devtools.build.lib.clock.Clock;
 import java.time.Duration;
 import java.time.Instant;
@@ -60,18 +64,37 @@ static Limit of(Duration period, int count) {
     }
   }
 
+  /**
+   * If enabled in {@link MemoryPressureOptions}, creates a {@link GcThrashingDetector} and
+   * registers it with the {@link EventBus}.
+   */
+  public static void configureForCommand(MemoryPressureOptions options, EventBus eventBus) {
+    if (options.gcThrashingLimits.isEmpty() || options.oomMoreEagerlyThreshold == 100) {
+      return;
+    }
+
+    eventBus.register(
+        new GcThrashingDetector(
+            options.oomMoreEagerlyThreshold,
+            options.gcThrashingLimits,
+            BlazeClock.instance(),
+            BugReporter.defaultInstance()));
+  }
+
   private final int threshold;
   private final ImmutableList<SingleLimitTracker> trackers;
   private final Clock clock;
   private final BugReporter bugReporter;
 
+  @VisibleForTesting
   GcThrashingDetector(int threshold, List<Limit> limits, Clock clock, BugReporter bugReporter) {
     this.threshold = threshold;
     this.trackers = limits.stream().map(SingleLimitTracker::new).collect(toImmutableList());
     this.clock = clock;
     this.bugReporter = bugReporter;
   }
 
+  @Subscribe // EventBus synchronizes calls by default, making thread safety unnecessary.
   void handle(MemoryPressureEvent event) {
     if (event.percentTenuredSpaceUsed() < threshold) {
       for (var tracker : trackers) {

src/main/java/com/google/devtools/build/lib/runtime/MemoryPressureModule.java

@@ -53,7 +53,7 @@ public void beforeCommand(CommandEnvironment env) {
     MemoryPressureOptions options = env.getOptions().getOptions(MemoryPressureOptions.class);
     highWaterMarkLimiter =
         new HighWaterMarkLimiter(env.getSkyframeExecutor(), env.getSyscallCache(), options);
-
+    GcThrashingDetector.configureForCommand(options, eventBus);
     retainedHeapLimiter.setOptions(options);
 
     eventBus.register(this);

src/main/java/com/google/devtools/build/lib/runtime/MemoryPressureOptions.java

@@ -13,12 +13,17 @@
 // limitations under the License.
 package com.google.devtools.build.lib.runtime;
 
+import com.google.common.collect.ImmutableList;
+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.PercentageConverter;
 import com.google.devtools.common.options.Converters.RangeConverter;
 import com.google.devtools.common.options.Option;
 import com.google.devtools.common.options.OptionDocumentationCategory;
 import com.google.devtools.common.options.OptionEffectTag;
 import com.google.devtools.common.options.OptionsBase;
+import com.google.devtools.common.options.OptionsParsingException;
 import java.time.Duration;
 
 /** Options for responding to memory pressure. */
@@ -91,9 +96,64 @@ public final class MemoryPressureOptions extends OptionsBase {
               + " threshold is exceeded.")
   public int skyframeHighWaterMarkFullGcDropsPerInvocation;
 
+  @Option(
+      name = "experimental_gc_thrashing_limits",
+      defaultValue = "",
+      documentationCategory = OptionDocumentationCategory.BUILD_TIME_OPTIMIZATION,
+      effectTags = {OptionEffectTag.HOST_MACHINE_RESOURCE_OPTIMIZATIONS},
+      converter = GcThrashingLimitsConverter.class,
+      help =
+          "Limits which, if reached, cause GcThrashingDetector to crash Bazel with an OOM. Each"
+              + " limit is specified as <period>:<count> where period is a duration and count is a"
+              + " positive integer. If more than --experimental_oom_more_eagerly_threshold percent"
+              + " of tenured space (old gen heap) remains occupied after <count> consecutive full"
+              + " GCs within <period>, an OOM is triggered. Multiple limits can be specified"
+              + " separated by commas.")
+  public ImmutableList<GcThrashingDetector.Limit> gcThrashingLimits;
+
+  @Option(
+      name = "gc_thrashing_limits_retained_heap_limiter_mutually_exclusive",
+      defaultValue = "true",
+      documentationCategory = OptionDocumentationCategory.BUILD_TIME_OPTIMIZATION,
+      effectTags = {OptionEffectTag.HOST_MACHINE_RESOURCE_OPTIMIZATIONS},
+      help =
+          "If true, specifying non-empty --experimental_gc_thrashing_limits deactivates"
+              + " RetainedHeapLimiter to make it mutually exclusive with GcThrashingDetector."
+              + " Setting to false permits both to be active for the same command.")
+  public boolean gcThrashingLimitsRetainedHeapLimiterMutuallyExclusive;
+
   static final class NonNegativeIntegerConverter extends RangeConverter {
     NonNegativeIntegerConverter() {
       super(0, Integer.MAX_VALUE);
     }
   }
+
+  static final class GcThrashingLimitsConverter
+      extends Converter.Contextless<ImmutableList<GcThrashingDetector.Limit>> {
+    private final CommaSeparatedOptionListConverter commaListConverter =
+        new CommaSeparatedOptionListConverter();
+    private final DurationConverter durationConverter = new DurationConverter();
+    private final RangeConverter positiveIntConverter = new RangeConverter(1, Integer.MAX_VALUE);
+
+    @Override
+    public ImmutableList<GcThrashingDetector.Limit> convert(String input)
+        throws OptionsParsingException {
+      ImmutableList.Builder<GcThrashingDetector.Limit> result = ImmutableList.builder();
+      for (String part : commaListConverter.convert(input)) {
+        int colonIndex = part.indexOf(':');
+        if (colonIndex == -1) {
+          throw new OptionsParsingException("Expected <period>:<count>, got " + part);
+        }
+        Duration period = durationConverter.convert(part.substring(0, colonIndex));
+        int count = positiveIntConverter.convert(part.substring(colonIndex + 1));
+        result.add(GcThrashingDetector.Limit.of(period, count));
+      }
+      return result.build();
+    }
+
+    @Override
+    public String getTypeDescription() {
+      return "comma separated pairs of <period>:<count>";
+    }
+  }
 }

src/main/java/com/google/devtools/build/lib/runtime/RetainedHeapLimiter.java

@@ -45,7 +45,7 @@ final class RetainedHeapLimiter implements MemoryPressureStatCollector {
   private final BugReporter bugReporter;
   private final Clock clock;
 
-  private volatile MemoryPressureOptions options = Options.getDefaults(MemoryPressureOptions.class);
+  private volatile MemoryPressureOptions options = inactiveOptions();
 
   private final AtomicBoolean throwingOom = new AtomicBoolean(false);
   private final AtomicBoolean heapLimiterTriggeredGc = new AtomicBoolean(false);
@@ -71,7 +71,12 @@ private RetainedHeapLimiter(BugReporter bugReporter, Clock clock) {
 
   @ThreadSafety.ThreadCompatible // Can only be called on the logical main Bazel thread.
   void setOptions(MemoryPressureOptions options) {
-    this.options = options;
+    if (options.gcThrashingLimitsRetainedHeapLimiterMutuallyExclusive
+        && !options.gcThrashingLimits.isEmpty()) {
+      this.options = inactiveOptions();
+    } else {
+      this.options = options;
+    }
   }
 
   // Can be called concurrently, handles concurrent calls with #setThreshold gracefully.
@@ -169,4 +174,10 @@ public void addStatsAndReset(MemoryPressureStats.Builder stats) {
             maxConsecutiveIgnoredFullGcsOverThreshold.getAndSet(0));
     consecutiveIgnoredFullGcsOverThreshold.set(0);
   }
+
+  private static MemoryPressureOptions inactiveOptions() {
+    var options = Options.getDefaults(MemoryPressureOptions.class);
+    options.oomMoreEagerlyThreshold = 100;
+    return options;
+  }
 }

src/test/java/com/google/devtools/build/lib/runtime/RetainedHeapLimiterTest.java

@@ -23,8 +23,10 @@
 import static org.mockito.Mockito.verifyNoInteractions;
 import static org.mockito.Mockito.verifyNoMoreInteractions;
 
+import com.google.common.collect.ImmutableList;
 import com.google.devtools.build.lib.bugreport.BugReporter;
 import com.google.devtools.build.lib.bugreport.Crash;
+import com.google.devtools.build.lib.runtime.GcThrashingDetector.Limit;
 import com.google.devtools.build.lib.runtime.MemoryPressure.MemoryPressureStats;
 import com.google.devtools.build.lib.testutil.ManualClock;
 import com.google.devtools.common.options.Options;
@@ -277,11 +279,35 @@ public void threshold100_noGcTriggeredEvenWithNonsenseStats() {
   }
 
   @Test
-  public void worksWithoutSettingOptions() {
-    underTest.handle(percentUsedAfterOrganicFullGc(95));
+  public void optionsNotSet_disabled() {
+    underTest.handle(percentUsedAfterOrganicFullGc(99));
     assertStats(MemoryPressureStats.newBuilder().setManuallyTriggeredGcs(0));
   }
 
+  @Test
+  public void gcThrashingLimitsSet_mutuallyExclusive_disabled() {
+    options.oomMoreEagerlyThreshold = 90;
+    options.gcThrashingLimits = ImmutableList.of(Limit.of(Duration.ofMinutes(1), 2));
+    options.gcThrashingLimitsRetainedHeapLimiterMutuallyExclusive = true;
+    underTest.setOptions(options);
+
+    underTest.handle(percentUsedAfterOrganicFullGc(99));
+
+    assertStats(MemoryPressureStats.newBuilder().setManuallyTriggeredGcs(0));
+  }
+
+  @Test
+  public void gcThrashingLimitsSet_mutuallyInclusive_enabled() {
+    options.oomMoreEagerlyThreshold = 90;
+    options.gcThrashingLimits = ImmutableList.of(Limit.of(Duration.ofMinutes(1), 2));
+    options.gcThrashingLimitsRetainedHeapLimiterMutuallyExclusive = false;
+    underTest.setOptions(options);
+
+    underTest.handle(percentUsedAfterOrganicFullGc(99));
+
+    assertStats(MemoryPressureStats.newBuilder().setManuallyTriggeredGcs(1));
+  }
+
   @Test
   public void statsReset() {
     options.oomMoreEagerlyThreshold = 90;