[Skymeld] Adjust DataProviders and SuggestProviders for Skymeld (#…

…117) This change adds support for analysing profiles generated when using Skymeld. For this, the interlaced analysis and execution phase is split into two parts: 1. Analysis only, no execution interlaced. 2. Execution has started. analysis and execution may be interlaced. This is done by checking for the first observed `action processing` event. The distinction is relevant, because it helps us determine how many cores are available on the machine that ran the Bazel client, as well has how many cores are used during execution. The later may be higher than the prior, e.g. when remote execution is used. * Update `SkymeldUsed` to include the combined analysis and execution phase, as well as optionally the phase at which execution likely starts. * Update `EstimatedCoresDataProvider` to estimate cores when Skymeld is used. * Update `CriticalPathNotDominantSuggestionProvider` when Skymeld is used, if the information when execution started is available. Contributes to #97. Signed-off-by: Sara Adams <[email protected]>
EngFlow · Nov 27, 2023 · a4fa1e4 · a4fa1e4
1 parent 7c4feb5
commit a4fa1e4
Show file tree

Hide file tree

Showing 10 changed files with 331 additions and 59 deletions.
diff --git a/...zer/java/com/engflow/bazel/invocation/analyzer/dataproviders/EstimatedCoresAvailable.java b/...zer/java/com/engflow/bazel/invocation/analyzer/dataproviders/EstimatedCoresAvailable.java
@@ -29,7 +29,8 @@ public EstimatedCoresAvailable(Integer estimatedCoresAvailable, Integer gaps) {
 
   @Override
   public String getDescription() {
-    return "The estimated number of cores available, as well as how many numbers were skipped"
-        + " when naming skyframe-evaluators. Extracted from the Bazel profile.";
+    return "The estimated number of cores available on the machine that ran the Bazel client, as"
+        + " well as how many numbers were skipped when naming skyframe-evaluators. Extracted"
+        + " from the Bazel profile.";
   }
 }
diff --git a/.../java/com/engflow/bazel/invocation/analyzer/dataproviders/EstimatedCoresDataProvider.java b/.../java/com/engflow/bazel/invocation/analyzer/dataproviders/EstimatedCoresDataProvider.java
@@ -157,33 +157,30 @@ EstimatedCoresUsed getEstimatedCoresUsed()
    * It assumes the maxValue passed in is the largest value present in the set of values.
    */
   @VisibleForTesting
-  int getGaps(Set<Integer> values, Integer maxValue) {
+  static int getGaps(Set<Integer> values, Integer maxValue) {
     return (maxValue + 1) - values.size();
   }
 
   private synchronized void determineEstimatedCoresAvailable()
       throws InvalidProfileException, MissingInputException, NullDatumException {
     if (evaluateAndDependenciesPhaseSkyframeEvaluators == null) {
       BazelProfile bazelProfile = getDataManager().getDatum(BazelProfile.class);
+      SkymeldUsed skymeldUsed = getDataManager().getDatum(SkymeldUsed.class);
       BazelPhaseDescriptions bazelPhaseDescriptions =
           getDataManager().getDatum(BazelPhaseDescriptions.class);
-      // Evaluate only events within the target pattern evaluation and dependency analysis
-      // phases. These phases should use as many cores as there are available, irrespective of
-      // whether the Bazel flag `--jobs` is set or not.
-      Optional<BazelPhaseDescription> start =
-          bazelPhaseDescriptions.has(BazelProfilePhase.TARGET_PATTERN_EVAL)
-              ? bazelPhaseDescriptions.get(BazelProfilePhase.TARGET_PATTERN_EVAL)
-              : bazelPhaseDescriptions.get(BazelProfilePhase.ANALYZE);
-      Optional<BazelPhaseDescription> end =
-          bazelPhaseDescriptions.has(BazelProfilePhase.ANALYZE)
-              ? bazelPhaseDescriptions.get(BazelProfilePhase.ANALYZE)
-              : bazelPhaseDescriptions.get(BazelProfilePhase.TARGET_PATTERN_EVAL);
-      if (start.isEmpty() || end.isEmpty()) {
+
+      Optional<BazelPhaseDescription> evalAndAnalysisPhase =
+          getEvalAndAnalysisPhase(skymeldUsed, bazelPhaseDescriptions);
+      if (evalAndAnalysisPhase.isEmpty()) {
         // The profile does not include that data necessary.
         return;
       }
+
       evaluateAndDependenciesPhaseSkyframeEvaluators =
-          getSkyframeEvaluators(bazelProfile, start.get().getStart(), end.get().getEnd());
+          getSkyframeEvaluators(
+              bazelProfile,
+              evalAndAnalysisPhase.get().getStart(),
+              evalAndAnalysisPhase.get().getEnd());
       evaluateAndDependenciesPhaseSkyframeEvaluatorsMaxValue =
           evaluateAndDependenciesPhaseSkyframeEvaluators.stream()
               .max(Integer::compareTo)
@@ -195,24 +192,27 @@ private synchronized void determineEstimatedCoresUsed()
       throws InvalidProfileException, MissingInputException, NullDatumException {
     if (executionPhaseSkyframeEvaluators == null) {
       BazelProfile bazelProfile = getDataManager().getDatum(BazelProfile.class);
+      SkymeldUsed skymeldUsed = getDataManager().getDatum(SkymeldUsed.class);
       BazelPhaseDescriptions bazelPhaseDescriptions =
           getDataManager().getDatum(BazelPhaseDescriptions.class);
-      // Evaluate only threads with events in the execution phase, as the Bazel flag `--jobs`
-      // applies to that phase specifically.
-      Optional<BazelPhaseDescription> execution =
-          bazelPhaseDescriptions.get(BazelProfilePhase.EXECUTE);
-      if (execution.isEmpty()) {
+
+      Optional<BazelPhaseDescription> executionPhase =
+          getExecutionPhase(skymeldUsed, bazelPhaseDescriptions);
+      if (executionPhase.isEmpty()) {
+        // The profile does not include that data necessary.
         return;
       }
+
       executionPhaseSkyframeEvaluators =
-          getSkyframeEvaluators(bazelProfile, execution.get().getStart(), execution.get().getEnd());
+          getSkyframeEvaluators(
+              bazelProfile, executionPhase.get().getStart(), executionPhase.get().getEnd());
       executionPhaseSkyframeEvaluatorsMaxValue =
           executionPhaseSkyframeEvaluators.stream().max(Integer::compareTo).orElse(null);
     }
   }
 
   private static Set<Integer> getSkyframeEvaluators(
-      BazelProfile bazelProfile, Timestamp start, Timestamp end) throws InvalidProfileException {
+      BazelProfile bazelProfile, Timestamp start, Timestamp end) {
     Set<Integer> result = new HashSet<>();
     bazelProfile
         .getThreads()
@@ -237,4 +237,56 @@ private static Set<Integer> getSkyframeEvaluators(
         .forEach(x -> result.add(x));
     return result;
   }
+
+  @VisibleForTesting
+  static Optional<BazelPhaseDescription> getEvalAndAnalysisPhase(
+      SkymeldUsed skymeldUsed, BazelPhaseDescriptions bazelPhaseDescriptions) {
+    Timestamp start = null;
+    Timestamp end = null;
+
+    if (skymeldUsed.isSkymeldUsed()) {
+      // Start evaluating events within the target pattern evaluation phase, if present.
+      // Continue evaluating until the interleaved analysis and execution phase has a first
+      // event that suggests action execution.
+      // Within this period, only as many cores should be used as there are available,
+      // irrespective of whether the Bazel flag `--jobs` is set or not.
+      Optional<BazelPhaseDescription> phaseStart =
+          bazelPhaseDescriptions.has(BazelProfilePhase.TARGET_PATTERN_EVAL)
+              ? bazelPhaseDescriptions.get(BazelProfilePhase.TARGET_PATTERN_EVAL)
+              : bazelPhaseDescriptions.get(BazelProfilePhase.ANALYZE_AND_EXECUTE);
+      if (phaseStart.isPresent() && skymeldUsed.getExecutionPhase().isPresent()) {
+        start = phaseStart.get().getStart();
+        end = skymeldUsed.getExecutionPhase().get().getStart();
+      }
+    } else {
+      // Evaluate only events within the target pattern evaluation and dependency analysis
+      // phases. These phases should use as many cores as there are available, irrespective of
+      // whether the Bazel flag `--jobs` is set or not.
+      Optional<BazelPhaseDescription> phaseStart =
+          bazelPhaseDescriptions.has(BazelProfilePhase.TARGET_PATTERN_EVAL)
+              ? bazelPhaseDescriptions.get(BazelProfilePhase.TARGET_PATTERN_EVAL)
+              : bazelPhaseDescriptions.get(BazelProfilePhase.ANALYZE);
+      Optional<BazelPhaseDescription> phaseEnd =
+          bazelPhaseDescriptions.has(BazelProfilePhase.ANALYZE)
+              ? bazelPhaseDescriptions.get(BazelProfilePhase.ANALYZE)
+              : bazelPhaseDescriptions.get(BazelProfilePhase.TARGET_PATTERN_EVAL);
+      if (phaseStart.isPresent() || phaseEnd.isPresent()) {
+        start = phaseStart.get().getStart();
+        end = phaseEnd.get().getEnd();
+      }
+    }
+
+    if (start == null || end == null) {
+      return Optional.empty();
+    }
+    return Optional.of(new BazelPhaseDescription(start, end));
+  }
+
+  @VisibleForTesting
+  static Optional<BazelPhaseDescription> getExecutionPhase(
+      SkymeldUsed skymeldUsed, BazelPhaseDescriptions bazelPhaseDescriptions) {
+    return skymeldUsed.isSkymeldUsed()
+        ? skymeldUsed.getExecutionPhase()
+        : bazelPhaseDescriptions.get(BazelProfilePhase.EXECUTE);
+  }
 }
diff --git a/analyzer/java/com/engflow/bazel/invocation/analyzer/dataproviders/EstimatedCoresUsed.java b/analyzer/java/com/engflow/bazel/invocation/analyzer/dataproviders/EstimatedCoresUsed.java
@@ -29,7 +29,7 @@ public EstimatedCoresUsed(Integer estimatedCoresUsed, Integer gaps) {
 
   @Override
   public String getDescription() {
-    return "The estimated number of cores used, as well as how many numbers were skipped when"
-        + " naming skyframe-evaluators. Extracted from the Bazel profile.";
+    return "The estimated number of cores used during execution, as well as how many numbers were"
+        + " skipped when naming skyframe-evaluators. Extracted from the Bazel profile.";
   }
 }
diff --git a/analyzer/java/com/engflow/bazel/invocation/analyzer/dataproviders/SkymeldUsed.java b/analyzer/java/com/engflow/bazel/invocation/analyzer/dataproviders/SkymeldUsed.java
@@ -15,6 +15,9 @@
 package com.engflow.bazel.invocation.analyzer.dataproviders;
 
 import com.engflow.bazel.invocation.analyzer.core.Datum;
+import com.engflow.bazel.invocation.analyzer.time.Timestamp;
+import com.google.common.base.Preconditions;
+import java.util.Optional;
 
 /**
  * Whether evidence was found that Skymeld was used.
@@ -23,15 +26,47 @@
  */
 public class SkymeldUsed implements Datum {
   private final boolean skymeldUsed;
+  private final Optional<BazelPhaseDescription> analysisAndExecutionPhase;
+  private final Optional<BazelPhaseDescription> executionPhase;
 
-  public SkymeldUsed(boolean skymeldUsed) {
+  public SkymeldUsed() {
+    this(false, Optional.empty(), Optional.empty());
+  }
+
+  public SkymeldUsed(
+      BazelPhaseDescription analysisAndExecutionPhase, Optional<Timestamp> executionStart) {
+    this(true, Optional.of(analysisAndExecutionPhase), executionStart);
+  }
+
+  private SkymeldUsed(
+      boolean skymeldUsed,
+      Optional<BazelPhaseDescription> analysisAndExecutionPhase,
+      Optional<Timestamp> executionStart) {
     this.skymeldUsed = skymeldUsed;
+    this.analysisAndExecutionPhase = Preconditions.checkNotNull(analysisAndExecutionPhase);
+    if (skymeldUsed && executionStart.isPresent()) {
+      Preconditions.checkArgument(analysisAndExecutionPhase.isPresent());
+      this.executionPhase =
+          Optional.of(
+              new BazelPhaseDescription(
+                  executionStart.get(), analysisAndExecutionPhase.get().getEnd()));
+    } else {
+      this.executionPhase = Optional.empty();
+    }
   }
 
   public boolean isSkymeldUsed() {
     return skymeldUsed;
   }
 
+  public Optional<BazelPhaseDescription> getAnalysisAndExecutionPhase() {
+    return analysisAndExecutionPhase;
+  }
+
+  public Optional<BazelPhaseDescription> getExecutionPhase() {
+    return executionPhase;
+  }
+
   @Override
   public boolean isEmpty() {
     return false;
@@ -49,6 +84,13 @@ public String getDescription() {
 
   @Override
   public String getSummary() {
-    return String.valueOf(skymeldUsed);
+    if (skymeldUsed) {
+      return String.format(
+          "%b (analysis started: %s, execution started: %s)",
+          true,
+          analysisAndExecutionPhase.get().getStart(),
+          executionPhase.isPresent() ? executionPhase.get().getStart() : "n/a");
+    }
+    return String.valueOf(false);
   }
 }
diff --git a/...zer/java/com/engflow/bazel/invocation/analyzer/dataproviders/SkymeldUsedDataProvider.java b/...zer/java/com/engflow/bazel/invocation/analyzer/dataproviders/SkymeldUsedDataProvider.java
@@ -14,13 +14,16 @@
 
 package com.engflow.bazel.invocation.analyzer.dataproviders;
 
+import com.engflow.bazel.invocation.analyzer.bazelprofile.BazelProfile;
+import com.engflow.bazel.invocation.analyzer.bazelprofile.BazelProfileConstants;
 import com.engflow.bazel.invocation.analyzer.bazelprofile.BazelProfilePhase;
 import com.engflow.bazel.invocation.analyzer.core.DataProvider;
 import com.engflow.bazel.invocation.analyzer.core.DatumSupplier;
 import com.engflow.bazel.invocation.analyzer.core.DatumSupplierSpecification;
 import com.engflow.bazel.invocation.analyzer.core.InvalidProfileException;
 import com.engflow.bazel.invocation.analyzer.core.MissingInputException;
 import com.engflow.bazel.invocation.analyzer.core.NullDatumException;
+import com.engflow.bazel.invocation.analyzer.time.Timestamp;
 import com.google.common.annotations.VisibleForTesting;
 import java.util.List;
 
@@ -31,7 +34,6 @@
  * @see <a href="https://github.com/bazelbuild/bazel/issues/14057">Project Skymeld GitHub issue</a>
  */
 public class SkymeldUsedDataProvider extends DataProvider {
-
   @Override
   public List<DatumSupplierSpecification<?>> getSuppliers() {
     return List.of(
@@ -44,7 +46,20 @@ SkymeldUsed getSkymeldUsed()
       throws InvalidProfileException, MissingInputException, NullDatumException {
     BazelPhaseDescriptions bazelPhaseDescriptions =
         getDataManager().getDatum(BazelPhaseDescriptions.class);
-    return new SkymeldUsed(
-        bazelPhaseDescriptions.get(BazelProfilePhase.ANALYZE_AND_EXECUTE).isPresent());
+    var interleavedAnalysisAndExecutionPhase =
+        bazelPhaseDescriptions.get(BazelProfilePhase.ANALYZE_AND_EXECUTE);
+    if (!interleavedAnalysisAndExecutionPhase.isPresent()) {
+      return new SkymeldUsed();
+    }
+    BazelProfile bazelProfile = getDataManager().getDatum(BazelProfile.class);
+    var firstActionProcessing =
+        bazelProfile
+            .getThreads()
+            // Find the first action processing event.
+            .flatMap(thread -> thread.getCompleteEvents().stream())
+            .filter(event -> BazelProfileConstants.CAT_ACTION_PROCESSING.equals(event.category))
+            .map(event -> event.start)
+            .min(Timestamp::compareTo);
+    return new SkymeldUsed(interleavedAnalysisAndExecutionPhase.get(), firstActionProcessing);
   }
 }
diff --git a/...el/invocation/analyzer/suggestionproviders/CriticalPathNotDominantSuggestionProvider.java b/...el/invocation/analyzer/suggestionproviders/CriticalPathNotDominantSuggestionProvider.java
@@ -21,13 +21,16 @@
 import com.engflow.bazel.invocation.analyzer.SuggestionOutput;
 import com.engflow.bazel.invocation.analyzer.bazelprofile.BazelProfilePhase;
 import com.engflow.bazel.invocation.analyzer.core.DataManager;
+import com.engflow.bazel.invocation.analyzer.core.InvalidProfileException;
 import com.engflow.bazel.invocation.analyzer.core.MissingInputException;
+import com.engflow.bazel.invocation.analyzer.core.NullDatumException;
 import com.engflow.bazel.invocation.analyzer.core.SuggestionProvider;
 import com.engflow.bazel.invocation.analyzer.dataproviders.BazelPhaseDescription;
 import com.engflow.bazel.invocation.analyzer.dataproviders.BazelPhaseDescriptions;
 import com.engflow.bazel.invocation.analyzer.dataproviders.CriticalPathDuration;
 import com.engflow.bazel.invocation.analyzer.dataproviders.EstimatedCoresUsed;
 import com.engflow.bazel.invocation.analyzer.dataproviders.EstimatedJobsFlagValue;
+import com.engflow.bazel.invocation.analyzer.dataproviders.SkymeldUsed;
 import com.engflow.bazel.invocation.analyzer.dataproviders.TotalDuration;
 import com.engflow.bazel.invocation.analyzer.dataproviders.remoteexecution.RemoteExecutionUsed;
 import com.engflow.bazel.invocation.analyzer.time.DurationUtil;
@@ -59,10 +62,8 @@ public class CriticalPathNotDominantSuggestionProvider extends SuggestionProvide
   @Override
   public SuggestionOutput getSuggestions(DataManager dataManager) {
     try {
-      BazelPhaseDescriptions phases = dataManager.getDatum(BazelPhaseDescriptions.class);
-      Optional<BazelPhaseDescription> optionalExecutionPhaseDescription =
-          phases.get(BazelProfilePhase.EXECUTE);
-      if (optionalExecutionPhaseDescription.isEmpty()) {
+      Optional<BazelPhaseDescription> optionalExecutionPhase = getExecutionPhase(dataManager);
+      if (optionalExecutionPhase.isEmpty()) {
         // No execution phase found, so critical path analysis not applicable.
         return SuggestionProviderUtil.createSuggestionOutputForEmptyInput(
             ANALYZER_CLASSNAME,
@@ -71,7 +72,7 @@ public SuggestionOutput getSuggestions(DataManager dataManager) {
                 + " is necessary for the analysis.");
       }
 
-      Duration executionDuration = optionalExecutionPhaseDescription.get().getDuration();
+      Duration executionDuration = optionalExecutionPhase.get().getDuration();
       if (executionDuration.compareTo(MIN_DURATION_FOR_EVALUATION) < 0) {
         Caveat caveat =
             SuggestionProviderUtil.createCaveat(
@@ -239,4 +240,17 @@ public SuggestionOutput getSuggestions(DataManager dataManager) {
       return SuggestionProviderUtil.createSuggestionOutputForFailure(ANALYZER_CLASSNAME, t);
     }
   }
+
+  @VisibleForTesting
+  static Optional<BazelPhaseDescription> getExecutionPhase(DataManager dataManager)
+      throws InvalidProfileException, MissingInputException, NullDatumException {
+    BazelPhaseDescriptions phases = dataManager.getDatum(BazelPhaseDescriptions.class);
+    SkymeldUsed skymeldUsed = dataManager.getDatum((SkymeldUsed.class));
+
+    if (skymeldUsed.isSkymeldUsed() && skymeldUsed.getExecutionPhase().isPresent()) {
+      return skymeldUsed.getExecutionPhase();
+    } else {
+      return phases.get(BazelProfilePhase.EXECUTE);
+    }
+  }
 }