-
Notifications
You must be signed in to change notification settings - Fork 193
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
Add 'compare performance' view #3843
Merged
Merged
Changes from all commits
Commits
Show all changes
47 commits
Select commit
Hold shift + click to select a range
3b06977
Add Compare Performance command (WIP)
asgerf 96aa770
Show evaluation and iteration counts in table
asgerf aaf23ea
compare-perf: Add support for sorting options
tausbn 70ec570
Make RAPrettyPrinter generate JSX fragments
asgerf 8268d68
Apply styling to RA predicate names
asgerf 58afeba
Apply a background color to the pipeline header rows
asgerf 03ca407
Make "..." clickable to reveal abbreviated name
asgerf 317e52c
Also abbreviate RA names in predicate overview
asgerf 876c5b6
Colorize positive/negative deltas
tausbn 260bf0e
Add option to choose metric
asgerf 453aa83
Check for nullness of 'data' in a separate component
asgerf ccf2dc6
Simplify datasets assignment
asgerf 412338c
feat: parallel log scanning
esbena 6d4427e
compare-perf: Add support for selecting a single run as input
tausbn e039f6b
Simplify view when not in comparison mode
tausbn 1d2c2cf
Allow word wrap to break anywhere
asgerf b05ec33
Use useMemo for 'nameSet'
asgerf 20f6e3d
Use useMemo a few places to speed up UI interactions
asgerf 558d957
Reformat code
asgerf 6568b56
Also useMemo the 'total' row computation
asgerf 6f7eb74
Reset hasCacheMismatch when rebuilding 'rows'
asgerf 62f3b4f
Reformat code again
asgerf 9800fa1
Use interface instead of type alias for TRow
asgerf d008963
Refactor OptionalValue
asgerf eec42c5
Split renderAbsoluteValue into renderOptionalValue and renderPredicat…
asgerf 9a0699f
Refactor predicate row into a separate component
asgerf 48954c7
Rename TRow -> Row
asgerf ab00152
Group together rows by fingerprinting
asgerf 4a835b8
Factor out rendering of table body to a memoized component
asgerf 568f082
Fix some crashes when pretty-printing an empty name
asgerf 2cae71c
Add predicate-renaming support
asgerf d37469f
Add 'per evaluation' option next to metric
asgerf 8a58279
Move "total" row to the top and render the metric
asgerf b9d1551
Hide "sort by" dropdown when there is no delta to sort by
asgerf afa3d55
Factor header rows into a component
asgerf c99bf5b
Only warn about cache hits in comparison mode
asgerf aa528c6
Remove unused export
asgerf 6f461e7
Update extensions/ql-vscode/package.json
asgerf 2293cc3
Update extensions/ql-vscode/src/log-insights/log-scanner.ts
asgerf 08bffab
Add more description of the "struct of arrays" layout
asgerf 44d33d6
Merge branch 'asgerf/compare-perf-view' of github.com:asgerf/vscode-c…
asgerf f09210b
Only record cache hits prior to first evaluation
asgerf 37dcd08
Remove TODO that was just resolved
asgerf 370b17c
Remove TODOs
asgerf 8b8d174
Clean up some more TODOs
asgerf bba31c0
Add comment to clarify reverse parsing
asgerf 666c26e
Permit performance comparisons across DBs
asgerf File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
108 changes: 108 additions & 0 deletions
108
extensions/ql-vscode/src/compare-performance/compare-performance-view.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,108 @@ | ||
import { statSync } from "fs"; | ||
import { ViewColumn } from "vscode"; | ||
|
||
import type { App } from "../common/app"; | ||
import { redactableError } from "../common/errors"; | ||
import type { | ||
FromComparePerformanceViewMessage, | ||
ToComparePerformanceViewMessage, | ||
} from "../common/interface-types"; | ||
import type { Logger } from "../common/logging"; | ||
import { showAndLogExceptionWithTelemetry } from "../common/logging"; | ||
import { extLogger } from "../common/logging/vscode"; | ||
import type { WebviewPanelConfig } from "../common/vscode/abstract-webview"; | ||
import { AbstractWebview } from "../common/vscode/abstract-webview"; | ||
import { withProgress } from "../common/vscode/progress"; | ||
import { telemetryListener } from "../common/vscode/telemetry"; | ||
import type { HistoryItemLabelProvider } from "../query-history/history-item-label-provider"; | ||
import { PerformanceOverviewScanner } from "../log-insights/performance-comparison"; | ||
import { scanLog } from "../log-insights/log-scanner"; | ||
import type { ResultsView } from "../local-queries"; | ||
|
||
export class ComparePerformanceView extends AbstractWebview< | ||
ToComparePerformanceViewMessage, | ||
FromComparePerformanceViewMessage | ||
> { | ||
constructor( | ||
app: App, | ||
public logger: Logger, | ||
public labelProvider: HistoryItemLabelProvider, | ||
private resultsView: ResultsView, | ||
) { | ||
super(app); | ||
} | ||
|
||
async showResults(fromJsonLog: string, toJsonLog: string) { | ||
const panel = await this.getPanel(); | ||
panel.reveal(undefined, false); | ||
|
||
// Close the results viewer as it will have opened when the user clicked the query in the history view | ||
// (which they must do as part of the UI interaction for opening the performance view). | ||
// The performance view generally needs a lot of width so it's annoying to have the result viewer open. | ||
this.resultsView.hidePanel(); | ||
|
||
await this.waitForPanelLoaded(); | ||
|
||
function scanLogWithProgress(log: string, logDescription: string) { | ||
const bytes = statSync(log).size; | ||
return withProgress( | ||
async (progress) => | ||
scanLog(log, new PerformanceOverviewScanner(), progress), | ||
|
||
{ | ||
title: `Scanning evaluator log ${logDescription} (${(bytes / 1024 / 1024).toFixed(1)} MB)`, | ||
}, | ||
); | ||
} | ||
|
||
const [fromPerf, toPerf] = await Promise.all([ | ||
fromJsonLog === "" | ||
? new PerformanceOverviewScanner() | ||
: scanLogWithProgress(fromJsonLog, "1/2"), | ||
scanLogWithProgress(toJsonLog, fromJsonLog === "" ? "1/1" : "2/2"), | ||
]); | ||
|
||
await this.postMessage({ | ||
t: "setPerformanceComparison", | ||
from: fromPerf.getData(), | ||
to: toPerf.getData(), | ||
comparison: fromJsonLog !== "", | ||
}); | ||
} | ||
|
||
protected getPanelConfig(): WebviewPanelConfig { | ||
return { | ||
viewId: "comparePerformanceView", | ||
title: "Compare CodeQL Performance", | ||
viewColumn: ViewColumn.Active, | ||
preserveFocus: true, | ||
view: "compare-performance", | ||
}; | ||
} | ||
|
||
protected onPanelDispose(): void {} | ||
|
||
protected async onMessage( | ||
msg: FromComparePerformanceViewMessage, | ||
): Promise<void> { | ||
switch (msg.t) { | ||
case "viewLoaded": | ||
this.onWebViewLoaded(); | ||
break; | ||
|
||
case "telemetry": | ||
telemetryListener?.sendUIInteraction(msg.action); | ||
break; | ||
|
||
case "unhandledError": | ||
void showAndLogExceptionWithTelemetry( | ||
extLogger, | ||
telemetryListener, | ||
redactableError( | ||
msg.error, | ||
)`Unhandled error in performance comparison view: ${msg.error.message}`, | ||
); | ||
break; | ||
} | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you want to remove references to
resultsView
andlabelProvider
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm...not sure what the lifecycle of the view is. Is there a single
view
instance created for the entire extension, or is it one view per window that's opened?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The instance of this class persists across multiple open/close of the panel so I'd say we shouldn't dispose of these fields.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see how
onPanelDispose
is called in the super class. Your implementation here is fine.