diff --git a/analyzer/java/com/engflow/bazel/invocation/analyzer/dataproviders/GarbageCollectionStats.java b/analyzer/java/com/engflow/bazel/invocation/analyzer/dataproviders/GarbageCollectionStats.java index bda5b28..b443a69 100644 --- a/analyzer/java/com/engflow/bazel/invocation/analyzer/dataproviders/GarbageCollectionStats.java +++ b/analyzer/java/com/engflow/bazel/invocation/analyzer/dataproviders/GarbageCollectionStats.java @@ -16,7 +16,11 @@ import com.engflow.bazel.invocation.analyzer.core.Datum; import com.engflow.bazel.invocation.analyzer.time.DurationUtil; +import com.google.common.base.Preconditions; +import com.google.common.base.Strings; import java.time.Duration; +import java.util.Optional; +import javax.annotation.Nullable; /** * Data on the garbage collection performed during the invocation. Major garbage collection suspends @@ -25,28 +29,36 @@ * an invocation's performance. */ public class GarbageCollectionStats implements Datum { - private final Duration majorGarbageCollectionDuration; + private final Optional majorGarbageCollectionDuration; + @Nullable private final String emptyReason; public GarbageCollectionStats(Duration majorGarbageCollectionDuration) { - this.majorGarbageCollectionDuration = majorGarbageCollectionDuration; + this.majorGarbageCollectionDuration = Optional.of(majorGarbageCollectionDuration); + this.emptyReason = null; + } + + public GarbageCollectionStats(String emptyReason) { + Preconditions.checkArgument(!Strings.isNullOrEmpty(emptyReason)); + this.majorGarbageCollectionDuration = Optional.empty(); + this.emptyReason = emptyReason; } public boolean hasMajorGarbageCollection() { - return !majorGarbageCollectionDuration.isZero(); + return !isEmpty() && !majorGarbageCollectionDuration.get().isZero(); } - public Duration getMajorGarbageCollectionDuration() { + public Optional getMajorGarbageCollectionDuration() { return majorGarbageCollectionDuration; } @Override public boolean isEmpty() { - return false; + return majorGarbageCollectionDuration.isEmpty(); } @Override public String getEmptyReason() { - return null; + return emptyReason; } @Override @@ -56,9 +68,13 @@ public String getDescription() { @Override public String getSummary() { + if (isEmpty()) { + return null; + } return hasMajorGarbageCollection() ? String.format( - "Major GC duration of %s", DurationUtil.formatDuration(majorGarbageCollectionDuration)) + "Major GC duration of %s", + DurationUtil.formatDuration(majorGarbageCollectionDuration.get())) : "No major GC occurred"; } } diff --git a/analyzer/java/com/engflow/bazel/invocation/analyzer/dataproviders/GarbageCollectionStatsDataProvider.java b/analyzer/java/com/engflow/bazel/invocation/analyzer/dataproviders/GarbageCollectionStatsDataProvider.java index 170c5fe..f3109b7 100644 --- a/analyzer/java/com/engflow/bazel/invocation/analyzer/dataproviders/GarbageCollectionStatsDataProvider.java +++ b/analyzer/java/com/engflow/bazel/invocation/analyzer/dataproviders/GarbageCollectionStatsDataProvider.java @@ -42,7 +42,8 @@ public GarbageCollectionStats getGarbageCollectionStats() BazelProfile bazelProfile = getDataManager().getDatum(BazelProfile.class); Optional garbageCollectorThread = bazelProfile.getGarbageCollectorThread(); if (garbageCollectorThread.isEmpty()) { - throw new InvalidProfileException("Unable to find garbage collector thread."); + return new GarbageCollectionStats( + "Failed to find a garbage collector thread in the Bazel profile."); } Duration majorGarbageCollection = garbageCollectorThread.get().getCompleteEvents().stream() diff --git a/analyzer/java/com/engflow/bazel/invocation/analyzer/suggestionproviders/GarbageCollectionSuggestionProvider.java b/analyzer/java/com/engflow/bazel/invocation/analyzer/suggestionproviders/GarbageCollectionSuggestionProvider.java index 21ff105..a21083d 100644 --- a/analyzer/java/com/engflow/bazel/invocation/analyzer/suggestionproviders/GarbageCollectionSuggestionProvider.java +++ b/analyzer/java/com/engflow/bazel/invocation/analyzer/suggestionproviders/GarbageCollectionSuggestionProvider.java @@ -51,19 +51,21 @@ public SuggestionOutput getSuggestions(DataManager dataManager) { List suggestions = new ArrayList<>(); GarbageCollectionStats gcStats = dataManager.getDatum(GarbageCollectionStats.class); + if (gcStats.isEmpty()) { + return SuggestionProviderUtil.createSuggestionOutputForEmptyInput( + ANALYZER_CLASSNAME, EMPTY_REASON_PREFIX + gcStats.getEmptyReason()); + } if (gcStats.hasMajorGarbageCollection()) { + Duration majorGcDuration = gcStats.getMajorGarbageCollectionDuration().get(); TotalDuration totalDurationDatum = dataManager.getDatum(TotalDuration.class); if (totalDurationDatum.isEmpty()) { return SuggestionProviderUtil.createSuggestionOutputForEmptyInput( ANALYZER_CLASSNAME, EMPTY_REASON_PREFIX + totalDurationDatum.getEmptyReason()); } Duration totalDuration = totalDurationDatum.getTotalDuration().get(); - double percentOfTotal = - DurationUtil.getPercentageOf( - gcStats.getMajorGarbageCollectionDuration(), totalDuration); + double percentOfTotal = DurationUtil.getPercentageOf(majorGcDuration, totalDuration); if (percentOfTotal >= MAJOR_GC_MIN_PERCENTAGE) { - Duration optimalDuration = - totalDuration.minus(gcStats.getMajorGarbageCollectionDuration()); + Duration optimalDuration = totalDuration.minus(majorGcDuration); PotentialImprovement potentialImprovement = SuggestionProviderUtil.createPotentialImprovement( String.format( @@ -78,7 +80,7 @@ public SuggestionOutput getSuggestions(DataManager dataManager) { Locale.US, "%s or %.2f%% of the invocation is spent on major garbage collection which" + " suspends all other threads.", - DurationUtil.formatDuration(gcStats.getMajorGarbageCollectionDuration()), + DurationUtil.formatDuration(majorGcDuration), percentOfTotal); String titleIncreaseHeapSize = "Increase the Java heap size available to Bazel"; diff --git a/analyzer/javatests/com/engflow/bazel/invocation/analyzer/dataproviders/GarbageCollectionStatsDataProviderTest.java b/analyzer/javatests/com/engflow/bazel/invocation/analyzer/dataproviders/GarbageCollectionStatsDataProviderTest.java index 773600d..35d7ef9 100644 --- a/analyzer/javatests/com/engflow/bazel/invocation/analyzer/dataproviders/GarbageCollectionStatsDataProviderTest.java +++ b/analyzer/javatests/com/engflow/bazel/invocation/analyzer/dataproviders/GarbageCollectionStatsDataProviderTest.java @@ -22,11 +22,9 @@ import static com.engflow.bazel.invocation.analyzer.WriteBazelProfile.thread; import static com.engflow.bazel.invocation.analyzer.WriteBazelProfile.trace; import static com.google.common.truth.Truth.assertThat; -import static org.junit.Assert.assertThrows; import com.engflow.bazel.invocation.analyzer.bazelprofile.BazelProfileConstants; import com.engflow.bazel.invocation.analyzer.core.DuplicateProviderException; -import com.engflow.bazel.invocation.analyzer.core.InvalidProfileException; import com.engflow.bazel.invocation.analyzer.time.TimeUtil; import com.engflow.bazel.invocation.analyzer.time.Timestamp; import java.time.Duration; @@ -66,8 +64,9 @@ public void shouldReturnMajorGarbageCollection() throws Exception { singleGcDuration)))))); GarbageCollectionStats gcStats = provider.getGarbageCollectionStats(); + assertThat(gcStats.isEmpty()).isFalse(); assertThat(gcStats.hasMajorGarbageCollection()).isTrue(); - assertThat(gcStats.getMajorGarbageCollectionDuration()) + assertThat(gcStats.getMajorGarbageCollectionDuration().get()) .isEqualTo(singleGcDuration.multipliedBy(4)); } @@ -93,14 +92,18 @@ public void shouldReturnNoMajorGarbageCollection() throws Exception { singleGcDuration)))))); GarbageCollectionStats gcStats = provider.getGarbageCollectionStats(); + assertThat(gcStats.isEmpty()).isFalse(); assertThat(gcStats.hasMajorGarbageCollection()).isFalse(); - assertThat(gcStats.getMajorGarbageCollectionDuration()).isEqualTo(Duration.ZERO); + assertThat(gcStats.getMajorGarbageCollectionDuration().get()).isEqualTo(Duration.ZERO); } @Test - public void shouldThrowWhenNoMajorGarbageCollectorThreadIsPresent() throws Exception { + public void shouldReturnEmptyGarbageCollectionStatsWhenNoGarbageCollectorThreadIsPresent() + throws Exception { useProfile(metaData(), trace(mainThread())); - assertThrows(InvalidProfileException.class, () -> provider.getGarbageCollectionStats()); + GarbageCollectionStats gcStats = provider.getGarbageCollectionStats(); + assertThat(gcStats.isEmpty()).isTrue(); + assertThat(gcStats.hasMajorGarbageCollection()).isFalse(); } } diff --git a/analyzer/javatests/com/engflow/bazel/invocation/analyzer/suggestionproviders/GarbageCollectionSuggestionProviderTest.java b/analyzer/javatests/com/engflow/bazel/invocation/analyzer/suggestionproviders/GarbageCollectionSuggestionProviderTest.java index 69a0e38..16f0bbe 100644 --- a/analyzer/javatests/com/engflow/bazel/invocation/analyzer/suggestionproviders/GarbageCollectionSuggestionProviderTest.java +++ b/analyzer/javatests/com/engflow/bazel/invocation/analyzer/suggestionproviders/GarbageCollectionSuggestionProviderTest.java @@ -49,9 +49,27 @@ public void setup() throws Exception { suggestionProvider = new GarbageCollectionSuggestionProvider(); } + @Test + public void shouldNotReturnSuggestionForEmptyGarbageCollectionStats() { + String emptyReason = "The GC stats are empty!"; + garbageCollectionStats = new GarbageCollectionStats(emptyReason); + + SuggestionOutput suggestionOutput = suggestionProvider.getSuggestions(dataManager); + + assertThat(suggestionOutput.getAnalyzerClassname()) + .isEqualTo(GarbageCollectionSuggestionProvider.class.getName()); + assertThat(suggestionOutput.getSuggestionList()).isEmpty(); + assertThat(suggestionOutput.hasFailure()).isFalse(); + assertThat(suggestionOutput.getCaveatList()).hasSize(1); + assertThat(suggestionOutput.getCaveat(0).getMessage()) + .contains(GarbageCollectionSuggestionProvider.EMPTY_REASON_PREFIX); + assertThat(suggestionOutput.getCaveat(0).getMessage()).contains(emptyReason); + } + @Test public void shouldNotReturnSuggestionForEmptyTotalDuration() { - totalDuration = new TotalDuration("empty"); + String emptyReason = "The total duration is empty!"; + totalDuration = new TotalDuration(emptyReason); SuggestionOutput suggestionOutput = suggestionProvider.getSuggestions(dataManager); @@ -62,7 +80,7 @@ public void shouldNotReturnSuggestionForEmptyTotalDuration() { assertThat(suggestionOutput.getCaveatList()).hasSize(1); assertThat(suggestionOutput.getCaveat(0).getMessage()) .contains(GarbageCollectionSuggestionProvider.EMPTY_REASON_PREFIX); - assertThat(suggestionOutput.getCaveat(0).getMessage()).contains("empty"); + assertThat(suggestionOutput.getCaveat(0).getMessage()).contains(emptyReason); } @Test @@ -101,7 +119,7 @@ public void shouldReturnSuggestionsForMajorGarbageCollection() { Locale.US, "%s or %.2f%% of the invocation is spent on major garbage collection", DurationUtil.formatDuration( - garbageCollectionStats.getMajorGarbageCollectionDuration()), + garbageCollectionStats.getMajorGarbageCollectionDuration().get()), percent)); Suggestion rules = suggestionOutput.getSuggestionList().get(1);