Add peak post heap size to the BEP memory metrics. Note that you have to pass the option --bep_publish_used_heap_size_post_build for this metric to be populated. It is also not set when there was no major GC during this invocation. RELNOTES: None PiperOrigin-RevId: 289610222
diff --git a/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/build_event_stream.proto b/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/build_event_stream.proto index 97b4c70..5d675e9 100644 --- a/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/build_event_stream.proto +++ b/src/main/java/com/google/devtools/build/lib/buildeventstream/proto/build_event_stream.proto
@@ -687,6 +687,10 @@ // --bep_publish_used_heap_size_post_build is set, // since it forces a full GC. int64 used_heap_size_post_build = 1; + + // Size of the peak JVM heap size in bytes post GC. Note that this reports 0 + // if there was no major GC during the build. + int64 peak_post_gc_heap_size = 2; } MemoryMetrics memory_metrics = 2;
diff --git a/src/main/java/com/google/devtools/build/lib/metrics/MetricsCollector.java b/src/main/java/com/google/devtools/build/lib/metrics/MetricsCollector.java index e94483d..d13c5b1 100644 --- a/src/main/java/com/google/devtools/build/lib/metrics/MetricsCollector.java +++ b/src/main/java/com/google/devtools/build/lib/metrics/MetricsCollector.java
@@ -30,6 +30,7 @@ import java.lang.management.ManagementFactory; import java.lang.management.MemoryMXBean; import java.time.Duration; +import java.util.Optional; import java.util.concurrent.atomic.AtomicLong; class MetricsCollector { @@ -97,6 +98,11 @@ System.gc(); MemoryMXBean memBean = ManagementFactory.getMemoryMXBean(); memoryMetrics.setUsedHeapSizePostBuild(memBean.getHeapMemoryUsage().getUsed()); + Optional<Long> peakPostGcHeapSize = + PostGCMemoryUseRecorder.get().getPeakPostGCHeapMemoryUsed(); + if (peakPostGcHeapSize.isPresent()) { + memoryMetrics.setPeakPostGcHeapSize(peakPostGcHeapSize.get()); + } } return memoryMetrics.build(); }
diff --git a/src/main/java/com/google/devtools/build/lib/metrics/PostGCMemoryUseRecorder.java b/src/main/java/com/google/devtools/build/lib/metrics/PostGCMemoryUseRecorder.java index 9a80081..7103749 100644 --- a/src/main/java/com/google/devtools/build/lib/metrics/PostGCMemoryUseRecorder.java +++ b/src/main/java/com/google/devtools/build/lib/metrics/PostGCMemoryUseRecorder.java
@@ -36,6 +36,7 @@ import java.lang.management.ManagementFactory; import java.lang.management.MemoryUsage; import java.util.Map; +import java.util.Optional; import java.util.logging.Logger; import javax.management.Notification; import javax.management.NotificationEmitter; @@ -69,7 +70,7 @@ private static final Logger logger = Logger.getLogger(PostGCMemoryUseRecorder.class.getName()); // Protected by PostGCMemoryUseRecorder's lock. - private long peakPostGCHeapMemoryUsed = 0; + private Optional<Long> peakPostGCHeapMemoryUsed = Optional.empty(); // Protected by PostGcMemoryUseRecorder's lock. Set to true iff a GarbageCollectionNotification // reported that we were using no memory. @@ -87,7 +88,7 @@ } } - public synchronized long getPeakPostGCHeapMemoryUsed() { + public synchronized Optional<Long> getPeakPostGCHeapMemoryUsed() { return peakPostGCHeapMemoryUsed; } @@ -96,12 +97,16 @@ } public synchronized void reset() { - peakPostGCHeapMemoryUsed = 0; + peakPostGCHeapMemoryUsed = Optional.empty(); memoryUsageReportedZero = false; } private synchronized void updatePostGCHeapMemoryUsed(long used) { - peakPostGCHeapMemoryUsed = Math.max(used, peakPostGCHeapMemoryUsed); + if (peakPostGCHeapMemoryUsed.isPresent()) { + peakPostGCHeapMemoryUsed = Optional.of(Math.max(used, peakPostGCHeapMemoryUsed.get())); + } else { + peakPostGCHeapMemoryUsed = Optional.of(used); + } } private synchronized void updateMemoryUsageReportedZero(boolean value) { @@ -179,9 +184,13 @@ @Override public byte[] get(Supplier<BuildConfiguration> configurationSupplier, CommandEnvironment env) { - return print( - StringUtilities.prettyPrintBytes( - PostGCMemoryUseRecorder.get().getPeakPostGCHeapMemoryUsed())); + if (PostGCMemoryUseRecorder.get().getPeakPostGCHeapMemoryUsed().isPresent()) { + return print( + StringUtilities.prettyPrintBytes( + PostGCMemoryUseRecorder.get().getPeakPostGCHeapMemoryUsed().get())); + } else { + return print("unknown"); + } } } @@ -221,7 +230,7 @@ @Override public void afterCommand() { - if (forceGc && PostGCMemoryUseRecorder.get().getPeakPostGCHeapMemoryUsed() == 0L) { + if (forceGc && !PostGCMemoryUseRecorder.get().getPeakPostGCHeapMemoryUsed().isPresent()) { System.gc(); } }
diff --git a/src/test/java/com/google/devtools/build/lib/metrics/BUILD b/src/test/java/com/google/devtools/build/lib/metrics/BUILD index fd003c8..6878c15 100644 --- a/src/test/java/com/google/devtools/build/lib/metrics/BUILD +++ b/src/test/java/com/google/devtools/build/lib/metrics/BUILD
@@ -16,5 +16,6 @@ "//third_party:junit4", "//third_party:mockito", "//third_party:truth", + "//third_party:truth8", ], )
diff --git a/src/test/java/com/google/devtools/build/lib/metrics/PostGCMemoryUseRecorderTest.java b/src/test/java/com/google/devtools/build/lib/metrics/PostGCMemoryUseRecorderTest.java index c3ac5c7..7b67cf0 100644 --- a/src/test/java/com/google/devtools/build/lib/metrics/PostGCMemoryUseRecorderTest.java +++ b/src/test/java/com/google/devtools/build/lib/metrics/PostGCMemoryUseRecorderTest.java
@@ -15,6 +15,7 @@ package com.google.devtools.build.lib.metrics; import static com.google.common.truth.Truth.assertThat; +import static com.google.common.truth.Truth8.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; @@ -130,19 +131,17 @@ } @Test - public void peakHeapStartsAtZero() { - PostGCMemoryUseRecorder rec = - new PostGCMemoryUseRecorder(new ArrayList<GarbageCollectorMXBean>()); - assertThat(rec.getPeakPostGCHeapMemoryUsed()).isEqualTo(0L); + public void peakHeapStartsAbsent() { + PostGCMemoryUseRecorder rec = new PostGCMemoryUseRecorder(new ArrayList<>()); + assertThat(rec.getPeakPostGCHeapMemoryUsed()).isEmpty(); } @Test - public void peakHeapZeroAfterReset() { - PostGCMemoryUseRecorder rec = - new PostGCMemoryUseRecorder(new ArrayList<GarbageCollectorMXBean>()); + public void peakHeapAbsentAfterReset() { + PostGCMemoryUseRecorder rec = new PostGCMemoryUseRecorder(new ArrayList<>()); rec.handleNotification(createMajorGCNotification(1000L), null); rec.reset(); - assertThat(rec.getPeakPostGCHeapMemoryUsed()).isEqualTo(0); + assertThat(rec.getPeakPostGCHeapMemoryUsed()).isEmpty(); } @Test @@ -157,63 +156,58 @@ underTest.doHandleNotification(notificationWithNoGcCause, /*handback=*/ null); - assertThat(underTest.getPeakPostGCHeapMemoryUsed()).isEqualTo(100L); + assertThat(underTest.getPeakPostGCHeapMemoryUsed()).hasValue(100L); } @Test public void peakHeapIncreasesWhenBigger() { - PostGCMemoryUseRecorder rec = - new PostGCMemoryUseRecorder(new ArrayList<GarbageCollectorMXBean>()); + PostGCMemoryUseRecorder rec = new PostGCMemoryUseRecorder(new ArrayList<>()); rec.handleNotification(createMajorGCNotification(1000L), null); - assertThat(rec.getPeakPostGCHeapMemoryUsed()).isEqualTo(1000L); + assertThat(rec.getPeakPostGCHeapMemoryUsed()).hasValue(1000L); rec.handleNotification(createMajorGCNotification(1001L), null); - assertThat(rec.getPeakPostGCHeapMemoryUsed()).isEqualTo(1001L); + assertThat(rec.getPeakPostGCHeapMemoryUsed()).hasValue(1001L); rec.handleNotification(createMajorGCNotification(2001L), null); - assertThat(rec.getPeakPostGCHeapMemoryUsed()).isEqualTo(2001L); + assertThat(rec.getPeakPostGCHeapMemoryUsed()).hasValue(2001L); } @Test public void peakHeapDoesntDecrease() { - PostGCMemoryUseRecorder rec = - new PostGCMemoryUseRecorder(new ArrayList<GarbageCollectorMXBean>()); + PostGCMemoryUseRecorder rec = new PostGCMemoryUseRecorder(new ArrayList<>()); rec.handleNotification(createMajorGCNotification(1000L), null); rec.handleNotification(createMajorGCNotification(500L), null); - assertThat(rec.getPeakPostGCHeapMemoryUsed()).isEqualTo(1000L); + assertThat(rec.getPeakPostGCHeapMemoryUsed()).hasValue(1000L); rec.handleNotification(createMajorGCNotification(999L), null); - assertThat(rec.getPeakPostGCHeapMemoryUsed()).isEqualTo(1000L); + assertThat(rec.getPeakPostGCHeapMemoryUsed()).hasValue(1000L); } @Test public void ignoreNonGCNotification() { - PostGCMemoryUseRecorder rec = - new PostGCMemoryUseRecorder(new ArrayList<GarbageCollectorMXBean>()); + PostGCMemoryUseRecorder rec = new PostGCMemoryUseRecorder(new ArrayList<>()); rec.handleNotification( createMockNotification( "some other notification", "end of major GC", ImmutableMap.of("Foo", 1000L)), null); - assertThat(rec.getPeakPostGCHeapMemoryUsed()).isEqualTo(0L); + assertThat(rec.getPeakPostGCHeapMemoryUsed()).isEmpty(); } @Test public void ignoreNonMajorGCNotification() { - PostGCMemoryUseRecorder rec = - new PostGCMemoryUseRecorder(new ArrayList<GarbageCollectorMXBean>()); + PostGCMemoryUseRecorder rec = new PostGCMemoryUseRecorder(new ArrayList<>()); rec.handleNotification( createMockNotification( GarbageCollectionNotificationInfo.GARBAGE_COLLECTION_NOTIFICATION, "end of minor GC", ImmutableMap.of("Foo", 1000L)), null); - assertThat(rec.getPeakPostGCHeapMemoryUsed()).isEqualTo(0L); + assertThat(rec.getPeakPostGCHeapMemoryUsed()).isEmpty(); } @Test public void sumMemUsageInfo() { - PostGCMemoryUseRecorder rec = - new PostGCMemoryUseRecorder(new ArrayList<GarbageCollectorMXBean>()); + PostGCMemoryUseRecorder rec = new PostGCMemoryUseRecorder(new ArrayList<>()); rec.handleNotification( createMajorGCNotification(ImmutableMap.of("Foo", 111L, "Bar", 222L, "Qux", 333L)), null); - assertThat(rec.getPeakPostGCHeapMemoryUsed()).isEqualTo(666L); + assertThat(rec.getPeakPostGCHeapMemoryUsed()).hasValue(666L); } @Test