From d6c5094b9857e77e45a0296658165d1928b17a71 Mon Sep 17 00:00:00 2001 From: Esben Sparre Andreasen Date: Fri, 15 Nov 2024 12:37:04 +0100 Subject: [PATCH 1/4] fixup: add missing await (only improves UI) --- .../src/compare-performance/compare-performance-view.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions/ql-vscode/src/compare-performance/compare-performance-view.ts b/extensions/ql-vscode/src/compare-performance/compare-performance-view.ts index 14e99579e3e..956615538c2 100644 --- a/extensions/ql-vscode/src/compare-performance/compare-performance-view.ts +++ b/extensions/ql-vscode/src/compare-performance/compare-performance-view.ts @@ -65,7 +65,7 @@ export class ComparePerformanceView extends AbstractWebview< const bytes = statSync(log).size; return withProgress( async (progress) => - scanLog(log, new PerformanceOverviewScanner(), progress), + await scanLog(log, new PerformanceOverviewScanner(), progress), { title: `Scanning evaluator log ${logDescription} (${(bytes / 1024 / 1024).toFixed(1)} MB)`, From 27e6b0d6949ad17d8bcc62aedc68366a4bce005f Mon Sep 17 00:00:00 2001 From: Esben Sparre Andreasen Date: Fri, 15 Nov 2024 12:38:39 +0100 Subject: [PATCH 2/4] feat: support mono view of remote logs --- .../src/compare-performance/remote-logs.ts | 43 +++++++++++++++---- .../log-insights/performance-comparison.ts | 2 +- ...omparePerformanceRemoteLogsDescription.tsx | 6 ++- 3 files changed, 39 insertions(+), 12 deletions(-) diff --git a/extensions/ql-vscode/src/compare-performance/remote-logs.ts b/extensions/ql-vscode/src/compare-performance/remote-logs.ts index e4ce6e7de3e..a0663b3264d 100644 --- a/extensions/ql-vscode/src/compare-performance/remote-logs.ts +++ b/extensions/ql-vscode/src/compare-performance/remote-logs.ts @@ -251,7 +251,7 @@ export class RemoteLogs { */ public async downloadAndProcess(): Promise< | { - before: string; + before: string | undefined; after: string; description: ComparePerformanceDescriptionData; } @@ -262,17 +262,26 @@ export class RemoteLogs { void extLogger.log("No targets picked, aborting download"); return undefined; } - const processed = await Promise.all([ - this.downloadAndProcessLogsForTarget(picked.before), + const [processedBefore, processedAfter] = await Promise.all([ + ...(picked.before + ? [this.downloadAndProcessLogsForTarget(picked.before)] + : [undefined]), this.downloadAndProcessLogsForTarget(picked.after), ]); - if (processed.some((d) => typeof d === "undefined")) { - throw new Error("Silently failed to download or process some logs!?"); + if (picked.before && processedBefore === undefined) { + throw new Error( + "Silently failed to download or process the 'before' logs!?", + ); + } + if (processedAfter === undefined) { + throw new Error( + "Silently failed to download or process the 'after' logs!?", + ); } return { - before: processed[0]!, - after: processed[1]!, + before: processedBefore, + after: processedAfter, description: picked.description, }; } @@ -643,7 +652,7 @@ export class RemoteLogs { private async pickTargets(progress?: ProgressCallback): Promise< | { - before: ArtifactDownload; + before?: ArtifactDownload; after: ArtifactDownload; description: ComparePerformanceDescriptionData; } @@ -703,6 +712,7 @@ export class RemoteLogs { step: 4, maxStep: this.PICK_TARGETS_PROGRESS_STEPS, }); + const NONE = "NONE"; const targetChoice2 = await window.showQuickPick( targets .filter( @@ -714,7 +724,8 @@ export class RemoteLogs { t.info.source_id === targetInfoChoice1.info.source_id && t.info.variant_id !== targetInfoChoice1.info.variant_id, ) - .map((t) => t.info.target_id), + .map((t) => t.info.target_id) + .concat(NONE), { title: `Pick target 2`, ignoreFocusOut: true, @@ -726,6 +737,20 @@ export class RemoteLogs { void extLogger.log( `Picked ${experimentChoice} ${targetChoice1} ${targetChoice2}`, ); + if (targetChoice2 === NONE) { + return { + // the convention downstream is that "from" can be optional, but here it is the opposite ... + before: undefined, + after: targetInfoChoice1.downloads["evaluator-logs"], + description: { + kind: "remote-logs", + experimentName: experimentChoice, + fromTarget: undefined, + toTarget: targetChoice1, + info, + }, + }; + } const targetInfoChoice2 = targets.find( (t) => t.info.target_id === targetChoice2, )!; diff --git a/extensions/ql-vscode/src/log-insights/performance-comparison.ts b/extensions/ql-vscode/src/log-insights/performance-comparison.ts index e37b9e02d81..42888caec7b 100644 --- a/extensions/ql-vscode/src/log-insights/performance-comparison.ts +++ b/extensions/ql-vscode/src/log-insights/performance-comparison.ts @@ -62,7 +62,7 @@ export type ComparePerformanceDescriptionData = | { kind: "remote-logs"; experimentName: string; - fromTarget: string; + fromTarget?: string; toTarget: string; info: MinimalDownloadsType; }; diff --git a/extensions/ql-vscode/src/view/common/ComparePerformanceRemoteLogsDescription.tsx b/extensions/ql-vscode/src/view/common/ComparePerformanceRemoteLogsDescription.tsx index e19737a4765..aa7d49dca64 100644 --- a/extensions/ql-vscode/src/view/common/ComparePerformanceRemoteLogsDescription.tsx +++ b/extensions/ql-vscode/src/view/common/ComparePerformanceRemoteLogsDescription.tsx @@ -34,7 +34,7 @@ const TargetRow = ({ info, }: { kind: string; - target: Props["fromTarget"] | Props["toTarget"]; + target: Exclude; info: Props["info"]; }) => { const targetObj = info.targets[target]; @@ -90,7 +90,9 @@ const TargetTable = ({ - + {fromTarget ? ( + + ) : null} From 3b640c60040fee66baa0d9d3e8ab4016109ec3c3 Mon Sep 17 00:00:00 2001 From: Esben Sparre Andreasen Date: Fri, 15 Nov 2024 13:02:57 +0100 Subject: [PATCH 3/4] fix: mono log comparison view start --- extensions/ql-vscode/src/extension.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/extensions/ql-vscode/src/extension.ts b/extensions/ql-vscode/src/extension.ts index 72504e21f7c..6b9be27da9b 100644 --- a/extensions/ql-vscode/src/extension.ts +++ b/extensions/ql-vscode/src/extension.ts @@ -1225,9 +1225,14 @@ async function showPerformanceComparison( const fromLog = from?.evalutorLogPaths?.jsonSummary; const toLog = to.evalutorLogPaths?.jsonSummary; - if (fromLog === undefined || toLog === undefined) { + if (from && fromLog === undefined) { return extLogger.showWarningMessage( - `Cannot compare performance as the structured logs are missing. Did they queries complete normally?`, + `Cannot compare performance as the "from" structured logs are missing. Did they queries complete normally?`, + ); + } + if (toLog === undefined) { + return extLogger.showWarningMessage( + `Cannot compare performance as the "to" structured logs are missing. Did they queries complete normally?`, ); } if (from) { From 44dee31016c2fba87c785c09e230ea7bbb55c192 Mon Sep 17 00:00:00 2001 From: Esben Sparre Andreasen Date: Fri, 15 Nov 2024 13:15:33 +0100 Subject: [PATCH 4/4] fix: correct local run description --- .../common/ComparePerformanceLocalRunDescription.tsx | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/extensions/ql-vscode/src/view/common/ComparePerformanceLocalRunDescription.tsx b/extensions/ql-vscode/src/view/common/ComparePerformanceLocalRunDescription.tsx index dc67b79ad72..142c2f5df91 100644 --- a/extensions/ql-vscode/src/view/common/ComparePerformanceLocalRunDescription.tsx +++ b/extensions/ql-vscode/src/view/common/ComparePerformanceLocalRunDescription.tsx @@ -10,11 +10,13 @@ export const ComparePerformanceLocalRunDescription = ({ }: Props) => { return (
- (fromQuery? - - Comparison of local runs of {fromQuery} and {toQuery} - - : Local run of {toQuery}) + {fromQuery ? ( + + Comparison of local runs of {fromQuery} and {toQuery} + + ) : ( + Local run of {toQuery} + )}
); };