From 0f0065ba03765a832a339906727263f6f85242e7 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 18 Feb 2020 09:39:11 -0800 Subject: [PATCH 1/4] Ensure DS auto start code is non-blocking (#10171) * Perf fixes * Add news entry --- news/2 Fixes/10170.md | 1 + src/client/datascience/activation.ts | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) create mode 100644 news/2 Fixes/10170.md diff --git a/news/2 Fixes/10170.md b/news/2 Fixes/10170.md new file mode 100644 index 000000000000..f9cc91fb8d8c --- /dev/null +++ b/news/2 Fixes/10170.md @@ -0,0 +1 @@ +Perf improvements to executing startup code for `Data Science` features when extension loads. diff --git a/src/client/datascience/activation.ts b/src/client/datascience/activation.ts index 7e73b10dd1a3..f2b7fbd012c6 100644 --- a/src/client/datascience/activation.ts +++ b/src/client/datascience/activation.ts @@ -30,7 +30,7 @@ export class Activation implements IExtensionSingleActivationService { this.disposables.push(this.jupyterInterpreterService.onDidChangeInterpreter(this.onDidChangeInterpreter, this)); // Warm up our selected interpreter for the extension this.jupyterInterpreterService.setInitialInterpreter().ignoreErrors(); - await this.contextService.activate(); + this.contextService.activate().ignoreErrors(); } private onDidOpenNotebookEditor(_: INotebookEditor) { From fa0e613d547158ac2ee7343a1e57234e26b5bb93 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 18 Feb 2020 10:19:47 -0800 Subject: [PATCH 2/4] Include interperter name in message (#10174) For #10071 Fix to ensure interpreter is included in message, here's the actual message seen by the user. Error: Jupyter cannot be started. Error attempting to locate jupyter: 'Kernelspec' module not installed in the selected interpreter ({0}). Note the {0} --- .../interpreter/jupyterInterpreterSubCommandExecutionService.ts | 2 +- .../jupyterInterpreterSubCommandExecutionService.unit.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/client/datascience/jupyter/interpreter/jupyterInterpreterSubCommandExecutionService.ts b/src/client/datascience/jupyter/interpreter/jupyterInterpreterSubCommandExecutionService.ts index 6596e5189dc8..83bc261d1431 100644 --- a/src/client/datascience/jupyter/interpreter/jupyterInterpreterSubCommandExecutionService.ts +++ b/src/client/datascience/jupyter/interpreter/jupyterInterpreterSubCommandExecutionService.ts @@ -83,7 +83,7 @@ export class JupyterInterpreterSubCommandExecutionService implements IJupyterSub } if (productsNotInstalled.length === 1 && productsNotInstalled[0] === Product.kernelspec) { - return DataScience.jupyterKernelSpecModuleNotFound(); + return DataScience.jupyterKernelSpecModuleNotFound().format(interpreter.path); } return getMessageForLibrariesNotInstalled(productsNotInstalled, interpreter.displayName); diff --git a/src/test/datascience/jupyter/interpreter/jupyterInterpreterSubCommandExecutionService.unit.test.ts b/src/test/datascience/jupyter/interpreter/jupyterInterpreterSubCommandExecutionService.unit.test.ts index f7d80baaa9d4..1a37eb1111ce 100644 --- a/src/test/datascience/jupyter/interpreter/jupyterInterpreterSubCommandExecutionService.unit.test.ts +++ b/src/test/datascience/jupyter/interpreter/jupyterInterpreterSubCommandExecutionService.unit.test.ts @@ -223,7 +223,7 @@ suite('Data Science - Jupyter InterpreterSubCommandExecutionService', () => { const reason = await jupyterInterpreterExecutionService.getReasonForJupyterNotebookNotBeingSupported(undefined); - assert.equal(reason, DataScience.jupyterKernelSpecModuleNotFound()); + assert.equal(reason, DataScience.jupyterKernelSpecModuleNotFound().format(selectedJupyterInterpreter.path)); }); test('Can start jupyer notebook', async () => { const output = await jupyterInterpreterExecutionService.startNotebook([], {}); From d19d7ae6cf7821d9950f385ac025e57828b3a425 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 18 Feb 2020 12:20:22 -0800 Subject: [PATCH 3/4] Ignore display name when searching interpreters (#10175) Partial fix for #10173 Comparing against path of interpreter alone is sufficient. Figured there's no harm in trying to minimise occurrences of dup kernels (basically just compare against python executable path, as thats sufficient and always accurate) Display name: Can change from version to version of Python extension (i.e. it shouldn't have been used as a unique identifier) Display name can change after extension loads and more information about interpreter is available. --- src/client/datascience/jupyter/kernels/kernelService.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/client/datascience/jupyter/kernels/kernelService.ts b/src/client/datascience/jupyter/kernels/kernelService.ts index aee36b7050f9..2fe79ba5bfb7 100644 --- a/src/client/datascience/jupyter/kernels/kernelService.ts +++ b/src/client/datascience/jupyter/kernels/kernelService.ts @@ -99,9 +99,6 @@ export class KernelService { if (item.language.toLowerCase() !== PYTHON_LANGUAGE.toLowerCase()) { return false; } - if (item.display_name !== option.displayName) { - return false; - } return this.fileSystem.arePathsSame(item.argv[0], option.path) || this.fileSystem.arePathsSame(item.metadata?.interpreter?.path || '', option.path); }); } else { From 501043b30b073c4d2c33fcf2638b9aa60c970330 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Tue, 18 Feb 2020 13:39:12 -0800 Subject: [PATCH 4/4] Track cold/warm times to execute notebook cells (#10180) For #10176 --- news/3 Code Health/10176.md | 1 + .../interactive-ipynb/nativeEditor.ts | 17 ++++++++++++++++- src/client/telemetry/index.ts | 12 ++++++++++-- 3 files changed, 27 insertions(+), 3 deletions(-) create mode 100644 news/3 Code Health/10176.md diff --git a/news/3 Code Health/10176.md b/news/3 Code Health/10176.md new file mode 100644 index 000000000000..28237633c5b2 --- /dev/null +++ b/news/3 Code Health/10176.md @@ -0,0 +1 @@ +Track cold/warm times to execute notebook cells. diff --git a/src/client/datascience/interactive-ipynb/nativeEditor.ts b/src/client/datascience/interactive-ipynb/nativeEditor.ts index 8cdb2480432d..04f25608d803 100644 --- a/src/client/datascience/interactive-ipynb/nativeEditor.ts +++ b/src/client/datascience/interactive-ipynb/nativeEditor.ts @@ -77,6 +77,7 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor { public get onDidChangeViewState(): Event { return this._onDidChangeViewState.event; } + private sentExecuteCellTelemetry: boolean = false; private _onDidChangeViewState = new EventEmitter(); private closedEvent: EventEmitter = new EventEmitter(); private executedEvent: EventEmitter = new EventEmitter(); @@ -362,8 +363,10 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor { } protected submitCode(code: string, file: string, line: number, id?: string, editor?: TextEditor, debug?: boolean): Promise { + const stopWatch = new StopWatch(); + const submitCodePromise = super.submitCode(code, file, line, id, editor, debug).finally(() => this.sendPerceivedCellExecute(stopWatch)); // When code is executed, update the version number in the metadata. - return super.submitCode(code, file, line, id, editor, debug).then(value => { + return submitCodePromise.then(value => { this.updateVersionInfoInNotebook() .then(() => { this.metadataUpdatedEvent.fire(this); @@ -513,6 +516,18 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor { // Actually don't close, just let the error bubble out } + private sendPerceivedCellExecute(runningStopWatch?: StopWatch) { + if (runningStopWatch) { + const props = { notebook: true }; + if (!this.sentExecuteCellTelemetry) { + this.sentExecuteCellTelemetry = true; + sendTelemetryEvent(Telemetry.ExecuteCellPerceivedCold, runningStopWatch.elapsedTime, props); + } else { + sendTelemetryEvent(Telemetry.ExecuteCellPerceivedWarm, runningStopWatch.elapsedTime, props); + } + } + } + /** * Update the Python Version number in the notebook data. * diff --git a/src/client/telemetry/index.ts b/src/client/telemetry/index.ts index 4d8ae0be241c..c0c212d3abb2 100644 --- a/src/client/telemetry/index.ts +++ b/src/client/telemetry/index.ts @@ -1459,8 +1459,16 @@ export interface IEventNamePropertyMapping { [Telemetry.DisableInteractiveShiftEnter]: never | undefined; [Telemetry.EnableInteractiveShiftEnter]: never | undefined; [Telemetry.ExecuteCell]: never | undefined; - [Telemetry.ExecuteCellPerceivedCold]: never | undefined; - [Telemetry.ExecuteCellPerceivedWarm]: never | undefined; + /** + * Telemetry sent to capture first time execution of a cell. + * If `notebook = true`, this its telemetry for native editor/notebooks. + */ + [Telemetry.ExecuteCellPerceivedCold]: undefined | { notebook: boolean }; + /** + * Telemetry sent to capture subsequent execution of a cell. + * If `notebook = true`, this its telemetry for native editor/notebooks. + */ + [Telemetry.ExecuteCellPerceivedWarm]: undefined | { notebook: boolean }; [Telemetry.ExecuteNativeCell]: never | undefined; [Telemetry.ExpandAll]: never | undefined; [Telemetry.ExportNotebook]: never | undefined;