Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

compare-perf: Add support for selecting a single run as input #3821

Draft
wants to merge 1 commit into
base: hackathon/compare-perf
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,11 @@ export class ComparePerformanceView extends AbstractWebview<
await this.waitForPanelLoaded();

// TODO: try processing in (async) parallel once readJsonl is streaming
const fromPerf = await scanLog(
fromJsonLog,
new PerformanceOverviewScanner(),
);
const fromPerf =
fromJsonLog === ""
? new PerformanceOverviewScanner()
: await scanLog(fromJsonLog, new PerformanceOverviewScanner());

const toPerf = await scanLog(toJsonLog, new PerformanceOverviewScanner());

// TODO: filter out irrelevant common predicates before transfer?
Expand Down
15 changes: 10 additions & 5 deletions extensions/ql-vscode/src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -927,7 +927,7 @@ async function activateWithInstalledDistribution(
): Promise<void> => showResultsForComparison(compareView, from, to),
async (
from: CompletedLocalQueryInfo,
to: CompletedLocalQueryInfo,
to: CompletedLocalQueryInfo | undefined,
): Promise<void> =>
showPerformanceComparison(comparePerformanceView, from, to),
);
Expand Down Expand Up @@ -1210,17 +1210,22 @@ async function showResultsForComparison(
async function showPerformanceComparison(
view: ComparePerformanceView,
from: CompletedLocalQueryInfo,
to: CompletedLocalQueryInfo,
to: CompletedLocalQueryInfo | undefined,
): Promise<void> {
const fromLog = from.evalutorLogPaths?.jsonSummary;
const toLog = to.evalutorLogPaths?.jsonSummary;
let fromLog = from.evalutorLogPaths?.jsonSummary;
let toLog = to?.evalutorLogPaths?.jsonSummary;

if (to === undefined) {
toLog = fromLog;
fromLog = "";
}
if (fromLog === undefined || toLog === undefined) {
return extLogger.showWarningMessage(
`Cannot compare performance as the structured logs are missing. Did they queries complete normally?`,
);
}
await extLogger.log(
`Comparing performance of ${from.getQueryName()} and ${to.getQueryName()}`,
`Comparing performance of ${from.getQueryName()} and ${to?.getQueryName() ?? "baseline"}`,
);

await view.showResults(fromLog, toLog);
Expand Down
66 changes: 61 additions & 5 deletions extensions/ql-vscode/src/query-history/query-history-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ export class QueryHistoryManager extends DisposableObject {
) => Promise<void>,
private readonly doComparePerformanceCallback: (
from: CompletedLocalQueryInfo,
to: CompletedLocalQueryInfo,
to: CompletedLocalQueryInfo | undefined,
) => Promise<void>,
) {
super();
Expand Down Expand Up @@ -706,17 +706,18 @@ export class QueryHistoryManager extends DisposableObject {

let toItem: CompletedLocalQueryInfo | undefined = undefined;
try {
toItem = await this.findOtherQueryToCompare(fromItem, multiSelect);
toItem = await this.findOtherQueryToComparePerformance(
fromItem,
multiSelect,
);
} catch (e) {
void showAndLogErrorMessage(
this.app.logger,
`Failed to compare queries: ${getErrorMessage(e)}`,
);
}

if (toItem !== undefined) {
await this.doComparePerformanceCallback(fromItem, toItem);
}
await this.doComparePerformanceCallback(fromItem, toItem);
}

async handleItemClicked(item: QueryHistoryInfo) {
Expand Down Expand Up @@ -1116,6 +1117,7 @@ export class QueryHistoryManager extends DisposableObject {
detail: item.completedQuery.message,
query: item,
}));

if (comparableQueryLabels.length < 1) {
throw new Error("No other queries available to compare with.");
}
Expand All @@ -1124,6 +1126,60 @@ export class QueryHistoryManager extends DisposableObject {
return choice?.query;
}

private async findOtherQueryToComparePerformance(
fromItem: CompletedLocalQueryInfo,
allSelectedItems: CompletedLocalQueryInfo[],
): Promise<CompletedLocalQueryInfo | undefined> {
const dbName = fromItem.databaseName;

// If exactly 2 items are selected, return the one that
// isn't being used as the "from" item.
if (allSelectedItems.length === 2) {
const otherItem =
fromItem === allSelectedItems[0]
? allSelectedItems[1]
: allSelectedItems[0];
if (otherItem.databaseName !== dbName) {
throw new Error("Query databases must be the same.");
}
return otherItem;
}

if (allSelectedItems.length > 2) {
throw new Error("Please select no more than 2 queries.");
}

// Otherwise, present a dialog so the user can choose the item they want to use.
const comparableQueryLabels = this.treeDataProvider.allHistory
.filter(this.isSuccessfulCompletedLocalQueryInfo)
.filter(
(otherItem) =>
otherItem !== fromItem && otherItem.databaseName === dbName,
)
.map((item) => ({
label: this.labelProvider.getLabel(item),
description: item.databaseName,
detail: item.completedQuery.message,
query: item,
}));
const comparableQueryLabelsWithDefault = [
{
label: "Single run",
description:
"Look at the performance of this run, compared to a trivial baseline",
detail: undefined,
query: undefined,
},
...comparableQueryLabels,
];
if (comparableQueryLabelsWithDefault.length < 1) {
throw new Error("No other queries available to compare with.");
}
const choice = await window.showQuickPick(comparableQueryLabelsWithDefault);

return choice?.query;
}

/**
* Updates the compare with source query. This ensures that all compare command invocations
* when exactly 2 queries are selected always have the proper _from_ query. Always use
Expand Down
Loading