diff --git a/analyzer/java/com/engflow/bazel/invocation/analyzer/suggestionproviders/InvestigateRemoteCacheMissesSuggestionProvider.java b/analyzer/java/com/engflow/bazel/invocation/analyzer/suggestionproviders/InvestigateRemoteCacheMissesSuggestionProvider.java new file mode 100644 index 0000000..013519b --- /dev/null +++ b/analyzer/java/com/engflow/bazel/invocation/analyzer/suggestionproviders/InvestigateRemoteCacheMissesSuggestionProvider.java @@ -0,0 +1,131 @@ +/* + * Copyright 2023 EngFlow Inc. + * + * 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.engflow.bazel.invocation.analyzer.suggestionproviders; + +import static com.engflow.bazel.invocation.analyzer.time.DurationUtil.formatDuration; + +import com.engflow.bazel.invocation.analyzer.Caveat; +import com.engflow.bazel.invocation.analyzer.SuggestionCategory; +import com.engflow.bazel.invocation.analyzer.SuggestionOutput; +import com.engflow.bazel.invocation.analyzer.core.DataManager; +import com.engflow.bazel.invocation.analyzer.core.MissingInputException; +import com.engflow.bazel.invocation.analyzer.core.SuggestionProvider; +import com.engflow.bazel.invocation.analyzer.dataproviders.LocalActions; +import com.engflow.bazel.invocation.analyzer.dataproviders.remoteexecution.RemoteCachingUsed; +import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Strings; +import java.util.ArrayList; +import java.util.List; +import java.util.stream.Collectors; + +/** A {@link SuggestionProvider} that suggests investigating cache misses. */ +public class InvestigateRemoteCacheMissesSuggestionProvider extends SuggestionProviderBase { + private static final String ANALYZER_CLASSNAME = + InvestigateRemoteCacheMissesSuggestionProvider.class.getName(); + private static final String EMPTY_REASON_PREFIX = "No remote cache misses could be highlighted. "; + + private static final String INVESTIGATE_REMOTE_CACHE_MISSES = "InvestigateRemoteCacheMisses"; + + public static InvestigateRemoteCacheMissesSuggestionProvider createDefault() { + return new InvestigateRemoteCacheMissesSuggestionProvider(5); + } + + public static InvestigateRemoteCacheMissesSuggestionProvider createVerbose() { + return new InvestigateRemoteCacheMissesSuggestionProvider(Integer.MAX_VALUE); + } + + /** The maximum number of cache miss actions to list in a suggestion. */ + private final int maxActions; + + @VisibleForTesting + InvestigateRemoteCacheMissesSuggestionProvider(int maxActions) { + this.maxActions = maxActions; + } + + @Override + public SuggestionOutput getSuggestions(DataManager dataManager) { + try { + RemoteCachingUsed remoteCachingUsed = dataManager.getDatum(RemoteCachingUsed.class); + if (!remoteCachingUsed.isRemoteCachingUsed()) { + return SuggestionProviderUtil.createSuggestionOutputForEmptyInput( + ANALYZER_CLASSNAME, EMPTY_REASON_PREFIX + "Remote caching wasn't used."); + } + LocalActions localActions = dataManager.getDatum(LocalActions.class); + var cacheMisses = + localActions.stream() + .filter(action -> !action.isRemoteCacheHit()) + .sorted((a, b) -> b.getAction().duration.compareTo(a.getAction().duration)) + .collect(Collectors.toList()); + if (cacheMisses.isEmpty()) { + return SuggestionProviderUtil.createSuggestionOutputForEmptyInput( + ANALYZER_CLASSNAME, EMPTY_REASON_PREFIX + "No cache misses were found."); + } + + String title = "Investigate remote cache misses"; + StringBuilder recommendation = new StringBuilder(); + recommendation.append( + "The following actions with cache misses took the longest to execute:\n"); + cacheMisses.stream() + .limit(maxActions) + .forEachOrdered( + action -> { + var completeAction = action.getAction(); + recommendation.append("\t- Action: \""); + recommendation.append(completeAction.name); + recommendation.append("\"\n"); + // TODO: Use constant once #122 is merged. + var forTarget = completeAction.args.get("target"); + if (!Strings.isNullOrEmpty(forTarget)) { + recommendation.append("\t\tTarget: \""); + recommendation.append(forTarget); + recommendation.append("\"\n"); + } + recommendation.append("\t\tAction duration: "); + recommendation.append(formatDuration(completeAction.duration)); + recommendation.append("\n"); + }); + recommendation.append( + "Check https://bazel.build/remote/cache-remote#troubleshooting-cache-hits to learn more" + + " about how to debug remote cache misses. Increasing the cache hit rate can" + + " significantly speed up builds."); + var caveats = new ArrayList(); + if (cacheMisses.size() > maxActions) { + caveats.add( + SuggestionProviderUtil.createCaveat( + String.format( + "Only the first %d of %d cache misses are listed.", + maxActions, cacheMisses.size()), + true)); + } + // TODO: Once #122 is merged, add caveat if target name is not present and suggest using + // `--experimental_profile_include_target_label`. + var suggestion = + SuggestionProviderUtil.createSuggestion( + SuggestionCategory.OTHER, + createSuggestionId(INVESTIGATE_REMOTE_CACHE_MISSES), + title, + recommendation.toString(), + null, + null, + caveats); + return SuggestionProviderUtil.createSuggestionOutput( + ANALYZER_CLASSNAME, List.of(suggestion), null); + } catch (MissingInputException e) { + return SuggestionProviderUtil.createSuggestionOutputForMissingInput(ANALYZER_CLASSNAME, e); + } catch (Throwable t) { + return SuggestionProviderUtil.createSuggestionOutputForFailure(ANALYZER_CLASSNAME, t); + } + } +} diff --git a/analyzer/java/com/engflow/bazel/invocation/analyzer/suggestionproviders/SuggestionProviderUtil.java b/analyzer/java/com/engflow/bazel/invocation/analyzer/suggestionproviders/SuggestionProviderUtil.java index 1c6a0a0..6246e3c 100644 --- a/analyzer/java/com/engflow/bazel/invocation/analyzer/suggestionproviders/SuggestionProviderUtil.java +++ b/analyzer/java/com/engflow/bazel/invocation/analyzer/suggestionproviders/SuggestionProviderUtil.java @@ -48,6 +48,9 @@ public static List getAllSuggestionProviders(boolean verbose verbose ? LocalActionsWithRemoteExecutionSuggestionProvider.createVerbose() : LocalActionsWithRemoteExecutionSuggestionProvider.createDefault(), + verbose + ? InvestigateRemoteCacheMissesSuggestionProvider.createVerbose() + : InvestigateRemoteCacheMissesSuggestionProvider.createDefault(), new BuildWithoutTheBytesSuggestionProvider(), new CriticalPathNotDominantSuggestionProvider(), new GarbageCollectionSuggestionProvider(), diff --git a/analyzer/javatests/com/engflow/bazel/invocation/analyzer/suggestionproviders/InvestigateRemoteCacheMissesSuggestionProviderTest.java b/analyzer/javatests/com/engflow/bazel/invocation/analyzer/suggestionproviders/InvestigateRemoteCacheMissesSuggestionProviderTest.java new file mode 100644 index 0000000..900cb52 --- /dev/null +++ b/analyzer/javatests/com/engflow/bazel/invocation/analyzer/suggestionproviders/InvestigateRemoteCacheMissesSuggestionProviderTest.java @@ -0,0 +1,155 @@ +/* + * Copyright 2023 EngFlow Inc. + * + * 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.engflow.bazel.invocation.analyzer.suggestionproviders; + +import static com.engflow.bazel.invocation.analyzer.bazelprofile.BazelProfileConstants.CAT_GENERAL_INFORMATION; +import static com.engflow.bazel.invocation.analyzer.bazelprofile.BazelProfileConstants.CAT_REMOTE_ACTION_CACHE_CHECK; +import static com.engflow.bazel.invocation.analyzer.bazelprofile.BazelProfileConstants.CAT_REMOTE_ACTION_EXECUTION; +import static com.engflow.bazel.invocation.analyzer.bazelprofile.BazelProfileConstants.CAT_REMOTE_OUTPUT_DOWNLOAD; +import static com.engflow.bazel.invocation.analyzer.bazelprofile.BazelProfileConstants.COMPLETE_SUBPROCESS_RUN; +import static com.google.common.truth.Truth.assertThat; +import static org.mockito.Mockito.when; + +import com.engflow.bazel.invocation.analyzer.EventThreadBuilder; +import com.engflow.bazel.invocation.analyzer.SuggestionOutput; +import com.engflow.bazel.invocation.analyzer.dataproviders.LocalActions; +import com.engflow.bazel.invocation.analyzer.dataproviders.remoteexecution.RemoteCachingUsed; +import java.util.List; +import org.junit.Before; +import org.junit.Test; + +public class InvestigateRemoteCacheMissesSuggestionProviderTest + extends SuggestionProviderUnitTestBase { + // These variables are returned from calls to DataManager.getDatum for the associated types. They + // are set up with reasonable defaults before each test is run, but can be overridden within the + // tests when custom values are desired for the testing being conducted (without the need to + // re-initialize the mocking). + private RemoteCachingUsed remoteCachingUsed; + private LocalActions localActions; + + @Before + public void setup() throws Exception { + // Create reasonable defaults and set up to return the class-variables when the associated types + // are requested. + when(dataManager.getDatum(RemoteCachingUsed.class)).thenAnswer(i -> remoteCachingUsed); + when(dataManager.getDatum(LocalActions.class)).thenAnswer(i -> localActions); + + suggestionProvider = InvestigateRemoteCacheMissesSuggestionProvider.createDefault(); + } + + @Test + public void shouldNotReturnSuggestionIfNoRemoteCachingIsUsed() { + remoteCachingUsed = new RemoteCachingUsed(false); + + SuggestionOutput suggestionOutput = suggestionProvider.getSuggestions(dataManager); + + assertThat(suggestionOutput.getSuggestionList()).isEmpty(); + assertThat(suggestionOutput.hasFailure()).isFalse(); + } + + @Test + public void shouldNotReturnSuggestionWithoutCacheMisses() { + remoteCachingUsed = new RemoteCachingUsed(true); + localActions = LocalActions.create(List.of()); + + SuggestionOutput suggestionOutput = suggestionProvider.getSuggestions(dataManager); + + assertThat(suggestionOutput.getSuggestionList()).isEmpty(); + assertThat(suggestionOutput.hasFailure()).isFalse(); + } + + @Test + public void shouldReturnSuggestionIfThereAreCacheMisses() { + remoteCachingUsed = new RemoteCachingUsed(true); + var thread = new EventThreadBuilder(1, 1); + var cacheMissWithLocalExec = + new LocalActions.LocalAction( + thread.actionProcessingAction("cache miss action with local execution", "a", 10, 10), + List.of( + thread.related(10, 1, CAT_REMOTE_ACTION_CACHE_CHECK), + thread.related(12, 2, CAT_GENERAL_INFORMATION, COMPLETE_SUBPROCESS_RUN))); + var cacheMissWithRemoteExec = + new LocalActions.LocalAction( + thread.actionProcessingAction("cache miss with remote execution", "a", 10, 10), + List.of( + thread.related(10, 1, CAT_REMOTE_ACTION_CACHE_CHECK), + thread.related(12, 2, CAT_REMOTE_ACTION_EXECUTION), + thread.related(14, 1, CAT_REMOTE_OUTPUT_DOWNLOAD))); + var cacheHit = + new LocalActions.LocalAction( + thread.actionProcessingAction("example cache hit action", "a", 10, 10), + List.of( + thread.related(10, 1, CAT_REMOTE_ACTION_CACHE_CHECK), + thread.related(11, 5, CAT_REMOTE_OUTPUT_DOWNLOAD))); + localActions = + LocalActions.create(List.of(cacheMissWithLocalExec, cacheMissWithRemoteExec, cacheHit)); + + SuggestionOutput suggestionOutput = suggestionProvider.getSuggestions(dataManager); + + assertThat(suggestionOutput.getAnalyzerClassname()) + .isEqualTo(InvestigateRemoteCacheMissesSuggestionProvider.class.getName()); + assertThat(suggestionOutput.hasFailure()).isFalse(); + assertThat(suggestionOutput.getSuggestionList().size()).isEqualTo(1); + var suggestion = suggestionOutput.getSuggestion(0); + assertThat(suggestion.getRecommendation()).contains(cacheMissWithLocalExec.getAction().name); + assertThat(suggestion.getRecommendation()).contains(cacheMissWithRemoteExec.getAction().name); + assertThat(suggestion.getRecommendation()).doesNotContain(cacheHit.getAction().name); + assertThat(suggestion.getCaveatList()).isEmpty(); + } + + @Test + public void shouldReturnSuggestionWithCaveatIfThereAreManyCacheMisses() { + remoteCachingUsed = new RemoteCachingUsed(true); + suggestionProvider = new InvestigateRemoteCacheMissesSuggestionProvider(1); + var thread = new EventThreadBuilder(1, 1); + var cacheMissWithLocalExec = + new LocalActions.LocalAction( + thread.actionProcessingAction("cache miss action with local execution", "a", 10, 10), + List.of( + thread.related(10, 1, CAT_REMOTE_ACTION_CACHE_CHECK), + thread.related(12, 2, CAT_GENERAL_INFORMATION, COMPLETE_SUBPROCESS_RUN))); + var cacheMissWithRemoteExec = + new LocalActions.LocalAction( + thread.actionProcessingAction("cache miss with remote execution", "a", 10, 100), + List.of( + thread.related(10, 1, CAT_REMOTE_ACTION_CACHE_CHECK), + thread.related(12, 90, CAT_REMOTE_ACTION_EXECUTION), + thread.related(14, 1, CAT_REMOTE_OUTPUT_DOWNLOAD))); + var cacheHit = + new LocalActions.LocalAction( + thread.actionProcessingAction("example cache hit action", "a", 10, 10), + List.of( + thread.related(10, 1, CAT_REMOTE_ACTION_CACHE_CHECK), + thread.related(11, 5, CAT_REMOTE_OUTPUT_DOWNLOAD))); + localActions = + LocalActions.create(List.of(cacheMissWithLocalExec, cacheMissWithRemoteExec, cacheHit)); + + SuggestionOutput suggestionOutput = suggestionProvider.getSuggestions(dataManager); + + assertThat(suggestionOutput.getAnalyzerClassname()) + .isEqualTo(InvestigateRemoteCacheMissesSuggestionProvider.class.getName()); + assertThat(suggestionOutput.hasFailure()).isFalse(); + assertThat(suggestionOutput.getSuggestionList().size()).isEqualTo(1); + var suggestion = suggestionOutput.getSuggestion(0); + // This is the longest cache miss. It is included. + assertThat(suggestion.getRecommendation()).contains(cacheMissWithRemoteExec.getAction().name); + // This is another cache miss, but it exceeds the max count, so it's not included. + assertThat(suggestion.getRecommendation()) + .doesNotContain(cacheMissWithLocalExec.getAction().name); + assertThat(suggestion.getRecommendation()).doesNotContain(cacheHit.getAction().name); + assertThat(suggestion.getCaveatCount()).isEqualTo(1); + assertThat(suggestion.getCaveat(0).getSuggestVerboseMode()).isTrue(); + } +} diff --git a/analyzer/javatests/com/engflow/bazel/invocation/analyzer/suggestionproviders/SuggestionProvidersTestSuite.java b/analyzer/javatests/com/engflow/bazel/invocation/analyzer/suggestionproviders/SuggestionProvidersTestSuite.java index ec1f997..db448d3 100644 --- a/analyzer/javatests/com/engflow/bazel/invocation/analyzer/suggestionproviders/SuggestionProvidersTestSuite.java +++ b/analyzer/javatests/com/engflow/bazel/invocation/analyzer/suggestionproviders/SuggestionProvidersTestSuite.java @@ -24,6 +24,7 @@ CriticalPathNotDominantSuggestionProviderTest.class, GarbageCollectionSuggestionProviderTest.class, IncompleteProfileSuggestionProviderTest.class, + InvestigateRemoteCacheMissesSuggestionProviderTest.class, JobsSuggestionProviderTest.class, LocalActionsWithRemoteExecutionSuggestionProviderTest.class, MergedEventsSuggestionProviderTest.class,