Collect MemoryPressureStats.

PiperOrigin-RevId: 516222399 Change-Id: I0b1074e382aeb21e38c2b55df5edf606930acabc
bazelbuild · Mar 13, 2023 · 0e6cc07 · 0e6cc07
1 parent 09c5cc3
commit 0e6cc07
Show file tree

Hide file tree

Showing 12 changed files with 273 additions and 49 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/BUILD b/src/main/java/com/google/devtools/build/lib/BUILD
@@ -222,11 +222,15 @@ java_library(
 )
 
 java_library(
-    name = "runtime/memory_pressure_event",
+    name = "runtime/memory_pressure",
     srcs = [
         "runtime/MemoryPressureEvent.java",
+        "runtime/MemoryPressureOptions.java",
+        "runtime/MemoryPressureStatCollector.java",
     ],
     deps = [
+        "//src/main/java/com/google/devtools/common/options",
+        "//src/main/protobuf:memory_pressure_java_proto",
         "//third_party:auto_value",
         "//third_party:guava",
     ],
@@ -261,6 +265,9 @@ java_library(
             "runtime/CommandLinePathFactory.java",
             "runtime/KeepGoingOption.java",
             "runtime/LoadingPhaseThreadsOption.java",
+            "runtime/MemoryPressureEvent.java",
+            "runtime/MemoryPressureOptions.java",
+            "runtime/MemoryPressureStatCollector.java",
         ],
     ),
     deps = [
@@ -271,6 +278,7 @@ java_library(
         ":runtime/blaze_command_result",
         ":runtime/command_dispatcher",
         ":runtime/command_line_path_factory",
+        ":runtime/memory_pressure",
         "//src/main/java/com/google/devtools/build/lib/actions",
         "//src/main/java/com/google/devtools/build/lib/actions:action_lookup_data",
         "//src/main/java/com/google/devtools/build/lib/actions:artifacts",
@@ -420,6 +428,7 @@ java_library(
         "//src/main/protobuf:execution_graph_java_proto",
         "//src/main/protobuf:failure_details_java_proto",
         "//src/main/protobuf:invocation_policy_java_proto",
+        "//src/main/protobuf:memory_pressure_java_proto",
         "//src/main/protobuf:option_filters_java_proto",
         "//src/main/protobuf:test_status_java_proto",
         "//third_party:auto_value",

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
@@ -15,10 +15,14 @@
 package com.google.devtools.build.lib.runtime;
 
 import com.google.common.collect.ImmutableList;
+import com.google.common.eventbus.EventBus;
+import com.google.common.eventbus.Subscribe;
 import com.google.devtools.build.lib.analysis.BlazeDirectories;
+import com.google.devtools.build.lib.runtime.MemoryPressure.MemoryPressureStats;
 import com.google.devtools.build.lib.skyframe.HighWaterMarkLimiter;
 import com.google.devtools.build.lib.util.AbruptExitException;
 import com.google.devtools.common.options.OptionsBase;
+import com.google.errorprone.annotations.Keep;
 
 /**
  * A {@link BlazeModule} that installs a {@link MemoryPressureListener} that reacts to memory
@@ -27,6 +31,8 @@
 public final class MemoryPressureModule extends BlazeModule {
   private RetainedHeapLimiter retainedHeapLimiter;
   private MemoryPressureListener memoryPressureListener;
+  private HighWaterMarkLimiter highWaterMarkLimiter;
+  private EventBus eventBus;
 
   @Override
   public void workspaceInit(
@@ -35,31 +41,44 @@ public void workspaceInit(
     memoryPressureListener = MemoryPressureListener.create(retainedHeapLimiter);
   }
 
+  @Override
+  public ImmutableList<Class<? extends OptionsBase>> getCommandOptions(Command command) {
+    return ImmutableList.of(MemoryPressureOptions.class);
+  }
+
   @Override
   public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
-    memoryPressureListener.setEventBus(env.getEventBus());
+    eventBus = env.getEventBus();
+    memoryPressureListener.setEventBus(eventBus);
 
     MemoryPressureOptions options = env.getOptions().getOptions(MemoryPressureOptions.class);
-    HighWaterMarkLimiter highWaterMarkLimiter =
-        new HighWaterMarkLimiter(
-            env.getSkyframeExecutor(),
-            env.getSyscallCache(),
-            options.skyframeHighWaterMarkMemoryThreshold,
-            options.skyframeHighWaterMarkMinorGcDropsPerInvocation,
-            options.skyframeHighWaterMarkFullGcDropsPerInvocation);
+    highWaterMarkLimiter =
+        new HighWaterMarkLimiter(env.getSkyframeExecutor(), env.getSyscallCache(), options);
 
     retainedHeapLimiter.setOptions(options);
 
-    env.getEventBus().register(highWaterMarkLimiter);
+    eventBus.register(this);
+    eventBus.register(highWaterMarkLimiter);
   }
 
   @Override
   public void afterCommand() {
+    postStats();
     memoryPressureListener.setEventBus(null);
+    eventBus = null;
+    highWaterMarkLimiter = null;
   }
 
-  @Override
-  public ImmutableList<Class<? extends OptionsBase>> getCommandOptions(Command command) {
-    return ImmutableList.of(MemoryPressureOptions.class);
+  @Subscribe
+  @Keep
+  void onCrash(@SuppressWarnings("unused") CrashEvent event) {
+    postStats();
+  }
+
+  private void postStats() {
+    MemoryPressureStats.Builder stats = MemoryPressureStats.newBuilder();
+    retainedHeapLimiter.addStatsAndReset(stats);
+    highWaterMarkLimiter.addStatsAndReset(stats);
+    eventBus.post(stats.build());
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/MemoryPressureStatCollector.java b/src/main/java/com/google/devtools/build/lib/runtime/MemoryPressureStatCollector.java
@@ -0,0 +1,28 @@
+// Copyright 2023 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.build.lib.runtime;
+
+import com.google.devtools.build.lib.runtime.MemoryPressure.MemoryPressureStats;
+
+/** Contributes to {@link MemoryPressureStats}. */
+public interface MemoryPressureStatCollector {
+  /**
+   * Called after each command to populate {@link MemoryPressureStats}.
+   *
+   * <p>If the lifetime of this collector is more than a single command, all per-command statistics
+   * that it tracks are reset by this call.
+   */
+  void addStatsAndReset(MemoryPressureStats.Builder stats);
+}
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/RetainedHeapLimiter.java b/src/main/java/com/google/devtools/build/lib/runtime/RetainedHeapLimiter.java
@@ -24,12 +24,14 @@
 import com.google.devtools.build.lib.clock.BlazeClock;
 import com.google.devtools.build.lib.clock.Clock;
 import com.google.devtools.build.lib.concurrent.ThreadSafety;
+import com.google.devtools.build.lib.runtime.MemoryPressure.MemoryPressureStats;
 import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
 import com.google.devtools.build.lib.server.FailureDetails.MemoryOptions;
 import com.google.devtools.build.lib.util.AbruptExitException;
 import com.google.devtools.build.lib.util.DetailedExitCode;
 import com.google.devtools.common.options.Options;
 import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicLong;
 
 /**
@@ -40,7 +42,7 @@
  * stop-the-world collection; if it's still more than {@link
  * MemoryPressureOptions#oomMoreEagerlyThreshold}% full, exit with an {@link OutOfMemoryError}.
  */
-final class RetainedHeapLimiter {
+final class RetainedHeapLimiter implements MemoryPressureStatCollector {
 
   private static final GoogleLogger logger = GoogleLogger.forEnclosingClass();
 
@@ -51,8 +53,11 @@ final class RetainedHeapLimiter {
 
   private final AtomicBoolean throwingOom = new AtomicBoolean(false);
   private final AtomicBoolean heapLimiterTriggeredGc = new AtomicBoolean(false);
+  private final AtomicInteger consecutiveIgnoredFullGcsOverThreshold = new AtomicInteger(0);
   private final AtomicBoolean loggedIgnoreWarningSinceLastGc = new AtomicBoolean(false);
   private final AtomicLong lastTriggeredGcMillis = new AtomicLong();
+  private final AtomicInteger gcsTriggered = new AtomicInteger(0);
+  private final AtomicInteger maxConsecutiveIgnoredFullGcsOverThreshold = new AtomicInteger(0);
 
   static RetainedHeapLimiter create(BugReporter bugReporter) {
     return new RetainedHeapLimiter(bugReporter, BlazeClock.instance());
@@ -118,6 +123,7 @@ public void handle(MemoryPressureEvent event) {
       if (wasHeapLimiterTriggeredGc) {
         logger.atInfo().log("Back under threshold (%s%% of tenured space)", actual);
       }
+      consecutiveIgnoredFullGcsOverThreshold.set(0);
       return;
     }
 
@@ -141,15 +147,33 @@ public void handle(MemoryPressureEvent event) {
           "Triggering a full GC (%s%% of tenured space after %s GC)",
           actual, event.wasFullGc() ? "full" : "minor");
       heapLimiterTriggeredGc.set(true);
+      gcsTriggered.incrementAndGet();
       // Force a full stop-the-world GC and see if it can get us below the threshold.
       System.gc();
       lastTriggeredGcMillis.set(clock.currentTimeMillis());
+      consecutiveIgnoredFullGcsOverThreshold.set(0);
       loggedIgnoreWarningSinceLastGc.set(false);
-    } else if (!loggedIgnoreWarningSinceLastGc.getAndSet(true) || event.wasFullGc()) {
+    } else if (event.wasFullGc()) {
+      int consecutiveIgnored = consecutiveIgnoredFullGcsOverThreshold.incrementAndGet();
+      maxConsecutiveIgnoredFullGcsOverThreshold.accumulateAndGet(consecutiveIgnored, Math::max);
       logger.atWarning().log(
-          "Ignoring possible GC thrashing (%s%% of tenured space after %s GC) because of recently"
-              + " triggered GC",
-          actual, event.wasFullGc() ? "full" : "minor");
+          "Ignoring possible GC thrashing x%s (%s%% of tenured space after full GC) because of"
+              + " recently triggered GC",
+          consecutiveIgnored, actual);
+    } else if (!loggedIgnoreWarningSinceLastGc.getAndSet(true)) {
+      logger.atWarning().log(
+          "Ignoring possible GC thrashing (%s%% of tenured space after minor GC) because of"
+              + " recently triggered GC",
+          actual);
     }
   }
+
+  @Override
+  public void addStatsAndReset(MemoryPressureStats.Builder stats) {
+    stats
+        .setManuallyTriggeredGcs(gcsTriggered.getAndSet(0))
+        .setMaxConsecutiveIgnoredGcsOverThreshold(
+            maxConsecutiveIgnoredFullGcsOverThreshold.getAndSet(0));
+    consecutiveIgnoredFullGcsOverThreshold.set(0);
+  }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BUILD b/src/main/java/com/google/devtools/build/lib/skyframe/BUILD
@@ -225,7 +225,7 @@ java_library(
         ":workspace_status_value",
         "//src/main/java/com/google/devtools/build/lib:build-request-options",
         "//src/main/java/com/google/devtools/build/lib:keep-going-option",
-        "//src/main/java/com/google/devtools/build/lib:runtime/memory_pressure_event",
+        "//src/main/java/com/google/devtools/build/lib:runtime/memory_pressure",
         "//src/main/java/com/google/devtools/build/lib/actions",
         "//src/main/java/com/google/devtools/build/lib/actions:action_lookup_data",
         "//src/main/java/com/google/devtools/build/lib/actions:action_lookup_key",
@@ -354,6 +354,7 @@ java_library(
         "//src/main/java/net/starlark/java/eval",
         "//src/main/java/net/starlark/java/syntax",
         "//src/main/protobuf:failure_details_java_proto",
+        "//src/main/protobuf:memory_pressure_java_proto",
         "//third_party:auto_value",
         "//third_party:caffeine",
         "//third_party:flogger",

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/HighWaterMarkLimiter.java b/src/main/java/com/google/devtools/build/lib/skyframe/HighWaterMarkLimiter.java
@@ -14,12 +14,15 @@
 
 package com.google.devtools.build.lib.skyframe;
 
-import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkNotNull;
 import static java.util.concurrent.TimeUnit.SECONDS;
 
 import com.google.common.eventbus.Subscribe;
 import com.google.common.flogger.GoogleLogger;
+import com.google.devtools.build.lib.runtime.MemoryPressure.MemoryPressureStats;
 import com.google.devtools.build.lib.runtime.MemoryPressureEvent;
+import com.google.devtools.build.lib.runtime.MemoryPressureOptions;
+import com.google.devtools.build.lib.runtime.MemoryPressureStatCollector;
 import com.google.devtools.build.lib.vfs.SyscallCache;
 
 /**
@@ -38,37 +41,28 @@
  * performance of Blaze's SkyFunctions and (ii) using SkyKeyComputeState but then GC thrashing and
  * suffering when Blaze is memory constrained. Instead, we get the best of both worlds.
  */
-public class HighWaterMarkLimiter {
+public final class HighWaterMarkLimiter implements MemoryPressureStatCollector {
   private static final GoogleLogger logger = GoogleLogger.forEnclosingClass();
 
   private final SkyframeExecutor skyframeExecutor;
   private final SyscallCache syscallCache;
-  private final int threshold;
+  private final MemoryPressureOptions options;
   private int minorGcDropsRemaining;
   private int fullGcDropsRemaining;
 
   public HighWaterMarkLimiter(
-      SkyframeExecutor skyframeExecutor,
-      SyscallCache syscallCache,
-      int threshold,
-      int minorGcDropLimit,
-      int fullGcDropLimit) {
-    this.skyframeExecutor = skyframeExecutor;
-    this.syscallCache = syscallCache;
-    this.threshold = threshold;
-
-    checkArgument(
-        minorGcDropLimit >= 0, "minorGcDropLimit must be non-negative, was %s", minorGcDropLimit);
-    this.minorGcDropsRemaining = minorGcDropLimit;
-
-    checkArgument(
-        fullGcDropLimit >= 0, "fullGcDropLimit must be non-negative, was %s", fullGcDropLimit);
-    this.fullGcDropsRemaining = fullGcDropLimit;
+      SkyframeExecutor skyframeExecutor, SyscallCache syscallCache, MemoryPressureOptions options) {
+    this.skyframeExecutor = checkNotNull(skyframeExecutor);
+    this.syscallCache = checkNotNull(syscallCache);
+    this.options = checkNotNull(options);
+    this.minorGcDropsRemaining = options.skyframeHighWaterMarkMinorGcDropsPerInvocation;
+    this.fullGcDropsRemaining = options.skyframeHighWaterMarkFullGcDropsPerInvocation;
   }
 
   @Subscribe
   void handle(MemoryPressureEvent event) {
     int actual = (int) ((event.tenuredSpaceUsedBytes() * 100L) / event.tenuredSpaceMaxBytes());
+    int threshold = options.skyframeHighWaterMarkMemoryThreshold;
     if (actual < threshold) {
       return;
     }
@@ -104,4 +98,13 @@ void handle(MemoryPressureEvent event) {
     skyframeExecutor.dropUnnecessaryTemporarySkyframeState();
     syscallCache.clear();
   }
+
+  @Override
+  public void addStatsAndReset(MemoryPressureStats.Builder stats) {
+    stats
+        .setMinorGcDrops(
+            options.skyframeHighWaterMarkMinorGcDropsPerInvocation - minorGcDropsRemaining)
+        .setFullGcDrops(
+            options.skyframeHighWaterMarkFullGcDropsPerInvocation - fullGcDropsRemaining);
+  }
 }
diff --git a/src/main/protobuf/BUILD b/src/main/protobuf/BUILD
@@ -19,6 +19,7 @@ FILES = [
     "extra_actions_base",
     "invocation_policy",
     "java_compilation",
+    "memory_pressure",
     "test_status",
     "worker_protocol",
     "execution_graph",

diff --git a/src/main/protobuf/memory_pressure.proto b/src/main/protobuf/memory_pressure.proto
@@ -0,0 +1,32 @@
+// Copyright 2023 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+syntax = "proto3";
+
+package memory_pressure;
+
+option java_package = "com.google.devtools.build.lib.runtime";
+
+// Statistics about memory pressure handling.
+message MemoryPressureStats {
+  // The number of calls to System.gc() made by RetainedHeapLimiter.
+  int32 manually_triggered_gcs = 1;
+  // The maximum number of consecutive full GC events over the threshold ignored
+  // by RetainedHeapLimiter because it recently triggered a GC.
+  int32 max_consecutive_ignored_gcs_over_threshold = 2;
+  // Number of times HighWaterMarkLimiter dropped caches after minor GCs.
+  int32 minor_gc_drops = 3;
+  // Number of times HighWaterMarkLimiter dropped caches after full GCs.
+  int32 full_gc_drops = 4;
+}
diff --git a/src/test/java/com/google/devtools/build/lib/BUILD b/src/test/java/com/google/devtools/build/lib/BUILD
@@ -127,6 +127,7 @@ java_test(
         "//src/main/java/com/google/devtools/build/lib:runtime/blaze_command_result",
         "//src/main/java/com/google/devtools/build/lib:runtime/command_dispatcher",
         "//src/main/java/com/google/devtools/build/lib:runtime/command_line_path_factory",
+        "//src/main/java/com/google/devtools/build/lib:runtime/memory_pressure",
         "//src/main/java/com/google/devtools/build/lib:runtime/safe_request_logging",
         "//src/main/java/com/google/devtools/build/lib/actions",
         "//src/main/java/com/google/devtools/build/lib/actions:action_input_helper",
@@ -196,6 +197,7 @@ java_test(
         "//src/main/protobuf:execution_graph_java_proto",
         "//src/main/protobuf:failure_details_java_proto",
         "//src/main/protobuf:invocation_policy_java_proto",
+        "//src/main/protobuf:memory_pressure_java_proto",
         "//src/main/protobuf:test_status_java_proto",
         "//src/main/protobuf:wrappers_java_proto",
         "//src/test/java/com/google/devtools/build/lib/actions/util",