Skip to content

Commit

Permalink
Lazy load list of all Python Environments (#11674)
Browse files Browse the repository at this point in the history 
* WIP

* Lazy load Python Environments

* Misc

* Use new Python API when waiting for new envs

* WIP

* Fixes

* Fix tests

* Remove test specific method

* fixes

* fixes

* Fixes

* More loggign

* more logging

* more logging

* more logging

* more logging

* more logging

* more logging

* more logging

* more logging

* more logging

* more logging
  • Loading branch information
@DonJayamanne
DonJayamanne authored Oct 18, 2022
1 parent 0111a77 commit 948cfcc
Show file tree
Hide file tree
Showing 25 changed files with 568 additions and 456 deletions.
4 changes: 0 additions & 4 deletions TELEMETRY.csv
View file
Original file line number Diff line number Diff line change
Expand Up @@ -766,10 +766,6 @@ If `jupyter`, then env variables were fetched from Jupyter extension.","","'pyth
"DS_INTERNAL.INTERACTIVE_FILE_TOOLTIPS_PERF","How long it took to return our hover tooltips for a .py file.","Telemetry.InteractiveFileTooltipsPerf","IanMatthewHuff","InteractiveWindow","IntelliSense","","duration","Duration of a measure in milliseconds.
Common measurement used across a number of events.","number","",false
"DS_INTERNAL.INTERACTIVE_FILE_TOOLTIPS_PERF","How long it took to return our hover tooltips for a .py file.","Telemetry.InteractiveFileTooltipsPerf","IanMatthewHuff","InteractiveWindow","IntelliSense","","isResultNull","Result is null if user signalled cancellation or if we timed out","boolean","",false
"DS_INTERNAL.INTERPRETER_LISTING_PERF","Time taken to list the Python interpreters.","Telemetry.InterpreterListingPerf","donjayamanne","N/A","","","duration","Total time taken to list interpreters.","number","",false
"DS_INTERNAL.INTERPRETER_LISTING_PERF","Time taken to list the Python interpreters.","Telemetry.InterpreterListingPerf","donjayamanne","N/A","","","firstTime","Whether this is the first time in the session.
(fetching kernels first time in the session is slower, later its cached).
This is a generic property supported for all telemetry (sent by decorators).","boolean","",true
"DS_INTERNAL.IPYWIDGET_DISCOVER_WIDGETS_NB_EXTENSIONS","Total time taken to discover all IPyWidgets.
This is how long it takes to discover all widgets on disc (from python environment).","Telemetry.DiscoverIPyWidgetNamesPerf","donjayamanne","Notebook, InteractiveWindow","Widgets","","duration","Duration of a measure in milliseconds.
Common measurement used across a number of events.","number","",false
Expand Down
16 changes: 0 additions & 16 deletions TELEMETRY.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1913,22 +1913,6 @@ Expand each section to see more information about that event.
Common measurement used across a number of events.
* DS_INTERNAL.INTERPRETER_LISTING_PERF (Telemetry.InterpreterListingPerf)
Owner: [@donjayamanne](https://github.com/donjayamanne)
```
Time taken to list the Python interpreters.
```
- Properties:
- `firstTime`?: `boolean`
Whether this is the first time in the session.
(fetching kernels first time in the session is slower, later its cached).
This is a generic property supported for all telemetry (sent by decorators).
- Measures:
- `duration`: `number`
Total time taken to list interpreters.
* DS_INTERNAL.IPYWIDGET_DISCOVER_WIDGETS_NB_EXTENSIONS (Telemetry.DiscoverIPyWidgetNamesPerf)
Owner: [@donjayamanne](https://github.com/donjayamanne)
```
Expand Down
10 changes: 0 additions & 10 deletions src/gdpr.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1131,16 +1131,6 @@
"${include}": [
"${F1}"
]
}
*/
//Telemetry.InterpreterListingPerf
/* __GDPR__
"DS_INTERNAL.INTERPRETER_LISTING_PERF" : {
"firstTime": {"classification":"SystemMetaData","purpose":"PerformanceAndHealth","comment":"Whether this is the first time in the session. (fetching kernels first time in the session is slower, later its cached). This is a generic property supported for all telemetry (sent by decorators).","owner":"donjayamanne"},
"${include}": [
"${F1}"
]
}
*/
Expand Down
6 changes: 3 additions & 3 deletions src/kernels/common/commonFinder.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import { Memento } from 'vscode';
import { noop } from '../../platform/common/utils/misc';

// Two cache keys so we can get local and remote separately
export const LocalKernelSpecsCacheKey = 'JUPYTER_LOCAL_KERNELSPECS_V4';
export const RemoteKernelSpecsCacheKey = 'JUPYTER_REMOTE_KERNELSPECS_V4';

export async function removeOldCachedItems(globalState: Memento): Promise<void> {
Expand All @@ -20,9 +19,10 @@ export async function removeOldCachedItems(globalState: Memento): Promise<void>
'JUPYTER_REMOTE_KERNELSPECS',
'JUPYTER_REMOTE_KERNELSPECS_V1',
'JUPYTER_REMOTE_KERNELSPECS_V2',
'JUPYTER_REMOTE_KERNELSPECS_V3'
'JUPYTER_REMOTE_KERNELSPECS_V3',
'JUPYTER_LOCAL_KERNELSPECS_V4'
]
.filter((key) => LocalKernelSpecsCacheKey !== key && RemoteKernelSpecsCacheKey !== key) // Exclude latest cache key
.filter((key) => RemoteKernelSpecsCacheKey !== key) // Exclude latest cache key
.filter((key) => globalState.get(key, undefined) !== undefined)
.map((key) => globalState.update(key, undefined).then(noop, noop))
);
Expand Down
14 changes: 1 addition & 13 deletions src/kernels/jupyter/finder/remoteKernelFinder.unit.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ import { IFileSystemNode } from '../../../platform/common/platform/types.node';
import { JupyterServerUriStorage } from '../launcher/serverUriStorage';
import { FileSystem } from '../../../platform/common/platform/fileSystem.node';
import { IApplicationEnvironment } from '../../../platform/common/application/types';
import { LocalKernelSpecsCacheKey, RemoteKernelSpecsCacheKey } from '../../common/commonFinder';
import { RemoteKernelSpecsCacheKey } from '../../common/commonFinder';
import { IKernelRankingHelper } from '../../../notebooks/controllers/types';
import { KernelRankingHelper } from '../../../notebooks/controllers/kernelRanking/kernelRankingHelper';
import { IExtensions } from '../../../platform/common/types';
Expand Down Expand Up @@ -342,12 +342,6 @@ suite(`Remote Kernel Finder`, () => {
liveRemoteKernel
];
when(cachedRemoteKernelValidator.isValid(anything())).thenResolve(false);
when(
memento.get<{ kernels: KernelConnectionMetadata[]; extensionVersion: string }>(
LocalKernelSpecsCacheKey,
anything()
)
).thenReturn({ kernels: [], extensionVersion: '' });
when(
memento.get<{ kernels: KernelConnectionMetadata[]; extensionVersion: string }>(
RemoteKernelSpecsCacheKey,
Expand Down Expand Up @@ -403,12 +397,6 @@ suite(`Remote Kernel Finder`, () => {
];
when(cachedRemoteKernelValidator.isValid(anything())).thenResolve(false);
when(cachedRemoteKernelValidator.isValid(liveRemoteKernel)).thenResolve(true);
when(
memento.get<{ kernels: KernelConnectionMetadata[]; extensionVersion: string }>(
LocalKernelSpecsCacheKey,
anything()
)
).thenReturn({ kernels: [], extensionVersion: '' });
when(
memento.get<{ kernels: KernelConnectionMetadata[]; extensionVersion: string }>(
RemoteKernelSpecsCacheKey,
Expand Down
161 changes: 23 additions & 138 deletions src/kernels/raw/finder/localKernelFinder.node.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,33 +3,27 @@

'use strict';

import { inject, injectable, named } from 'inversify';
import { CancellationToken, Event, EventEmitter, Memento, Uri } from 'vscode';
import { inject, injectable } from 'inversify';
import { Event, EventEmitter } from 'vscode';
import { IKernelFinder, LocalKernelConnectionMetadata } from '../../../kernels/types';
import { LocalPythonAndRelatedNonPythonKernelSpecFinder } from './localPythonAndRelatedNonPythonKernelSpecFinder.node';
import { LocalKnownPathKernelSpecFinder } from './localKnownPathKernelSpecFinder.node';
import { traceInfo, traceDecoratorError, traceError, traceVerbose } from '../../../platform/logging';
import { GLOBAL_MEMENTO, IDisposableRegistry, IExtensions, IMemento } from '../../../platform/common/types';
import { IDisposableRegistry, IExtensions } from '../../../platform/common/types';
import { capturePerfTelemetry, Telemetry } from '../../../telemetry';
import { ILocalKernelFinder } from '../types';
import { createPromiseFromCancellation } from '../../../platform/common/cancellation';
import { isArray } from '../../../platform/common/utils/sysTypes';
import { deserializeKernelConnection, serializeKernelConnection } from '../../helpers';
import { IApplicationEnvironment } from '../../../platform/common/application/types';
import { waitForCondition } from '../../../platform/common/utils/async';
import { noop } from '../../../platform/common/utils/misc';
import { IFileSystem } from '../../../platform/common/platform/types';
import { areObjectsWithUrisTheSame, noop } from '../../../platform/common/utils/misc';
import { KernelFinder } from '../../kernelFinder';
import { LocalKernelSpecsCacheKey, removeOldCachedItems } from '../../common/commonFinder';
import { IExtensionSyncActivationService } from '../../../platform/activation/types';
import { CondaService } from '../../../platform/common/process/condaService.node';
import * as localize from '../../../platform/common/utils/localize';
import { debounceAsync } from '../../../platform/common/utils/decorators';
import { IPythonExtensionChecker } from '../../../platform/api/types';
import { IInterpreterService } from '../../../platform/interpreter/contracts';
import { EnvironmentType } from '../../../platform/pythonEnvironments/info';
import { ContributedKernelFinderKind } from '../../internalTypes';
import { PYTHON_LANGUAGE } from '../../../platform/common/constants';
import { KnownEnvironmentTypes } from '../../../platform/api/pythonApiTypes';

// This class searches for local kernels.
// First it searches on a global persistent state, then on the installed python interpreters,
Expand All @@ -51,9 +45,6 @@ export class LocalKernelFinder implements ILocalKernelFinder, IExtensionSyncActi
@inject(LocalKnownPathKernelSpecFinder) private readonly nonPythonKernelFinder: LocalKnownPathKernelSpecFinder,
@inject(LocalPythonAndRelatedNonPythonKernelSpecFinder)
private readonly pythonKernelFinder: LocalPythonAndRelatedNonPythonKernelSpecFinder,
@inject(IMemento) @named(GLOBAL_MEMENTO) private readonly globalState: Memento,
@inject(IFileSystem) private readonly fs: IFileSystem,
@inject(IApplicationEnvironment) private readonly env: IApplicationEnvironment,
@inject(IKernelFinder) kernelFinder: KernelFinder,
@inject(IDisposableRegistry) private readonly disposables: IDisposableRegistry,
@inject(IPythonExtensionChecker) private readonly extensionChecker: IPythonExtensionChecker,
Expand All @@ -70,9 +61,7 @@ export class LocalKernelFinder implements ILocalKernelFinder, IExtensionSyncActi
this.condaService.onCondaEnvironmentsChanged(this.onDidChangeCondaEnvironments, this, this.disposables);

this.interpreters.onDidChangeInterpreters(
async () => {
this.updateCache().then(noop, noop);
},
async () => this.updateCache().then(noop, noop),
this,
this.disposables
);
Expand All @@ -90,38 +79,18 @@ export class LocalKernelFinder implements ILocalKernelFinder, IExtensionSyncActi
this.disposables
);
this.nonPythonKernelFinder.onDidChangeKernels(
() => {
this.updateCache().then(noop, noop);
},
this,
this.disposables
);
this.pythonKernelFinder.onDidChangeKernels(
() => {
this.updateCache().then(noop, noop);
},
() => this.updateCache().then(noop, noop),
this,
this.disposables
);
this.pythonKernelFinder.onDidChangeKernels(() => this.updateCache().then(noop, noop), this, this.disposables);
this.wasPythonInstalledWhenFetchingControllers = this.extensionChecker.isPythonExtensionInstalled;
}

private async loadInitialState() {
traceVerbose('LocalKernelFinder: load cache');
// loading cache, which is resource agnostic
const kernelsFromCache = await this.getFromCache();
let kernels: LocalKernelConnectionMetadata[] = [];

this.updateCache().catch(noop);

if (Array.isArray(kernelsFromCache) && kernelsFromCache.length > 0) {
kernels = kernelsFromCache;
await this.writeToCache(kernels);
} else {
await this.updateCache();
}

traceVerbose('LocalKernelFinder: load cache finished');
traceVerbose('LocalKernelFinder: load initial set of kernels');
await this.updateCache();
traceVerbose('LocalKernelFinder: loaded initial set of kernels');
}

@traceDecoratorError('List kernels failed')
Expand All @@ -130,9 +99,12 @@ export class LocalKernelFinder implements ILocalKernelFinder, IExtensionSyncActi
try {
let kernels: LocalKernelConnectionMetadata[] = [];
// Exclude python kernel specs (we'll get that from the pythonKernelFinder)
const nonPythonKernels = this.nonPythonKernelFinder.kernels.filter(
(item) => item.kernelSpec.language !== PYTHON_LANGUAGE
);
const nonPythonKernels = this.nonPythonKernelFinder.kernels.filter((item) => {
if (this.extensionChecker.isPythonExtensionInstalled) {
return item.kernelSpec.language !== PYTHON_LANGUAGE;
}
return true;
});
kernels = kernels.concat(nonPythonKernels).concat(this.pythonKernelFinder.kernels);
await this.writeToCache(kernels);
} catch (ex) {
Expand All @@ -147,11 +119,11 @@ export class LocalKernelFinder implements ILocalKernelFinder, IExtensionSyncActi
}
// A new conda environment was added or removed, hence refresh the kernels.
// Wait for the new env to be discovered before refreshing the kernels.
const previousCondaEnvCount = (await this.interpreters.getInterpreters()).filter(
(item) => item.envType === EnvironmentType.Conda
const previousCondaEnvCount = this.interpreters.environments.filter(
(item) => (item.environment?.type as KnownEnvironmentTypes) === 'Conda'
).length;

await this.interpreters.refreshInterpreters(true);
await this.interpreters.refreshInterpreters(true).ignoreErrors();
// Possible discovering interpreters is very quick and we've already discovered it, hence refresh kernels immediately.
await this.updateCache();

Expand All @@ -161,8 +133,8 @@ export class LocalKernelFinder implements ILocalKernelFinder, IExtensionSyncActi
// Wait for around 5s between each try, we know Python extension can be slow to discover interpreters.
await waitForCondition(
async () => {
const condaEnvCount = (await this.interpreters.getInterpreters()).filter(
(item) => item.envType === EnvironmentType.Conda
const condaEnvCount = this.interpreters.environments.filter(
(item) => (item.environment?.type as KnownEnvironmentTypes) === 'Conda'
).length;
if (condaEnvCount > previousCondaEnvCount) {
return true;
Expand All @@ -180,64 +152,6 @@ export class LocalKernelFinder implements ILocalKernelFinder, IExtensionSyncActi
public get kernels(): LocalKernelConnectionMetadata[] {
return this.cache;
}

private async getFromCache(cancelToken?: CancellationToken): Promise<LocalKernelConnectionMetadata[]> {
try {
traceVerbose('LocalKernelFinder: get from cache');

let results: LocalKernelConnectionMetadata[] = this.cache;

// If not in memory, check memento
if (!results || results.length === 0) {
// Check memento too
const values = this.globalState.get<{
kernels: LocalKernelConnectionMetadata[];
extensionVersion: string;
}>(this.getCacheKey(), { kernels: [], extensionVersion: '' });

/**
* The cached list of raw kernels is pointing to kernelSpec.json files in the extensions directory.
* Assume you have version 1 of extension installed.
* Now you update to version 2, at this point the cache still points to version 1 and the kernelSpec.json files are in the directory version 1.
* Those files in directory for version 1 could get deleted by VS Code at any point in time, as thats an old version of the extension and user has now installed version 2.
* Hence its wrong and buggy to use those files.
* To ensure we don't run into weird issues with the use of cached kernelSpec.json files, we ensure the cache is tied to each version of the extension.
*/
if (values && isArray(values.kernels) && values.extensionVersion === this.env.extensionVersion) {
results = values.kernels.map(deserializeKernelConnection) as LocalKernelConnectionMetadata[];
this.cache = results;
}
}

// Validate
const validValues: LocalKernelConnectionMetadata[] = [];
const promise = Promise.all(
results.map(async (item) => {
if (await this.isValidCachedKernel(item)) {
validValues.push(item);
}
})
);
if (cancelToken) {
await Promise.race([
promise,
createPromiseFromCancellation({
token: cancelToken,
cancelAction: 'resolve',
defaultValue: undefined
})
]);
} else {
await promise;
}
return validValues;
} catch (ex) {
traceError('LocalKernelFinder: Failed to get from cache', ex);
}

return [];
}

private filterKernels(kernels: LocalKernelConnectionMetadata[]) {
return kernels.filter(({ kernelSpec }) => {
if (!kernelSpec) {
Expand Down Expand Up @@ -265,42 +179,13 @@ export class LocalKernelFinder implements ILocalKernelFinder, IExtensionSyncActi
return true;
});
this.cache = this.filterKernels(uniqueKernels);
if (JSON.stringify(oldValues) === JSON.stringify(this.cache)) {
if (areObjectsWithUrisTheSame(oldValues, this.cache)) {
return;
}

this._onDidChangeKernels.fire();
const serialized = values.map(serializeKernelConnection);
await Promise.all([
removeOldCachedItems(this.globalState),
this.globalState.update(this.getCacheKey(), {
kernels: serialized,
extensionVersion: this.env.extensionVersion
})
]);
} catch (ex) {
traceError('LocalKernelFinder: Failed to write to cache', ex);
}
}

private getCacheKey() {
return LocalKernelSpecsCacheKey;
}

private async isValidCachedKernel(kernel: LocalKernelConnectionMetadata): Promise<boolean> {
switch (kernel.kind) {
case 'startUsingPythonInterpreter':
// Interpreters have to still exist
return this.fs.exists(kernel.interpreter.uri);

case 'startUsingLocalKernelSpec':
// Spec files have to still exist and interpreters have to exist
const promiseSpec = kernel.kernelSpec.specFile
? this.fs.exists(Uri.file(kernel.kernelSpec.specFile))
: Promise.resolve(true);
return promiseSpec.then((r) => {
return r && kernel.interpreter ? this.fs.exists(kernel.interpreter.uri) : Promise.resolve(true);
});
}
}
}
Loading

0 comments on commit 948cfcc

Please sign in to comment.