Skip to content

Commit

Permalink
improve code lens provider performance (#11696)
Browse files Browse the repository at this point in the history
* store localized text

* improved telemetry for code lens perf

* don't register code lens provider for notebook or cell documents

* added news

* undo scheme registration filter, filter code folding provider to match
  • Loading branch information
amunger authored Oct 20, 2022
1 parent 0b2b7cd commit 8b8f330
Show file tree
Hide file tree
Showing 11 changed files with 169 additions and 79 deletions.
5 changes: 4 additions & 1 deletion TELEMETRY.csv
Original file line number Diff line number Diff line change
Expand Up @@ -683,7 +683,8 @@ Used to detect the popularity of a mime type, that would help determine which mi
E.g. if we see widget mimetype, then we know how many use ipywidgets and the like and helps us prioritize widget issues,
or prioritize rendering of widgets when opening an existing notebook or the like.","Telemetry.CellOutputMimeType","donjayamanne","N/A","","","when","Whether the package was detected in an existing file (upon open, upon save, upon close) or when it was being used during execution.","","'onExecution'
'onOpenCloseOrSave' ",false
"DS_INTERNAL.CODE_LENS_ACQ_TIME","How long on average we spent parsing code lens. Sent on shutdown.","Telemetry.CodeLensAverageAcquisitionTime","amunger","InteractiveWindow","","","duration","Duration of a measure in milliseconds.
"DS_INTERNAL.CODE_LENS_ACQ_TIME","How long on average we spent parsing code lens. Sent on shutdown.
We should be able to deprecate in favor of DocumentWithCodeCells, but we should compare the numbers first.","Telemetry.CodeLensAverageAcquisitionTime","amunger","InteractiveWindow","","","duration","Duration of a measure in milliseconds.
Common measurement used across a number of events.","number","",false
"DS_INTERNAL.COMMAND_EXECUTED","A command that the extension contributes is executed.","Telemetry.CommandExecuted","amunger","N/A","","","command","Name of the command executed.","string","",false
"DS_INTERNAL.CONNECTFAILEDJUPYTER","Sent when we have failed to connect to the local Jupyter server we started.","Telemetry.ConnectFailedJupyter","donjayamanne","N/A","KernelStartup","","failed","Whether there was a failure.
Expand Down Expand Up @@ -717,6 +718,8 @@ Common to most of the events.","string","",true
Common to most of the events.","string","",true
"DS_INTERNAL.CONNECTREMOTEJUPYTER_VIA_LOCALHOST","Connecting to an existing Jupyter server, but connecting to localhost.","Telemetry.ConnectRemoteJupyterViaLocalHost","donjayamanne","N/A","KernelStartup","","","","","",""
"DS_INTERNAL.CONNECTREMOTESELFCERTFAILEDJUPYTER","Jupyter server's certificate is not from a trusted authority.","Telemetry.ConnectRemoteSelfCertFailedJupyter","donjayamanne","N/A","KernelStartup","","","","","",""
"DS_INTERNAL.DOCUMENT_WITH_CODE_CELLS","Info about code lenses, count and average time to parse the document.","Telemetry.DocumentWithCodeCells","amunger","InteractiveWindow","","","codeLensUpdateTime","Average time taken to aquire code lenses for a document without using the cache","number","",false
"DS_INTERNAL.DOCUMENT_WITH_CODE_CELLS","Info about code lenses, count and average time to parse the document.","Telemetry.DocumentWithCodeCells","amunger","InteractiveWindow","","","maxCellCount","Maximum number of code lenses returned for the document","number","",false
"DS_INTERNAL.FAILED_TO_UPDATE_JUPYTER_KERNEL_SPEC","Sent when we fail to update the kernel spec json file.","Telemetry.FailedToUpdateKernelSpec","donjayamanne","N/A","KernelStartup","","failed","Whether there was a failure.
Common to most of the events.","true","",false
"DS_INTERNAL.FAILED_TO_UPDATE_JUPYTER_KERNEL_SPEC","Sent when we fail to update the kernel spec json file.","Telemetry.FailedToUpdateKernelSpec","donjayamanne","N/A","KernelStartup","","failureCategory","A reason that we generate (e.g. kerneldied, noipykernel, etc), more like a category of the error.
Expand Down
14 changes: 14 additions & 0 deletions TELEMETRY.md
Original file line number Diff line number Diff line change
Expand Up @@ -1698,6 +1698,7 @@ Expand each section to see more information about that event.
Owner: [@amunger](https://github.com/amunger)
```
How long on average we spent parsing code lens. Sent on shutdown.
We should be able to deprecate in favor of DocumentWithCodeCells, but we should compare the numbers first.
```

- Measures:
Expand Down Expand Up @@ -1801,6 +1802,19 @@ Expand each section to see more information about that event.



* DS_INTERNAL.DOCUMENT_WITH_CODE_CELLS (Telemetry.DocumentWithCodeCells)
Owner: [@amunger](https://github.com/amunger)
```
Info about code lenses, count and average time to parse the document.
```

- Measures:
- `codeLensUpdateTime`: `number`
Average time taken to aquire code lenses for a document without using the cache
- `maxCellCount`: `number`
Maximum number of code lenses returned for the document


* DS_INTERNAL.FAILED_TO_UPDATE_JUPYTER_KERNEL_SPEC (Telemetry.FailedToUpdateKernelSpec)
Owner: [@donjayamanne](https://github.com/donjayamanne)
```
Expand Down
1 change: 1 addition & 0 deletions news/3 Code Health/11433.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Reduce the amount of work done for each code lens creation to help performance.
11 changes: 11 additions & 0 deletions src/gdpr.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1080,6 +1080,17 @@
"${include}": [
"${F1}"
]
}
*/
//Telemetry.DocumentWithCodeCells
/* __GDPR__
"DS_INTERNAL.DOCUMENT_WITH_CODE_CELLS" : {
"codeLensUpdateTime": {"classification":"SystemMetaData","purpose":"PerformanceAndHealth","comment":"Average time taken to aquire code lenses for a document without using the cache","owner":"amunger","isMeasurement":true},
"maxCellCount": {"classification":"SystemMetaData","purpose":"FeatureInsight","comment":"Maximum number of code lenses returned for the document","owner":"amunger","isMeasurement":true},
"${include}": [
"${F1}"
]
}
*/
Expand Down
152 changes: 80 additions & 72 deletions src/interactive-window/editor-integration/codeLensFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,20 +11,20 @@ import {
NotebookCellExecutionState,
NotebookCellExecutionStateChangeEvent,
Range,
TextDocument,
workspace
TextDocument
} from 'vscode';

import { IDocumentManager, IVSCodeNotebook, IWorkspaceService } from '../../platform/common/application/types';
import { traceWarning, traceInfoIfCI } from '../../platform/logging';
import { traceWarning, traceInfoIfCI, traceVerbose } from '../../platform/logging';

import { ICellRange, IConfigurationService, IDisposableRegistry, Resource } from '../../platform/common/types';
import * as localize from '../../platform/common/utils/localize';
import { getInteractiveCellMetadata } from '../helpers';
import { IKernelProvider } from '../../kernels/types';
import { CodeLensCommands, Commands, InteractiveWindowView } from '../../platform/common/constants';
import { generateCellRangesFromDocument } from './cellFactory';
import { ICodeLensFactory, IGeneratedCode, IGeneratedCodeStorageFactory } from './types';
import { CodeLensPerfMeasures, ICodeLensFactory, IGeneratedCode, IGeneratedCodeStorageFactory } from './types';
import { StopWatch } from '../../platform/common/utils/stopWatch';

type CodeLensCacheData = {
cachedDocumentVersion: number | undefined;
Expand All @@ -39,6 +39,20 @@ type PerNotebookData = {
documentExecutionCounts: Map<string, number>;
};

// localized code lense text
const runCurrentandAllBelowTitle = localize.DataScience.runCurrentCellAndAddBelow();
const addCellBelowTitle = localize.DataScience.addCellBelowCommandTitle();
const debugCurrentCellTitle = localize.DataScience.debugCellCommandTitle();
const debugCellTitle = localize.DataScience.debugCellCommandTitle();
const debugStepOverTitle = localize.DataScience.debugStepOverCommandTitle();
const DebugContinueTitle = localize.DataScience.debugContinueCommandTitle();
const debugStopTitle = localize.DataScience.debugStopCommandTitle();
const runCellTitle = localize.DataScience.runCellLensCommandTitle();
const runAllCellsTitle = localize.DataScience.runAllCellsLensCommandTitle();
const runAllAboveTitle = localize.DataScience.runAllCellsAboveLensCommandTitle();
const runAllBelowTitle = localize.DataScience.runCellAndAllBelowLensCommandTitle();
const scrollToCellFormat = localize.DataScience.scrollToCellTitleFormatMessage();

/**
* This class is a singleton that generates code lenses for any document the user opens. It listens
* to cells being execute so it can add 'goto' lenses on cells that have already been run.
Expand All @@ -48,6 +62,10 @@ export class CodeLensFactory implements ICodeLensFactory {
private updateEvent: EventEmitter<void> = new EventEmitter<void>();
private notebookData = new Map<string, PerNotebookData>();
private codeLensCache = new Map<string, CodeLensCacheData>();
private totalCodeLensUpdateTimeInMs: number = 0;
private codeLensUpdateCount: number = 0;
private maxCellCount: number = 0;

constructor(
@inject(IConfigurationService) private configService: IConfigurationService,
@inject(IDocumentManager) private documentManager: IDocumentManager,
Expand All @@ -70,6 +88,15 @@ export class CodeLensFactory implements ICodeLensFactory {
disposables
);
}

public getPerfMeasures(): CodeLensPerfMeasures {
return {
totalCodeLensUpdateTimeInMs: this.totalCodeLensUpdateTimeInMs,
codeLensUpdateCount: this.codeLensUpdateCount,
maxCellCount: this.maxCellCount
};
}

public get updateRequired(): Event<void> {
return this.updateEvent.event;
}
Expand All @@ -85,6 +112,9 @@ export class CodeLensFactory implements ICodeLensFactory {
}

private getCodeLensCacheData(document: TextDocument): CodeLensCacheData {
const stopWatch = new StopWatch();
let updated = false;

// See if we have a cached version of the code lenses for this document
const key = document.uri.toString();
let cache = this.codeLensCache.get(key);
Expand Down Expand Up @@ -127,19 +157,22 @@ export class CodeLensFactory implements ICodeLensFactory {

// Generate our code lenses if necessary
if (cache.documentLenses.length === 0 && needUpdate && cache.cellRanges.length) {
traceInfoIfCI(`Generating new code lenses for version ${document.version} of document ${document.uri}`);
updated = true;
// Enumerate the possible commands for the document based code lenses
const commands = this.enumerateCommands(document.uri);
traceInfoIfCI(`Enumerating commands for ${document.uri.toString()}, ${commands.join(', ')}`);
traceVerbose(
`CodeLensFactory: Generating new code lenses for version ${document.version} of document ${
document.uri
} for commands ${commands.join(', ')}`
);

// Then iterate over all of the cell ranges and generate code lenses for each possible
// commands
// Then iterate over all of the cell ranges and generate code lenses for each enabled command
let firstCell = true;
cache.cellRanges.forEach((r) => {
commands.forEach((c) => {
const codeLens = this.createCodeLens(document, r, c, firstCell);
if (codeLens) {
cache!.documentLenses.push(codeLens); // NOSONAR
cache!.documentLenses.push(codeLens);
}
});
firstCell = false;
Expand Down Expand Up @@ -171,6 +204,12 @@ export class CodeLensFactory implements ICodeLensFactory {
}
}

if (updated) {
this.totalCodeLensUpdateTimeInMs += stopWatch.elapsedTime;
this.codeLensUpdateCount += 1;
this.maxCellCount = Math.max(this.maxCellCount, cache.cellRanges.length);
}

return cache;
}
private getDocumentExecutionCounts(key: string): number[] {
Expand Down Expand Up @@ -270,42 +309,27 @@ export class CodeLensFactory implements ICodeLensFactory {
commandName: string,
isFirst: boolean
): CodeLens | undefined {
// Do not generate interactive window codelenses for TextDocuments which are part of NotebookDocuments
if (workspace.notebookDocuments.find((notebook) => notebook.uri.toString() === document.uri.toString())) {
return;
}
traceInfoIfCI(`Generating code lens for command ${commandName} in ${document.uri.toString()}`);
// We only support specific commands
// Be careful here. These arguments will be serialized during liveshare sessions
// and so shouldn't reference local objects.
const { range, cell_type } = cellRange;
switch (commandName) {
case Commands.RunCurrentCellAndAddBelow:
return this.generateCodeLens(
range,
Commands.RunCurrentCellAndAddBelow,
localize.DataScience.runCurrentCellAndAddBelow()
);
return this.generateCodeLens(range, Commands.RunCurrentCellAndAddBelow, runCurrentandAllBelowTitle);
case Commands.AddCellBelow:
return this.generateCodeLens(
range,
Commands.AddCellBelow,
localize.DataScience.addCellBelowCommandTitle(),
[document.uri, range.start.line]
);
return this.generateCodeLens(range, Commands.AddCellBelow, addCellBelowTitle, [
document.uri,
range.start.line
]);
case Commands.DebugCurrentCellPalette:
return this.generateCodeLens(
range,
Commands.DebugCurrentCellPalette,
localize.DataScience.debugCellCommandTitle()
);
return this.generateCodeLens(range, Commands.DebugCurrentCellPalette, debugCurrentCellTitle);

case Commands.DebugCell:
// If it's not a code cell (e.g. markdown), don't add the "Debug cell" action.
if (cell_type !== 'code') {
break;
}
return this.generateCodeLens(range, Commands.DebugCell, localize.DataScience.debugCellCommandTitle(), [
return this.generateCodeLens(range, Commands.DebugCell, debugCellTitle, [
document.uri,
range.start.line,
range.start.character,
Expand All @@ -318,37 +342,25 @@ export class CodeLensFactory implements ICodeLensFactory {
if (cell_type !== 'code') {
break;
}
return this.generateCodeLens(
range,
Commands.DebugStepOver,
localize.DataScience.debugStepOverCommandTitle(),
[document.uri]
);
return this.generateCodeLens(range, Commands.DebugStepOver, debugStepOverTitle, [document.uri]);

case Commands.DebugContinue:
// Only code cells get debug actions
if (cell_type !== 'code') {
break;
}
return this.generateCodeLens(
range,
Commands.DebugContinue,
localize.DataScience.debugContinueCommandTitle(),
[document.uri]
);
return this.generateCodeLens(range, Commands.DebugContinue, DebugContinueTitle, [document.uri]);

case Commands.DebugStop:
// Only code cells get debug actions
if (cell_type !== 'code') {
break;
}
return this.generateCodeLens(range, Commands.DebugStop, localize.DataScience.debugStopCommandTitle(), [
document.uri
]);
return this.generateCodeLens(range, Commands.DebugStop, debugStopTitle, [document.uri]);

case Commands.RunCurrentCell:
case Commands.RunCell:
return this.generateCodeLens(range, Commands.RunCell, localize.DataScience.runCellLensCommandTitle(), [
return this.generateCodeLens(range, Commands.RunCell, runCellTitle, [
document.uri,
range.start.line,
range.start.character,
Expand All @@ -357,39 +369,35 @@ export class CodeLensFactory implements ICodeLensFactory {
]);

case Commands.RunAllCells:
return this.generateCodeLens(
range,
Commands.RunAllCells,
localize.DataScience.runAllCellsLensCommandTitle(),
[document.uri, range.start.line, range.start.character]
);
return this.generateCodeLens(range, Commands.RunAllCells, runAllCellsTitle, [
document.uri,
range.start.line,
range.start.character
]);

case Commands.RunAllCellsAbovePalette:
case Commands.RunAllCellsAbove:
if (!isFirst) {
return this.generateCodeLens(
range,
Commands.RunAllCellsAbove,
localize.DataScience.runAllCellsAboveLensCommandTitle(),
[document.uri, range.start.line, range.start.character]
);
return this.generateCodeLens(range, Commands.RunAllCellsAbove, runAllAboveTitle, [
document.uri,
range.start.line,
range.start.character
]);
} else {
return this.generateCodeLens(
range,
Commands.RunCellAndAllBelow,
localize.DataScience.runCellAndAllBelowLensCommandTitle(),
[document.uri, range.start.line, range.start.character]
);
return this.generateCodeLens(range, Commands.RunCellAndAllBelow, runAllBelowTitle, [
document.uri,
range.start.line,
range.start.character
]);
}
break;
case Commands.RunCellAndAllBelowPalette:
case Commands.RunCellAndAllBelow:
return this.generateCodeLens(
range,
Commands.RunCellAndAllBelow,
localize.DataScience.runCellAndAllBelowLensCommandTitle(),
[document.uri, range.start.line, range.start.character]
);
return this.generateCodeLens(range, Commands.RunCellAndAllBelow, runAllBelowTitle, [
document.uri,
range.start.line,
range.start.character
]);

default:
traceWarning(`Invalid command for code lens ${commandName}`);
Expand Down Expand Up @@ -418,7 +426,7 @@ export class CodeLensFactory implements ICodeLensFactory {
return this.generateCodeLens(
range,
Commands.ScrollToCell,
localize.DataScience.scrollToCellTitleFormatMessage().format(matchingExecutionCount.toString()),
scrollToCellFormat.format(matchingExecutionCount.toString()),
[document.uri, rangeMatch.id]
);
}
Expand Down
Loading

0 comments on commit 8b8f330

Please sign in to comment.