Skip to content

Commit

Permalink
[SuggestionProvider] Fix bug in `InvestigateRemoteCacheMissesSuggesti…
Browse files Browse the repository at this point in the history
…onProvider` (#136)

Before, actions that did not check the remote cache were potentially
included in the action misses suggestion.

---------

Signed-off-by: Sara Adams <sara.e.adams@gmail.com>
  • Loading branch information
saraadams authored Nov 28, 2023
1 parent 0da07a6 commit a34a52e
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public SuggestionOutput getSuggestions(DataManager dataManager) {
LocalActions localActions = dataManager.getDatum(LocalActions.class);
var cacheMisses =
localActions.stream()
.filter(action -> !action.isRemoteCacheHit())
.filter(action -> action.hasRemoteCacheCheck() && !action.isRemoteCacheHit())
.sorted((a, b) -> b.getAction().duration.compareTo(a.getAction().duration))
.collect(Collectors.toList());
if (cacheMisses.isEmpty()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
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_LOCAL_ACTION_EXECUTION;
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;
Expand Down Expand Up @@ -68,7 +69,19 @@ public void shouldNotReturnSuggestionIfNoRemoteCachingIsUsed() {
@Test
public void shouldNotReturnSuggestionWithoutCacheMisses() {
remoteCachingUsed = new RemoteCachingUsed(true);
localActions = LocalActions.create(List.of());
var thread = new EventThreadBuilder(1, 1);
var actionWithRemoteCacheHit =
new LocalActions.LocalAction(
thread.actionProcessingAction("action with cache hit", "a", 10, 10),
List.of(
thread.related(10, 1, CAT_REMOTE_ACTION_CACHE_CHECK),
thread.related(12, 2, CAT_REMOTE_OUTPUT_DOWNLOAD)));
var actionWithoutRemoteCaching =
new LocalActions.LocalAction(
thread.actionProcessingAction("action without a cache check", "a", 10, 10),
List.of(thread.related(12, 2, CAT_LOCAL_ACTION_EXECUTION)));
localActions =
LocalActions.create(List.of(actionWithRemoteCacheHit, actionWithoutRemoteCaching));

SuggestionOutput suggestionOutput = suggestionProvider.getSuggestions(dataManager);

Expand Down

0 comments on commit a34a52e

Please sign in to comment.