Merge pull request #3819 from github/esbena/annotated-performance-view

Hackathon: description on top of performance table

Author: asgerf

extensions/ql-vscode/src/common/dca.ts

@@ -0,0 +1,42 @@
+type VariantId = string;
+type SourceId = string;
+type TargetId = string;
+
+type TargetInfo = {
+  target_id: TargetId;
+  variant_id: VariantId;
+  source_id: SourceId;
+};
+
+type SourceInfo = {
+  source_id: SourceId;
+  repository: string;
+  sha: string;
+};
+
+export type ArtifactDownload = {
+  repository: string;
+  run_id: number;
+  artifact_name: string;
+};
+
+type TargetDownloads = {
+  "evaluator-logs": ArtifactDownload;
+};
+
+export type MinimalDownloadsType = {
+  sources: {
+    [source: SourceId]: { info: SourceInfo };
+  };
+  targets: {
+    [target: string]: {
+      info: TargetInfo;
+      downloads: TargetDownloads;
+    };
+  };
+};
+
+export const dcaControllerRepository = {
+  owner: "github",
+  repo: "codeql-dca-main",
+};

extensions/ql-vscode/src/common/interface-types.ts

@@ -27,7 +27,10 @@ import type {
 } from "./raw-result-types";
 import type { AccessPathSuggestionOptions } from "../model-editor/suggestions";
 import type { ModelEvaluationRunState } from "../model-editor/shared/model-evaluation-run-state";
-import type { PerformanceComparisonDataFromLog } from "../log-insights/performance-comparison";
+import type {
+  ComparePerformanceDescriptionData,
+  PerformanceComparisonDataFromLog,
+} from "../log-insights/performance-comparison";
 
 /**
  * This module contains types and code that are shared between
@@ -398,9 +401,9 @@ export interface SetComparisonsMessage {
 }
 
 export type ToComparePerformanceViewMessage = SetPerformanceComparisonQueries;
-
 export interface SetPerformanceComparisonQueries {
   readonly t: "setPerformanceComparison";
+  readonly description: ComparePerformanceDescriptionData;
   readonly from: PerformanceComparisonDataFromLog;
   readonly to: PerformanceComparisonDataFromLog;
   readonly comparison: boolean;

extensions/ql-vscode/src/compare-performance/compare-performance-view.ts

@@ -17,6 +17,7 @@ import { withProgress } from "../common/vscode/progress";
 import { telemetryListener } from "../common/vscode/telemetry";
 import type { ResultsView } from "../local-queries";
 import { scanLog } from "../log-insights/log-scanner";
+import type { ComparePerformanceDescriptionData } from "../log-insights/performance-comparison";
 import { PerformanceOverviewScanner } from "../log-insights/performance-comparison";
 import type { HistoryItemLabelProvider } from "../query-history/history-item-label-provider";
 import { RemoteLogs } from "./remote-logs";
@@ -45,7 +46,11 @@ export class ComparePerformanceView extends AbstractWebview<
     );
   }
 
-  async showResults(fromJsonLog: string, toJsonLog: string) {
+  async showResults(
+    fromJsonLog: string | undefined,
+    toJsonLog: string,
+    description: ComparePerformanceDescriptionData,
+  ) {
     const panel = await this.getPanel();
     panel.reveal(undefined, false);
 
@@ -69,19 +74,20 @@ export class ComparePerformanceView extends AbstractWebview<
     }
 
     const [fromPerf, toPerf] = await Promise.all([
-      fromJsonLog === ""
-        ? new PerformanceOverviewScanner()
-        : scanLogWithProgress(fromJsonLog, "1/2"),
-      scanLogWithProgress(toJsonLog, fromJsonLog === "" ? "1/1" : "2/2"),
+      fromJsonLog
+        ? scanLogWithProgress(fromJsonLog, "1/2")
+        : new PerformanceOverviewScanner(),
+      scanLogWithProgress(toJsonLog, fromJsonLog ? "2/2" : "1/1"),
     ]);
 
     // TODO: filter out irrelevant common predicates before transfer?
 
     await this.postMessage({
       t: "setPerformanceComparison",
+      description,
       from: fromPerf.getData(),
       to: toPerf.getData(),
-      comparison: fromJsonLog !== "",
+      comparison: !!fromJsonLog,
     });
   }
 
@@ -140,6 +146,6 @@ export class ComparePerformanceView extends AbstractWebview<
       );
       return;
     }
-    await this.showResults(result.before, result.after);
+    await this.showResults(result.before, result.after, result.description);
   }
 }

extensions/ql-vscode/src/compare-performance/remote-logs.ts

@@ -14,47 +14,17 @@ import { basename, dirname, join, relative } from "path";
 import { Uri, window, workspace } from "vscode";
 import type { CodeQLCliServer } from "../codeql-cli/cli";
 import type { App } from "../common/app";
+import type { ArtifactDownload, MinimalDownloadsType } from "../common/dca";
+import { dcaControllerRepository } from "../common/dca";
 import { createTimeoutSignal } from "../common/fetch-stream";
 import { extLogger } from "../common/logging/vscode";
 import type { ProgressCallback } from "../common/vscode/progress";
 import { reportStreamProgress, withProgress } from "../common/vscode/progress";
 import { downloadTimeout, GITHUB_URL } from "../config";
 import { QueryOutputDir } from "../local-queries/query-output-dir";
+import type { ComparePerformanceDescriptionData } from "../log-insights/performance-comparison";
 import { tmpDir } from "../tmp-dir";
 
-type VariantId = string;
-type SourceId = string;
-type TargetId = string;
-
-type TargetInfo = {
-  target_id: TargetId;
-  variant_id: VariantId;
-  source_id: SourceId;
-};
-
-type ArtifactDownload = {
-  repository: string;
-  run_id: number;
-  artifact_name: string;
-};
-
-type TargetDownloads = {
-  "evaluator-logs": ArtifactDownload;
-};
-
-type MinimalDownloadsType = {
-  targets: {
-    [target: string]: {
-      info: TargetInfo;
-      downloads: TargetDownloads;
-    };
-  };
-};
-
-const dcaControllerRepository = {
-  owner: "github",
-  repo: "codeql-dca-main",
-};
 export class RemoteLogs {
   private LOG_DOWNLOAD_AND_PROCESS_PROGRESS_STEPS = 4;
   private PICK_TARGETS_PROGRESS_STEPS = 4;
@@ -283,6 +253,7 @@ export class RemoteLogs {
     | {
         before: string;
         after: string;
+        description: ComparePerformanceDescriptionData;
       }
     | undefined
   > {
@@ -299,7 +270,11 @@ export class RemoteLogs {
     if (processed.some((d) => typeof d === "undefined")) {
       throw new Error("Silently failed to download or process some logs!?");
     }
-    return { before: processed[0]!, after: processed[1]! };
+    return {
+      before: processed[0]!,
+      after: processed[1]!,
+      description: picked.description,
+    };
   }
 
   /**
@@ -489,20 +464,21 @@ export class RemoteLogs {
     };
   }
 
-  private async getPotentialTargetInfos(
-    experimentName: string,
-  ): Promise<Array<MinimalDownloadsType["targets"]["string"]>> {
+  private async getPotentialTargetInfos(experimentName: string): Promise<{
+    targets: Array<MinimalDownloadsType["targets"]["string"]>;
+    info: MinimalDownloadsType;
+  }> {
     const tasksDir = await this.getTasksForExperiment(experimentName);
 
     const downloads = await this.getDownloadsFromTasks(tasksDir);
     void extLogger.log(
       `Found ${Object.keys(downloads.targets).length} potential targets in experiment ${experimentName}`,
     );
-    return Object.values(downloads.targets);
+    return { targets: Object.values(downloads.targets), info: downloads };
   }
 
   /**
-   * Gets the "downloads" metadata from a taksks directory.
+   * Gets the "downloads" metadata from a tasks directory.
    */
   private async getDownloadsFromTasks(
     tasksDir: string,
@@ -669,6 +645,7 @@ export class RemoteLogs {
     | {
         before: ArtifactDownload;
         after: ArtifactDownload;
+        description: ComparePerformanceDescriptionData;
       }
     | undefined
   > {
@@ -696,8 +673,9 @@ export class RemoteLogs {
       step: 2,
       maxStep: this.PICK_TARGETS_PROGRESS_STEPS,
     });
-    const targetInfos = await this.getPotentialTargetInfos(experimentChoice);
-    if (targetInfos.length === 0) {
+    const { targets, info } =
+      await this.getPotentialTargetInfos(experimentChoice);
+    if (targets.length === 0) {
       throw new Error(
         `No targets found in experiment ${experimentChoice}. Is the experiment complete enough yet?`,
       );
@@ -708,7 +686,7 @@ export class RemoteLogs {
       maxStep: this.PICK_TARGETS_PROGRESS_STEPS,
     });
     const targetChoice1 = await window.showQuickPick(
-      targetInfos.map((t) => t.info.target_id),
+      targets.map((t) => t.info.target_id),
       {
         title: `Pick target 1`,
         ignoreFocusOut: true,
@@ -717,7 +695,7 @@ export class RemoteLogs {
     if (!targetChoice1) {
       return undefined;
     }
-    const targetInfoChoice1 = targetInfos.find(
+    const targetInfoChoice1 = targets.find(
       (t) => t.info.target_id === targetChoice1,
     )!;
     progress?.({
@@ -726,7 +704,7 @@ export class RemoteLogs {
       maxStep: this.PICK_TARGETS_PROGRESS_STEPS,
     });
     const targetChoice2 = await window.showQuickPick(
-      targetInfos
+      targets
         .filter(
           (t) =>
             t.info.target_id !== targetChoice1 &&
@@ -748,10 +726,19 @@ export class RemoteLogs {
     void extLogger.log(
       `Picked ${experimentChoice} ${targetChoice1} ${targetChoice2}`,
     );
+    const targetInfoChoice2 = targets.find(
+      (t) => t.info.target_id === targetChoice2,
+    )!;
     return {
       before: targetInfoChoice1.downloads["evaluator-logs"],
-      after: targetInfos.find((t) => t.info.target_id === targetChoice2)!
-        .downloads["evaluator-logs"],
+      after: targetInfoChoice2.downloads["evaluator-logs"],
+      description: {
+        kind: "remote-logs",
+        experimentName: experimentChoice,
+        fromTarget: targetChoice1,
+        toTarget: targetChoice2,
+        info,
+      },
     };
   }
 }