Skip to content

Commit

Permalink
[SuggestionProvider] Suggest investigating remote cache misses
Browse files Browse the repository at this point in the history
If remote caching is used and there are remote cache misses, suggest
investigating the misses with a link to Bazel documentation.

Contributes to #90

Signed-off-by: Sara Adams <sara.e.adams@gmail.com>
  • Loading branch information
saraadams committed Nov 27, 2023
1 parent aff8b45 commit 5b8c9ae
Show file tree
Hide file tree
Showing 4 changed files with 290 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -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<Caveat>();
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);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ public static List<SuggestionProvider> getAllSuggestionProviders(boolean verbose
verbose
? LocalActionsWithRemoteExecutionSuggestionProvider.createVerbose()
: LocalActionsWithRemoteExecutionSuggestionProvider.createDefault(),
verbose
? InvestigateRemoteCacheMissesSuggestionProvider.createVerbose()
: InvestigateRemoteCacheMissesSuggestionProvider.createDefault(),
new BuildWithoutTheBytesSuggestionProvider(),
new CriticalPathNotDominantSuggestionProvider(),
new GarbageCollectionSuggestionProvider(),
Expand Down
Original file line number Diff line number Diff line change
@@ -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();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
CriticalPathNotDominantSuggestionProviderTest.class,
GarbageCollectionSuggestionProviderTest.class,
IncompleteProfileSuggestionProviderTest.class,
InvestigateRemoteCacheMissesSuggestionProviderTest.class,
JobsSuggestionProviderTest.class,
LocalActionsWithRemoteExecutionSuggestionProviderTest.class,
MergedEventsSuggestionProviderTest.class,
Expand Down

0 comments on commit 5b8c9ae

Please sign in to comment.