Refactor `RetainedHeapLimiter` into (i) a general purpose mechanism for noticing memory pressure and (ii) a use of that mechanism to achieve the actual goal of `RetainedHeapLimiter`. This accomplishes two things: * Strict decrease in code complexity. * By using a custom `BlazeModule`, we can get rid of the special-casing in `BlazeCommandDispatcher` and `BlazeRuntime`. * The new `RetainedHeapLimiter` is simpler since the code for noticing memory pressure has been factored out. Check out the unit tests :) * Opens the door for new functionality triggered by memory pressure. I will be adding one myself in the very near future. And I can imagine other pieces of functionality we can add too. PiperOrigin-RevId: 417856795
diff --git a/src/main/java/com/google/devtools/build/lib/BUILD b/src/main/java/com/google/devtools/build/lib/BUILD index 8b7d2ec..5f20f18 100644 --- a/src/main/java/com/google/devtools/build/lib/BUILD +++ b/src/main/java/com/google/devtools/build/lib/BUILD
@@ -206,6 +206,17 @@ ) java_library( + name = "runtime/memory_pressure_handler", + srcs = [ + "runtime/MemoryPressureHandler.java", + ], + deps = [ + "//src/main/java/com/google/devtools/build/lib/concurrent:thread_safety", + "//third_party/java/auto:auto_value", + ], +) + +java_library( name = "runtime", srcs = glob( [
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/Bazel.java b/src/main/java/com/google/devtools/build/lib/bazel/Bazel.java index 11ad22e..54180cf 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/Bazel.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/Bazel.java
@@ -43,6 +43,7 @@ // implementation. com.google.devtools.build.lib.runtime.NoSpawnCacheModule.class, com.google.devtools.build.lib.runtime.CommandLogModule.class, + com.google.devtools.build.lib.runtime.MemoryPressureModule.class, com.google.devtools.build.lib.platform.SleepPreventionModule.class, com.google.devtools.build.lib.platform.SystemSuspensionModule.class, com.google.devtools.build.lib.runtime.BazelFileSystemModule.class,
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java index e7a0ce8..475850e 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java
@@ -444,8 +444,6 @@ reporter.addHandler(handler); env.getEventBus().register(handler); - runtime.getRetainedHeapLimiter().update(commonOptions.oomMoreEagerlyThreshold); - // We register an ANSI-allowing handler associated with {@code handler} so that ANSI control // codes can be re-introduced later even if blaze is invoked with --color=no. This is useful // for commands such as 'blaze run' where the output of the final executable shouldn't be
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java index 50d7f1b..b49fd0e 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
@@ -191,7 +191,6 @@ private final BuildEventArtifactUploaderFactoryMap buildEventArtifactUploaderFactoryMap; private final ActionKeyContext actionKeyContext; private final ImmutableMap<String, AuthHeadersProvider> authHeadersProviderMap; - private final RetainedHeapLimiter retainedHeapLimiter; @Nullable private final RepositoryRemoteExecutorFactory repositoryRemoteExecutorFactory; private final Supplier<Downloader> downloaderSupplier; @@ -243,7 +242,6 @@ this.queryOutputFormatters = queryOutputFormatters; this.eventBusExceptionHandler = eventBusExceptionHandler; this.bugReporter = bugReporter; - retainedHeapLimiter = RetainedHeapLimiter.create(bugReporter); CommandNameCache.CommandNameCacheInstance.INSTANCE.setCommandNameCache( new CommandNameCacheImpl(commandMap)); @@ -522,10 +520,6 @@ return queryRuntimeHelperFactory; } - RetainedHeapLimiter getRetainedHeapLimiter() { - return retainedHeapLimiter; - } - /** * Hook method called by the BlazeCommandDispatcher prior to the dispatch of each command. *
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/MemoryPressureHandler.java b/src/main/java/com/google/devtools/build/lib/runtime/MemoryPressureHandler.java new file mode 100644 index 0000000..8f3ec4e --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/runtime/MemoryPressureHandler.java
@@ -0,0 +1,49 @@ +// Copyright 2021 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.auto.value.AutoValue; +import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; + +/** A handler of memory pressure events. */ +public interface MemoryPressureHandler { + @ThreadSafe + void handle(Event event); + + /** A memory pressure event. */ + @AutoValue + abstract class Event { + public abstract boolean wasManualGc(); + + public abstract long tenuredSpaceUsedBytes(); + + public abstract long tenuredSpaceMaxBytes(); + + static Builder newBuilder() { + return new AutoValue_MemoryPressureHandler_Event.Builder(); + } + + @AutoValue.Builder + abstract static class Builder { + abstract Builder setWasManualGc(boolean value); + + abstract Builder setTenuredSpaceUsedBytes(long value); + + abstract Builder setTenuredSpaceMaxBytes(long value); + + abstract Event build(); + } + } +}
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 new file mode 100644 index 0000000..c3c70b9 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/runtime/MemoryPressureListener.java
@@ -0,0 +1,139 @@ +// Copyright 2021 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 static com.google.common.collect.ImmutableList.toImmutableList; + +import com.google.common.annotations.VisibleForTesting; +import com.google.common.collect.ImmutableList; +import com.google.common.flogger.GoogleLogger; +import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; +import com.sun.management.GarbageCollectionNotificationInfo; +import java.lang.management.GarbageCollectorMXBean; +import java.lang.management.ManagementFactory; +import java.lang.management.MemoryUsage; +import java.util.Arrays; +import java.util.Map; +import javax.annotation.Nullable; +import javax.management.Notification; +import javax.management.NotificationEmitter; +import javax.management.NotificationListener; +import javax.management.openmbean.CompositeData; + +@ThreadSafe +class MemoryPressureListener implements NotificationListener { + private static final GoogleLogger logger = GoogleLogger.forEnclosingClass(); + + private final ImmutableList<MemoryPressureHandler> handlers; + + private MemoryPressureListener(ImmutableList<MemoryPressureHandler> handlers) { + this.handlers = handlers; + } + + @Nullable + static MemoryPressureListener create(ImmutableList<MemoryPressureHandler> handlers) { + return createFromBeans( + ImmutableList.copyOf(ManagementFactory.getGarbageCollectorMXBeans()), + handlers); + } + + @VisibleForTesting + @Nullable + static MemoryPressureListener createFromBeans( + ImmutableList<GarbageCollectorMXBean> gcBeans, + ImmutableList<MemoryPressureHandler> handlers) { + ImmutableList<NotificationEmitter> tenuredGcEmitters = findTenuredCollectorBeans(gcBeans); + if (tenuredGcEmitters.isEmpty()) { + logger.atSevere().log( + "Unable to find tenured collector from %s: names were %s.", + gcBeans, + gcBeans.stream() + .map(GarbageCollectorMXBean::getMemoryPoolNames) + .map(Arrays::asList) + .collect(toImmutableList())); + return null; + } + MemoryPressureListener memoryPressureListener = new MemoryPressureListener(handlers); + tenuredGcEmitters.forEach(e -> e.addNotificationListener(memoryPressureListener, null, null)); + return memoryPressureListener; + } + + @VisibleForTesting + static ImmutableList<NotificationEmitter> findTenuredCollectorBeans( + Iterable<GarbageCollectorMXBean> gcBeans) { + ImmutableList.Builder<NotificationEmitter> builder = ImmutableList.builder(); + // Examine all collectors and register for notifications from those which collect the tenured + // space. Normally there is one such collector. + for (GarbageCollectorMXBean gcBean : gcBeans) { + for (String name : gcBean.getMemoryPoolNames()) { + if (isTenuredSpace(name)) { + builder.add((NotificationEmitter) gcBean); + } + } + } + return builder.build(); + } + + @Override + public void handleNotification(Notification notification, Object handback) { + if (!notification + .getType() + .equals(GarbageCollectionNotificationInfo.GARBAGE_COLLECTION_NOTIFICATION)) { + return; + } + + GarbageCollectionNotificationInfo gcInfo = + GarbageCollectionNotificationInfo.from((CompositeData) notification.getUserData()); + + long tenuredSpaceUsedBytes = 0L; + long tenuredSpaceMaxBytes = 0L; + for (Map.Entry<String, MemoryUsage> memoryUsageEntry : + gcInfo.getGcInfo().getMemoryUsageAfterGc().entrySet()) { + if (!isTenuredSpace(memoryUsageEntry.getKey())) { + continue; + } + MemoryUsage space = memoryUsageEntry.getValue(); + if (space.getMax() == 0L) { + // The collector sometimes passes us nonsense stats. + continue; + } + tenuredSpaceUsedBytes = space.getUsed(); + tenuredSpaceMaxBytes = space.getMax(); + break; + } + if (tenuredSpaceMaxBytes == 0L) { + return; + } + + MemoryPressureHandler.Event event = + MemoryPressureHandler.Event.newBuilder() + .setWasManualGc(gcInfo.getGcCause().equals("System.gc()")) + .setTenuredSpaceUsedBytes(tenuredSpaceUsedBytes) + .setTenuredSpaceMaxBytes(tenuredSpaceMaxBytes) + .build(); + for (MemoryPressureHandler handler : handlers) { + handler.handle(event); + } + } + + private static boolean isTenuredSpace(String name) { + return "CMS Old Gen".equals(name) + || "G1 Old Gen".equals(name) + || "PS Old Gen".equals(name) + || "Tenured Gen".equals(name) + || "Shenandoah".equals(name) + || "ZHeap".equals(name); + } +} \ No newline at end of file
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 new file mode 100644 index 0000000..d2edd15 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/runtime/MemoryPressureModule.java
@@ -0,0 +1,43 @@ +// Copyright 2021 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.common.collect.ImmutableList; +import com.google.devtools.build.lib.analysis.BlazeDirectories; +import com.google.devtools.build.lib.util.AbruptExitException; +import javax.annotation.Nullable; + +/** + * A {@link BlazeModule} that installs a {@link MemoryPressureListener} that reacts to memory + * pressure events. + */ +public class MemoryPressureModule extends BlazeModule { + private RetainedHeapLimiter retainedHeapLimiter; + @Nullable private MemoryPressureListener memoryPressureListener; + + @Override + public void workspaceInit( + BlazeRuntime runtime, BlazeDirectories directories, WorkspaceBuilder builder) { + retainedHeapLimiter = RetainedHeapLimiter.create(runtime.getBugReporter()); + memoryPressureListener = MemoryPressureListener.create(ImmutableList.of(retainedHeapLimiter)); + } + + @Override + public void beforeCommand(CommandEnvironment env) throws AbruptExitException { + CommonCommandOptions commonOptions = env.getOptions().getOptions(CommonCommandOptions.class); + retainedHeapLimiter.setThreshold( + /*listening=*/ memoryPressureListener != null, commonOptions.oomMoreEagerlyThreshold); + } +}
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 index 34224df..102b2b3 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/RetainedHeapLimiter.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/RetainedHeapLimiter.java
@@ -15,11 +15,7 @@ package com.google.devtools.build.lib.runtime; import static com.google.common.base.Preconditions.checkNotNull; -import static com.google.common.collect.ImmutableList.toImmutableList; -import static java.util.concurrent.TimeUnit.MINUTES; -import com.google.common.annotations.VisibleForTesting; -import com.google.common.collect.ImmutableList; import com.google.common.flogger.GoogleLogger; import com.google.devtools.build.lib.bugreport.BugReporter; import com.google.devtools.build.lib.bugreport.Crash; @@ -29,22 +25,8 @@ 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.sun.management.GarbageCollectionNotificationInfo; -import java.lang.management.GarbageCollectorMXBean; -import java.lang.management.ManagementFactory; -import java.lang.management.MemoryUsage; -import java.util.Arrays; -import java.util.List; -import java.util.Map; -import java.util.OptionalInt; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicLong; -import javax.annotation.Nullable; -import javax.management.ListenerNotFoundException; -import javax.management.Notification; -import javax.management.NotificationEmitter; -import javax.management.NotificationListener; -import javax.management.openmbean.CompositeData; /** * Monitors the size of the retained heap and exit promptly if it grows too large. @@ -54,184 +36,41 @@ * collection; if it's still more than {@link #occupiedHeapPercentageThreshold}% full, exit with an * {@link OutOfMemoryError}. */ -final class RetainedHeapLimiter implements NotificationListener { +final class RetainedHeapLimiter implements MemoryPressureHandler { private static final GoogleLogger logger = GoogleLogger.forEnclosingClass(); private static final long MIN_TIME_BETWEEN_TRIGGERED_GC_MILLISECONDS = 60000; private final AtomicBoolean throwingOom = new AtomicBoolean(false); private final AtomicBoolean heapLimiterTriggeredGc = new AtomicBoolean(false); - private final ImmutableList<NotificationEmitter> tenuredGcEmitters; - private OptionalInt occupiedHeapPercentageThreshold = OptionalInt.empty(); + private volatile int occupiedHeapPercentageThreshold = 100; private final AtomicLong lastTriggeredGcInMilliseconds = new AtomicLong(); private final BugReporter bugReporter; static RetainedHeapLimiter create(BugReporter bugReporter) { - return createFromBeans(ManagementFactory.getGarbageCollectorMXBeans(), bugReporter); + return new RetainedHeapLimiter(bugReporter); } - @VisibleForTesting - static RetainedHeapLimiter createFromBeans( - List<GarbageCollectorMXBean> gcBeans, BugReporter bugReporter) { - ImmutableList<NotificationEmitter> tenuredGcEmitters = findTenuredCollectorBeans(gcBeans); - if (tenuredGcEmitters.isEmpty()) { - logger.atSevere().log( - "Unable to find tenured collector from %s: names were %s. " - + "--experimental_oom_more_eagerly_threshold cannot be specified for this JVM", - gcBeans, - gcBeans.stream() - .map(GarbageCollectorMXBean::getMemoryPoolNames) - .map(Arrays::asList) - .collect(toImmutableList())); - } - return new RetainedHeapLimiter(tenuredGcEmitters, bugReporter); - } - - private RetainedHeapLimiter( - ImmutableList<NotificationEmitter> tenuredGcEmitters, BugReporter bugReporter) { - this.tenuredGcEmitters = checkNotNull(tenuredGcEmitters); + private RetainedHeapLimiter(BugReporter bugReporter) { this.bugReporter = checkNotNull(bugReporter); } @ThreadSafety.ThreadCompatible // Can only be called on the logical main Bazel thread. - void update(int oomMoreEagerlyThreshold) throws AbruptExitException { - if (tenuredGcEmitters.isEmpty() && oomMoreEagerlyThreshold != 100) { - throw createExitException( - "No tenured GC collectors were found: unable to watch for GC events to exit JVM when " - + oomMoreEagerlyThreshold - + "% of heap is used", - MemoryOptions.Code.EXPERIMENTAL_OOM_MORE_EAGERLY_NO_TENURED_COLLECTORS_FOUND); - } + void setThreshold(boolean listening, int oomMoreEagerlyThreshold) throws AbruptExitException { if (oomMoreEagerlyThreshold < 0 || oomMoreEagerlyThreshold > 100) { throw createExitException( "--experimental_oom_more_eagerly_threshold must be a percent between 0 and 100 but was " + oomMoreEagerlyThreshold, MemoryOptions.Code.EXPERIMENTAL_OOM_MORE_EAGERLY_THRESHOLD_INVALID_VALUE); } - boolean alreadyInstalled = this.occupiedHeapPercentageThreshold.isPresent(); - this.occupiedHeapPercentageThreshold = - oomMoreEagerlyThreshold < 100 - ? OptionalInt.of(oomMoreEagerlyThreshold) - : OptionalInt.empty(); - boolean shouldBeInstalled = this.occupiedHeapPercentageThreshold.isPresent(); - if (alreadyInstalled && !shouldBeInstalled) { - for (NotificationEmitter emitter : tenuredGcEmitters) { - try { - emitter.removeNotificationListener(this, null, null); - } catch (ListenerNotFoundException e) { - logger.atWarning().withCause(e).log("Couldn't remove self as listener from %s", emitter); - } - } - } else if (!alreadyInstalled && shouldBeInstalled) { - tenuredGcEmitters.forEach(e -> e.addNotificationListener(this, null, null)); + if (!listening && oomMoreEagerlyThreshold != 100) { + throw createExitException( + "No tenured GC collectors were found: unable to watch for GC events to exit JVM when " + + oomMoreEagerlyThreshold + + "% of heap is used", + MemoryOptions.Code.EXPERIMENTAL_OOM_MORE_EAGERLY_NO_TENURED_COLLECTORS_FOUND); } - } - - @VisibleForTesting - static ImmutableList<NotificationEmitter> findTenuredCollectorBeans( - List<GarbageCollectorMXBean> gcBeans) { - ImmutableList.Builder<NotificationEmitter> builder = ImmutableList.builder(); - // Examine all collectors and register for notifications from those which collect the tenured - // space. Normally there is one such collector. - for (GarbageCollectorMXBean gcBean : gcBeans) { - for (String name : gcBean.getMemoryPoolNames()) { - if (isTenuredSpace(name)) { - builder.add((NotificationEmitter) gcBean); - } - } - } - return builder.build(); - } - - // Can be called concurrently, handles concurrent calls with #update gracefully. - @ThreadSafety.ThreadSafe - @Override - public void handleNotification(Notification notification, Object handback) { - if (!notification - .getType() - .equals(GarbageCollectionNotificationInfo.GARBAGE_COLLECTION_NOTIFICATION)) { - return; - } - // Get a local reference to guard against concurrent modifications. - OptionalInt occupiedHeapPercentageThreshold = this.occupiedHeapPercentageThreshold; - if (!occupiedHeapPercentageThreshold.isPresent()) { - // Presumably failure above to uninstall this listener, or a racy GC. - logger.atInfo().atMostEvery(1, MINUTES).log( - "Got notification %s when should be disabled", notification); - return; - } - int threshold = occupiedHeapPercentageThreshold.getAsInt(); - GarbageCollectionNotificationInfo info = - GarbageCollectionNotificationInfo.from((CompositeData) notification.getUserData()); - boolean manualGc = info.getGcCause().equals("System.gc()"); - if (manualGc && !heapLimiterTriggeredGc.getAndSet(false)) { - // This was a manually triggered GC, but not from the other branch: short-circuit. - return; - } - - @Nullable - MemoryUsage space = getTenuredSpacedIfFull(info.getGcInfo().getMemoryUsageAfterGc(), threshold); - if (space == null) { - return; - } - - if (manualGc) { - if (!throwingOom.getAndSet(true)) { - // We got here from a GC initiated by the other branch. - OutOfMemoryError oom = - new OutOfMemoryError( - String.format( - "RetainedHeapLimiter forcing exit due to GC thrashing: After back-to-back full" - + " GCs, the tenured space is more than %s%% occupied (%s out of a tenured" - + " space size of %s).", - threshold, space.getUsed(), space.getMax())); - // Exits the runtime. - bugReporter.handleCrash(Crash.from(oom), CrashContext.halt()); - } - } else if (System.currentTimeMillis() - lastTriggeredGcInMilliseconds.get() - > MIN_TIME_BETWEEN_TRIGGERED_GC_MILLISECONDS) { - logger.atInfo().log( - "Triggering a full GC with %s tenured space used out of a tenured space size of %s", - space.getUsed(), space.getMax()); - heapLimiterTriggeredGc.set(true); - // Force a full stop-the-world GC and see if it can get us below the threshold. - System.gc(); - lastTriggeredGcInMilliseconds.set(System.currentTimeMillis()); - } - } - - private static boolean isTenuredSpace(String name) { - return "CMS Old Gen".equals(name) - || "G1 Old Gen".equals(name) - || "PS Old Gen".equals(name) - || "Tenured Gen".equals(name) - || "Shenandoah".equals(name) - || "ZHeap".equals(name); - } - - @Nullable - private static MemoryUsage getTenuredSpacedIfFull( - Map<String, MemoryUsage> spaces, int threshold) { - ImmutableList<Map.Entry<String, MemoryUsage>> fullSpaces = - spaces.entrySet().stream() - .filter( - e -> { - if (!isTenuredSpace(e.getKey())) { - return false; - } - MemoryUsage space = e.getValue(); - if (space.getMax() == 0) { - // The collector sometimes passes us nonsense stats. - return false; - } - - return (100 * space.getUsed() / space.getMax()) > threshold; - }) - .collect(toImmutableList()); - if (fullSpaces.size() > 1) { - logger.atInfo().log("Multiple full tenured spaces: %s", fullSpaces); - } - return fullSpaces.isEmpty() ? null : fullSpaces.get(0).getValue(); + this.occupiedHeapPercentageThreshold = oomMoreEagerlyThreshold; } private static AbruptExitException createExitException(String message, MemoryOptions.Code code) { @@ -242,4 +81,46 @@ .setMemoryOptions(MemoryOptions.newBuilder().setCode(code)) .build())); } + + // Can be called concurrently, handles concurrent calls with #setThreshold gracefully. + @ThreadSafety.ThreadSafe + @Override + public void handle(Event event) { + if (event.wasManualGc() && !heapLimiterTriggeredGc.getAndSet(false)) { + // This was a manually triggered GC, but not from us earlier: short-circuit. + return; + } + + // Get a local reference to guard against concurrent modifications. + int threshold = this.occupiedHeapPercentageThreshold; + + int actual = (int) ((event.tenuredSpaceUsedBytes() * 100L) / event.tenuredSpaceMaxBytes()); + if (actual < threshold) { + return; + } + + if (event.wasManualGc()) { + if (!throwingOom.getAndSet(true)) { + // We got here from a GC initiated by the other branch. + OutOfMemoryError oom = + new OutOfMemoryError( + String.format( + "RetainedHeapLimiter forcing exit due to GC thrashing: After back-to-back full" + + " GCs, the tenured space is more than %s%% occupied (%s out of a tenured" + + " space size of %s).", + threshold, event.tenuredSpaceUsedBytes(), event.tenuredSpaceMaxBytes())); + // Exits the runtime. + bugReporter.handleCrash(Crash.from(oom), CrashContext.halt()); + } + } else if (System.currentTimeMillis() - lastTriggeredGcInMilliseconds.get() + > MIN_TIME_BETWEEN_TRIGGERED_GC_MILLISECONDS) { + logger.atInfo().log( + "Triggering a full GC with %s tenured space used out of a tenured space size of %s", + event.tenuredSpaceUsedBytes(), event.tenuredSpaceMaxBytes()); + heapLimiterTriggeredGc.set(true); + // Force a full stop-the-world GC and see if it can get us below the threshold. + System.gc(); + lastTriggeredGcInMilliseconds.set(System.currentTimeMillis()); + } + } }
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 new file mode 100644 index 0000000..605ac6c --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/runtime/MemoryPressureListenerTest.java
@@ -0,0 +1,215 @@ +// Copyright 2021 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 static com.google.common.truth.Truth.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.sun.management.GarbageCollectionNotificationInfo; +import com.sun.management.GcInfo; +import java.lang.management.GarbageCollectorMXBean; +import java.lang.management.MemoryUsage; +import javax.management.Notification; +import javax.management.NotificationEmitter; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** Tests for {@link MemoryPressureListener}. */ +@RunWith(JUnit4.class) +public final class MemoryPressureListenerTest { + private interface NotificationBean extends GarbageCollectorMXBean, NotificationEmitter {} + + private static final String TENURED_SPACE_NAME = "CMS Old Gen"; + private final NotificationBean mockBean = mock(NotificationBean.class); + private final NotificationBean mockUselessBean = mock(NotificationBean.class); + + @Before + public void initMocks() { + when(mockBean.getMemoryPoolNames()) + .thenReturn(new String[] {"not tenured", TENURED_SPACE_NAME}); + when(mockUselessBean.getMemoryPoolNames()).thenReturn(new String[] {"assistant", "adjunct"}); + } + + @Test + public void findBeans() { + assertThat( + MemoryPressureListener.findTenuredCollectorBeans( + ImmutableList.of(mockUselessBean, mockBean))) + .containsExactly(mockBean); + } + + @Test + public void createFromBeans_returnsNullIfNoTenuredSpaceBean() { + assertThat( + MemoryPressureListener.createFromBeans( + ImmutableList.of(mockUselessBean), ImmutableList.of())) + .isNull(); + } + + @Test + public void simple() { + MemoryPressureHandler mockHandler = mock(MemoryPressureHandler.class); + + MemoryPressureListener underTest = MemoryPressureListener.createFromBeans( + ImmutableList.of(mockUselessBean, mockBean), + ImmutableList.of(mockHandler)); + verify(mockBean).addNotificationListener(underTest, null, null); + verify(mockUselessBean, never()).addNotificationListener(any(), any(), any()); + + GcInfo mockGcInfo = mock(GcInfo.class); + String nonTenuredSpaceName = "nope"; + MemoryUsage mockMemoryUsageForNonTenuredSpace = mock(MemoryUsage.class); + MemoryUsage mockMemoryUsageForTenuredSpace = mock(MemoryUsage.class); + when(mockMemoryUsageForTenuredSpace.getUsed()).thenReturn(42L); + when(mockMemoryUsageForTenuredSpace.getMax()).thenReturn(100L); + when(mockGcInfo.getMemoryUsageAfterGc()) + .thenReturn( + ImmutableMap.of( + nonTenuredSpaceName, + mockMemoryUsageForNonTenuredSpace, + TENURED_SPACE_NAME, + mockMemoryUsageForTenuredSpace)); + + Notification notification = + new Notification( + GarbageCollectionNotificationInfo.GARBAGE_COLLECTION_NOTIFICATION, "test", 123); + notification.setUserData( + new GarbageCollectionNotificationInfo("gcName", "gcAction", "non-manual", mockGcInfo) + .toCompositeData(null)); + underTest.handleNotification(notification, null); + + verify(mockHandler) + .handle( + MemoryPressureHandler.Event.newBuilder() + .setWasManualGc(false) + .setTenuredSpaceUsedBytes(42L) + .setTenuredSpaceMaxBytes(100L) + .build()); + } + + @Test + public void manualGc() { + MemoryPressureHandler mockHandler = mock(MemoryPressureHandler.class); + + MemoryPressureListener underTest = MemoryPressureListener.createFromBeans( + ImmutableList.of(mockBean), + ImmutableList.of(mockHandler)); + verify(mockBean).addNotificationListener(underTest, null, null); + + GcInfo mockGcInfo = mock(GcInfo.class); + MemoryUsage mockMemoryUsageForTenuredSpace = mock(MemoryUsage.class); + when(mockMemoryUsageForTenuredSpace.getUsed()).thenReturn(42L); + when(mockMemoryUsageForTenuredSpace.getMax()).thenReturn(100L); + when(mockGcInfo.getMemoryUsageAfterGc()) + .thenReturn(ImmutableMap.of(TENURED_SPACE_NAME, mockMemoryUsageForTenuredSpace)); + + Notification notification = + new Notification( + GarbageCollectionNotificationInfo.GARBAGE_COLLECTION_NOTIFICATION, "test", 123); + notification.setUserData( + new GarbageCollectionNotificationInfo("gcName", "gcAction", "System.gc()", mockGcInfo) + .toCompositeData(null)); + underTest.handleNotification(notification, null); + + verify(mockHandler) + .handle( + MemoryPressureHandler.Event.newBuilder() + .setWasManualGc(true) + .setTenuredSpaceUsedBytes(42L) + .setTenuredSpaceMaxBytes(100L) + .build()); + } + + @Test + public void doesntInvokeHandlerWhenTenuredSpaceMaxSizeIsZero() { + MemoryPressureHandler mockHandler = mock(MemoryPressureHandler.class); + + MemoryPressureListener underTest = MemoryPressureListener.createFromBeans( + ImmutableList.of(mockBean), + ImmutableList.of(mockHandler)); + verify(mockBean).addNotificationListener(underTest, null, null); + + GcInfo mockGcInfo = mock(GcInfo.class); + MemoryUsage mockMemoryUsageForTenuredSpace = mock(MemoryUsage.class); + when(mockMemoryUsageForTenuredSpace.getUsed()).thenReturn(42L); + when(mockMemoryUsageForTenuredSpace.getMax()).thenReturn(0L); + when(mockGcInfo.getMemoryUsageAfterGc()) + .thenReturn(ImmutableMap.of(TENURED_SPACE_NAME, mockMemoryUsageForTenuredSpace)); + + Notification notification = + new Notification( + GarbageCollectionNotificationInfo.GARBAGE_COLLECTION_NOTIFICATION, "test", 123); + notification.setUserData( + new GarbageCollectionNotificationInfo("gcName", "gcAction", "non-manual", mockGcInfo) + .toCompositeData(null)); + underTest.handleNotification(notification, null); + + verify(mockHandler, never()).handle(any()); + } + + @Test + public void findsTenuredSpaceWithNonZeroMaxSize() { + MemoryPressureHandler mockHandler = mock(MemoryPressureHandler.class); + + NotificationBean anotherMockBean = mock(NotificationBean.class); + String anotherTenuredSpaceName = "G1 Old Gen"; + when(anotherMockBean.getMemoryPoolNames()).thenReturn(new String[] {anotherTenuredSpaceName}); + + MemoryPressureListener underTest = MemoryPressureListener.createFromBeans( + ImmutableList.of(mockBean, anotherMockBean), + ImmutableList.of(mockHandler)); + verify(mockBean).addNotificationListener(underTest, null, null); + verify(anotherMockBean).addNotificationListener(underTest, null, null); + + GcInfo mockGcInfo = mock(GcInfo.class); + MemoryUsage mockMemoryUsageForTenuredSpace = mock(MemoryUsage.class); + when(mockMemoryUsageForTenuredSpace.getUsed()).thenReturn(1L); + when(mockMemoryUsageForTenuredSpace.getMax()).thenReturn(0L); + MemoryUsage mockMemoryUsageForAnotherTenuredSpace = mock(MemoryUsage.class); + when(mockMemoryUsageForAnotherTenuredSpace.getUsed()).thenReturn(2L); + when(mockMemoryUsageForAnotherTenuredSpace.getMax()).thenReturn(3L); + when(mockGcInfo.getMemoryUsageAfterGc()) + .thenReturn( + ImmutableMap.of( + TENURED_SPACE_NAME, + mockMemoryUsageForTenuredSpace, + anotherTenuredSpaceName, + mockMemoryUsageForAnotherTenuredSpace)); + + Notification notification = + new Notification( + GarbageCollectionNotificationInfo.GARBAGE_COLLECTION_NOTIFICATION, "test", 123); + notification.setUserData( + new GarbageCollectionNotificationInfo("gcName", "gcAction", "non-manual", mockGcInfo) + .toCompositeData(null)); + underTest.handleNotification(notification, null); + + verify(mockHandler) + .handle( + MemoryPressureHandler.Event.newBuilder() + .setWasManualGc(false) + .setTenuredSpaceUsedBytes(2L) + .setTenuredSpaceMaxBytes(3L) + .build()); + } +} \ No newline at end of file
diff --git a/src/test/java/com/google/devtools/build/lib/runtime/RetainedHeapLimiterTest.java b/src/test/java/com/google/devtools/build/lib/runtime/RetainedHeapLimiterTest.java index 1df76ef..d063356 100644 --- a/src/test/java/com/google/devtools/build/lib/runtime/RetainedHeapLimiterTest.java +++ b/src/test/java/com/google/devtools/build/lib/runtime/RetainedHeapLimiterTest.java
@@ -17,28 +17,13 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertThrows; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.never; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyNoMoreInteractions; -import static org.mockito.Mockito.when; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.bugreport.BugReport; import com.google.devtools.build.lib.bugreport.BugReporter; +import com.google.devtools.build.lib.runtime.MemoryPressureHandler.Event; import com.google.devtools.build.lib.server.FailureDetails; import com.google.devtools.build.lib.util.AbruptExitException; -import com.sun.management.GarbageCollectionNotificationInfo; -import com.sun.management.GcInfo; -import java.lang.management.GarbageCollectorMXBean; -import java.lang.management.MemoryUsage; -import javax.management.ListenerNotFoundException; -import javax.management.Notification; -import javax.management.NotificationEmitter; import org.junit.After; -import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -46,66 +31,18 @@ /** Tests for {@link RetainedHeapLimiter}. */ @RunWith(JUnit4.class) public final class RetainedHeapLimiterTest { - - private interface NotificationBean extends GarbageCollectorMXBean, NotificationEmitter {} - - private final NotificationBean mockBean = mock(NotificationBean.class); - private final GarbageCollectorMXBean mockUselessBean = mock(GarbageCollectorMXBean.class); - - @Before - public void initMocks() { - when(mockBean.getMemoryPoolNames()).thenReturn(new String[] {"not tenured", "CMS Old Gen"}); - when(mockUselessBean.getMemoryPoolNames()).thenReturn(new String[] {"assistant", "adjunct"}); - } - @After public void cleanUp() { BugReport.maybePropagateUnprocessedThrowableIfInTest(); } @Test - public void findBeans() { - assertThat( - RetainedHeapLimiter.findTenuredCollectorBeans( - ImmutableList.of(mockUselessBean, mockBean))) - .containsExactly(mockBean); - } - - @Test - public void smoke() throws AbruptExitException, ListenerNotFoundException { - RetainedHeapLimiter underTest = - RetainedHeapLimiter.createFromBeans( - ImmutableList.of(mockUselessBean, mockBean), BugReporter.defaultInstance()); - - underTest.update(100); - verify(mockBean, never()).addNotificationListener(underTest, null, null); - verify(mockBean, never()).removeNotificationListener(underTest, null, null); - - underTest.update(90); - verify(mockBean).addNotificationListener(underTest, null, null); - verify(mockBean, never()).removeNotificationListener(underTest, null, null); - - underTest.update(80); - // No additional calls. - verify(mockBean).addNotificationListener(underTest, null, null); - verify(mockBean, never()).removeNotificationListener(underTest, null, null); - - underTest.update(100); - verify(mockBean).addNotificationListener(underTest, null, null); - verify(mockBean).removeNotificationListener(underTest, null, null); - } - - @Test public void noTenuredSpaceFound() throws AbruptExitException { - RetainedHeapLimiter underTest = - RetainedHeapLimiter.createFromBeans( - ImmutableList.of(mockUselessBean), BugReporter.defaultInstance()); - verify(mockUselessBean, times(2)).getMemoryPoolNames(); + RetainedHeapLimiter underTest = RetainedHeapLimiter.create(BugReporter.defaultInstance()); - underTest.update(100); - verifyNoMoreInteractions(mockUselessBean); - - AbruptExitException e = assertThrows(AbruptExitException.class, () -> underTest.update(80)); + AbruptExitException e = + assertThrows( + AbruptExitException.class, () -> underTest.setThreshold(/*listening=*/ false, 80)); FailureDetails.FailureDetail failureDetail = e.getDetailedExitCode().getFailureDetail(); assertThat(failureDetail.getMessage()) .contains("unable to watch for GC events to exit JVM when 80% of heap is used"); @@ -117,30 +54,26 @@ @Test public void underThreshold_noOom() throws Exception { - RetainedHeapLimiter underTest = - RetainedHeapLimiter.createFromBeans( - ImmutableList.of(mockBean), BugReporter.defaultInstance()); - underTest.update(99); + RetainedHeapLimiter underTest = RetainedHeapLimiter.create(BugReporter.defaultInstance()); - // Triggers GC, and tells RetainedHeapLimiter to OOM if too much memory used next time. - underTest.handleNotification(percentUsedAfterOtherGc(100), null); + underTest.setThreshold(/*listening=*/ true, 99); - underTest.handleNotification(percentUsedAfterForcedGc(89), null); + underTest.handle(percentUsedAfterOtherGc(100)); + underTest.handle(percentUsedAfterForcedGc(89)); } @Test public void overThreshold_oom() throws Exception { - RetainedHeapLimiter underTest = - RetainedHeapLimiter.createFromBeans( - ImmutableList.of(mockBean), BugReporter.defaultInstance()); - underTest.update(90); + RetainedHeapLimiter underTest = RetainedHeapLimiter.create(BugReporter.defaultInstance()); + + underTest.setThreshold(/*listening=*/ true, 90); // Triggers GC, and tells RetainedHeapLimiter to OOM if too much memory used next time. - underTest.handleNotification(percentUsedAfterOtherGc(91), null); + underTest.handle(percentUsedAfterOtherGc(91)); assertThrows( SecurityException.class, // From attempt to halt jvm in test. - () -> underTest.handleNotification(percentUsedAfterForcedGc(91), null)); + () -> underTest.handle(percentUsedAfterForcedGc(91))); OutOfMemoryError oom = assertThrows(OutOfMemoryError.class, BugReport::maybePropagateUnprocessedThrowableIfInTest); @@ -150,78 +83,60 @@ @Test public void externalGcNoTrigger() throws Exception { - RetainedHeapLimiter underTest = - RetainedHeapLimiter.createFromBeans( - ImmutableList.of(mockBean), BugReporter.defaultInstance()); - underTest.update(90); + RetainedHeapLimiter underTest = RetainedHeapLimiter.create(BugReporter.defaultInstance()); + + underTest.setThreshold(/*listening=*/ true, 90); // No trigger because cause was "System.gc()". - underTest.handleNotification(percentUsedAfterForcedGc(91), null); + underTest.handle(percentUsedAfterForcedGc(91)); // Proof: no OOM. - underTest.handleNotification(percentUsedAfterForcedGc(91), null); + underTest.handle(percentUsedAfterForcedGc(91)); } @Test public void triggerReset() throws Exception { - RetainedHeapLimiter underTest = - RetainedHeapLimiter.createFromBeans( - ImmutableList.of(mockBean), BugReporter.defaultInstance()); - underTest.update(90); + RetainedHeapLimiter underTest = RetainedHeapLimiter.create(BugReporter.defaultInstance()); - underTest.handleNotification(percentUsedAfterOtherGc(91), null); + underTest.setThreshold(/*listening=*/ true, 90); + + underTest.handle(percentUsedAfterOtherGc(91)); // Got under the threshold, so no OOM. - underTest.handleNotification(percentUsedAfterForcedGc(89), null); + underTest.handle(percentUsedAfterForcedGc(89)); // No OOM this time since wasn't triggered. - underTest.handleNotification(percentUsedAfterForcedGc(91), null); + underTest.handle(percentUsedAfterForcedGc(91)); } @Test public void triggerRaceWithOtherGc() throws Exception { - RetainedHeapLimiter underTest = - RetainedHeapLimiter.createFromBeans( - ImmutableList.of(mockBean), BugReporter.defaultInstance()); - underTest.update(90); + RetainedHeapLimiter underTest = RetainedHeapLimiter.create(BugReporter.defaultInstance()); - underTest.handleNotification(percentUsedAfterOtherGc(91), null); - underTest.handleNotification(percentUsedAfterOtherGc(91), null); + underTest.setThreshold(/*listening=*/ true, 90); + + underTest.handle(percentUsedAfterOtherGc(91)); + underTest.handle(percentUsedAfterOtherGc(91)); assertThrows( SecurityException.class, // From attempt to halt jvm in test. - () -> underTest.handleNotification(percentUsedAfterForcedGc(91), null)); + () -> underTest.handle(percentUsedAfterForcedGc(91))); assertThrows(OutOfMemoryError.class, BugReport::maybePropagateUnprocessedThrowableIfInTest); } - private static Notification percentUsedAfterForcedGc(int percentUsed) { - return percentUsedAfterGc(percentUsed, "System.gc()"); + private static Event percentUsedAfterForcedGc(int percentUsed) { + return percentUsedAfterGc(/*wasManualGc=*/ true, percentUsed); } - private static Notification percentUsedAfterOtherGc(int percentUsed) { - return percentUsedAfterGc(percentUsed, "other cause"); + private static Event percentUsedAfterOtherGc(int percentUsed) { + return percentUsedAfterGc(/*wasManualGc=*/ false, percentUsed); } - private static Notification percentUsedAfterGc(int percentUsed, String gcCause) { - return percentUsedAfterGc(percentUsed, gcCause, "CMS Old Gen"); - } - - private static Notification percentUsedAfterGc( - int percentUsed, String gcCause, String spaceName) { + private static Event percentUsedAfterGc(boolean wasManualGc, int percentUsed) { checkArgument(percentUsed >= 0 && percentUsed <= 100, percentUsed); - MemoryUsage memoryUsage = mock(MemoryUsage.class); - when(memoryUsage.getUsed()).thenReturn((long) percentUsed); - when(memoryUsage.getMax()).thenReturn(100L); - - GcInfo gcInfo = mock(GcInfo.class); - when(gcInfo.getMemoryUsageAfterGc()).thenReturn(ImmutableMap.of(spaceName, memoryUsage)); - - GarbageCollectionNotificationInfo notificationInfo = - new GarbageCollectionNotificationInfo("name", "action", gcCause, gcInfo); - - Notification notification = - new Notification( - GarbageCollectionNotificationInfo.GARBAGE_COLLECTION_NOTIFICATION, "test", 123); - notification.setUserData(notificationInfo.toCompositeData(null)); - return notification; + return Event.newBuilder() + .setWasManualGc(wasManualGc) + .setTenuredSpaceUsedBytes(percentUsed) + .setTenuredSpaceMaxBytes(100L) + .build(); } }