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

preview suggestions before applying #76

Merged
merged 3 commits into from
Jan 29, 2024
Merged
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
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
# Change Log

## 0.0.8

- fix loading of results when hotspot detection was not executed
- preview suggestion patches before applying the suggestion

## 0.0.7

- automatically find installed DiscoPoP and HotspotDetection tools
Expand Down
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ The following table lists which discopop version is expected by a given version

| Extension Version | DiscoPoP Version |
| ----------------- | ---------------- |
| 0.0.8 | 3.2.0 |
| 0.0.7 | 3.2.0 |
| 0.0.6 | 3.2.0 |
| 0.0.5 | 3.0.0 |
Expand All @@ -30,4 +31,4 @@ The following table lists which discopop version is expected by a given version
## Known Issues

- CMake Configurations: Running the same tool multiple times creates some issues, e.g the same suggestions appearing multiple times. It usually helps to delete the build directory. A proposed fix is scheduled for DiscoPoP 4.0.0.
- Updating from 0.0.5 to 0.0.6 crashes the extension if Script Configurations have been previously created, as the new version does not support them. We recommend to delete script configurations before updating the extension.
- Updating from 0.0.5 to newer versions crashes the extension if Script Configurations have been previously created, as all newer versions do not support them. We recommend to delete script configurations before updating the extension.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"description": "An extension for the Parallelism Discovery tool DiscoPoP.",
"author": "TU Darmstadt - Laboratory for Parallel Programming",
"icon": "media/discopop_icon.png",
"version": "0.0.7",
"version": "0.0.8",
"publisher": "TUDarmstadt-LaboratoryforParallelProgramming",
"repository": {
"type": "git",
Expand Down
183 changes: 144 additions & 39 deletions src/DiscoPoPExtension.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import * as vscode from 'vscode'
import * as fs from 'fs'
import {
Configuration,
RunCapableConfiguration,
Expand Down Expand Up @@ -32,6 +33,7 @@ import { OptimizerWorkflow } from './runners/workflows/OptimizerWorkflow'
import { OptimizerWorkflowUI } from './runners/workflows/OptimizerWorkflowUI'
import { DiscoPoPConfigProvider } from './runners/tools/DiscoPoPConfigProvider'
import { CommandExecution } from './runners/helpers/CommandExecution'
import path = require('path')

function logAndShowErrorMessageHandler(error: any, optionalMessage?: string) {
if (optionalMessage) {
Expand Down Expand Up @@ -560,12 +562,26 @@ export class DiscoPoPExtension {
const returnCode = await dpTools.discopopPatchApplicator
.patchClear()
.catch(logAndShowErrorMessageHandler)
if (returnCode === 3) {
UIPrompts.showMessageForSeconds(
'Nothing to rollback, trivially successful'
)
this.codeLensProvider?.stopWaitingForAppliedStatus()
this.codeLensProvider?.stopWaitingForLineMapping()
switch (returnCode) {
case 0:
UIPrompts.showMessageForSeconds(
'Successfully rolled back all suggestions'
)
break
case 3:
UIPrompts.showMessageForSeconds(
'Nothing to rollback, trivially successful'
)
this.codeLensProvider?.stopWaitingForAppliedStatus()
this.codeLensProvider?.stopWaitingForLineMapping()
break
default:
UIPrompts.showMessageForSeconds(
'Failed to rollback all suggestions. Error code: ' +
returnCode
)
this.codeLensProvider?.stopWaitingForAppliedStatus()
this.codeLensProvider?.stopWaitingForLineMapping()
}
}
)
Expand Down Expand Up @@ -609,29 +625,7 @@ export class DiscoPoPExtension {
})
}

try {
const dpTools = new ToolSuite(dotDiscoPoP)
this.codeLensProvider?.wait()
await dpTools.discopopPatchApplicator.patchApply(
suggestion.id
)
} catch (err) {
if (err instanceof Error) {
vscode.window.showErrorMessage(err.message)
} else {
vscode.window.showErrorMessage(
'Failed to apply suggestion.'
)
}
console.error('FAILED TO APPLY PATCH:')
console.error(err)
console.error(suggestion)
}

// TODO before inserting, preview the changes and request confirmation
// --> we could peek the patch file as a preview https://github.com/microsoft/vscode/blob/8434c86e5665341c753b00c10425a01db4fb8580/src/vs/editor/contrib/gotoSymbol/goToCommands.ts#L760
// --> we should also set the detail view to the suggestion that is being applied
// --> if hotspot results are available for the loop/function, we should also show them in the detail view
this._applySuggestionConfirmed(dotDiscoPoP, suggestion)
}
)
)
Expand All @@ -641,18 +635,9 @@ export class DiscoPoPExtension {
vscode.commands.registerCommand(
Commands.applySingleSuggestion,
async (suggestionNode: DiscoPoPSuggestionNode) => {
this.codeLensProvider?.wait()
const suggestion = suggestionNode.suggestion
const dotDiscoPoP = this.dpResults.dotDiscoPoP
const dpTools = new ToolSuite(dotDiscoPoP)
dpTools.discopopPatchApplicator
.patchApply(suggestion.id)
.catch((error) => {
logAndShowErrorMessageHandler(
error,
`Failed to apply suggestion ${suggestionNode.suggestion.id}: `
)
})
this._applySuggestionConfirmed(dotDiscoPoP, suggestion)
}
)
)
Expand All @@ -673,6 +658,8 @@ export class DiscoPoPExtension {
error,
`Failed to rollback suggestion ${suggestionNode.suggestion.id}: `
)
this.codeLensProvider?.stopWaitingForAppliedStatus()
this.codeLensProvider?.stopWaitingForLineMapping()
})
}
)
Expand Down Expand Up @@ -768,6 +755,124 @@ export class DiscoPoPExtension {
)
}

private async _applySuggestionConfirmed(
dotDiscoPoP: string,
suggestion: Suggestion
) {
// TODO before inserting, preview the changes and request confirmation
// --> we could peek the patch file as a preview https://github.com/microsoft/vscode/blob/8434c86e5665341c753b00c10425a01db4fb8580/src/vs/editor/contrib/gotoSymbol/goToCommands.ts#L760
// --> we should also set the detail view to the suggestion that is being applied
// --> if hotspot results are available for the loop/function, we should also show them in the detail view

// show the relevant patch files in split editors to the right
// never open a patch file twice
// patch files are located in .discopop/patch_generator/<suggestion_id>/fileId.patch
const patchFileUris = fs
.readdirSync(
path.join(dotDiscoPoP, 'patch_generator', `${suggestion.id}`)
)
.map((patchFile) => {
return vscode.Uri.file(
path.join(
dotDiscoPoP,
'patch_generator',
`${suggestion.id}`,
patchFile
)
)
})

// // determine viewColumn (current view column + 1)
// let viewColumn =
// vscode.window.activeTextEditor?.viewColumn || vscode.ViewColumn.One
// viewColumn = (viewColumn + 1) % 9 // mod 9 just to be sure, window.activeTextEditor should however never go above 3

// for (let i = 0; i < patchFileUris.length; i++) {
// const uri = patchFileUris[i]

// // skip opening this patch file if it is already open
// for (const editor of vscode.window.visibleTextEditors) {
// if (editor.document.uri.toString() === uri.toString()) {
// continue
// }
// }

// const document = await vscode.workspace.openTextDocument(uri)
// const editor = await vscode.window.showTextDocument(document, {
// viewColumn: viewColumn,
// preserveFocus: true,
// preview: false,
// })
// editor.revealRange(new vscode.Range(0, 0, 0, 0))
// }

// this is the definition of editor.action.peekLocations:
// CommandsRegistry.registerCommand({
// id: 'editor.action.peekLocations',
// description: {
// description: 'Peek locations from a position in a file',
// args: [
// { name: 'uri', description: 'The text document in which to start', constraint: URI },
// { name: 'position', description: 'The position at which to start', constraint: corePosition.Position.isIPosition },
// { name: 'locations', description: 'An array of locations.', constraint: Array },
// { name: 'multiple', description: 'Define what to do when having multiple results, either `peek`, `gotoAndPeek`, or `goto' },
// ]
// },
// handler: async (accessor: ServicesAccessor, resource: any, position: any, references: any, multiple?: any) => {
// accessor.get(ICommandService).executeCommand('editor.action.goToLocations', resource, position, references, multiple, undefined, true);
// }
// });

const startUri = vscode.Uri.file(
this.dpResults.fileMapping.getFilePath(suggestion.fileId)
)
const startPosition = new vscode.Position(
suggestion.getMappedStartLine(this.dpResults.lineMapping),
0
)

const locations = patchFileUris.map((uri) => {
return new vscode.Location(uri, new vscode.Position(0, 0))
})
const multiple = 'peek'

vscode.commands.executeCommand(
'editor.action.peekLocations',
startUri,
startPosition,
locations,
multiple
)

if (
await UIPrompts.actionConfirmed(
'Do you want to apply this suggestion?'
)
) {
const dpTools = new ToolSuite(dotDiscoPoP)
this.codeLensProvider?.wait()
const returnCode = await dpTools.discopopPatchApplicator.patchApply(
suggestion.id
)
switch (returnCode) {
case 0:
UIPrompts.showMessageForSeconds(
'Successfully applied suggestion ' + suggestion.id
)
break
default:
UIPrompts.showMessageForSeconds(
'Failed to apply suggestion ' +
suggestion.id +
'. Error code: ' +
returnCode
)
this.codeLensProvider?.stopWaitingForAppliedStatus()
this.codeLensProvider?.stopWaitingForLineMapping()
}
}
}

private _updateTreeViewTitleAndMessage() {
if (this.suggestionTreeView && this.suggestionTree && this.dpResults) {
const visible = this.suggestionTree.countVisible
Expand Down
8 changes: 8 additions & 0 deletions src/discoPoP/classes/Suggestion/Suggestion.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,18 @@ export abstract class Suggestion {
) {}

public getMappedStartLine(lineMapping: LineMapping): number {
if (!lineMapping) {
console.error('lineMapping is undefined, using original startLine')
return this.startLine
}
return lineMapping.getMappedLineNr(this.fileId, this.startLine)
}

public getMappedEndLine(lineMapping: LineMapping): number {
if (!lineMapping) {
console.error('lineMapping is undefined, using original endLine')
return this.startLine
}
return lineMapping.getMappedLineNr(this.fileId, this.endLine)
}

Expand Down