From 5b516bc13b4f9b1071488433ef814acb04f332ad Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 21 Nov 2023 18:59:49 +0000 Subject: [PATCH 01/20] KeyAA --- src/client/pythonEnvironments/base/locator.ts | 2 +- .../pythonEnvironments/base/locatorUtils.ts | 2 +- .../locators/common/resourceBasedLocator.ts | 13 +++++++++-- .../composite/envsCollectionService.ts | 10 +++++--- .../base/locators/composite/envsReducer.ts | 23 ++++++++----------- .../base/locators/composite/envsResolver.ts | 11 ++++----- src/test/pythonEnvironments/base/common.ts | 2 +- 7 files changed, 35 insertions(+), 28 deletions(-) diff --git a/src/client/pythonEnvironments/base/locator.ts b/src/client/pythonEnvironments/base/locator.ts index ab3b17629bc5..3fd5194c37da 100644 --- a/src/client/pythonEnvironments/base/locator.ts +++ b/src/client/pythonEnvironments/base/locator.ts @@ -20,7 +20,7 @@ export type PythonEnvUpdatedEvent = { /** * The iteration index of The env info that was previously provided. */ - index: number; + index?: number; /** * The env info that was previously provided. */ diff --git a/src/client/pythonEnvironments/base/locatorUtils.ts b/src/client/pythonEnvironments/base/locatorUtils.ts index 97cb6298416f..dcbf2b1b5877 100644 --- a/src/client/pythonEnvironments/base/locatorUtils.ts +++ b/src/client/pythonEnvironments/base/locatorUtils.ts @@ -85,7 +85,7 @@ export async function getEnvs(iterator: IPythonEnvsIterator imp await this.disposables.dispose(); } - public async *iterEnvs(query?: PythonLocatorQuery): IPythonEnvsIterator { - await this.activate(); + public iterEnvs(query?: PythonLocatorQuery): IPythonEnvsIterator { const iterator = this.doIterEnvs(query); + const it = this._iterEnvs(iterator, query); + it.onUpdated = iterator.onUpdated; + return it; + } + + private async *_iterEnvs( + iterator: IPythonEnvsIterator, + query?: PythonLocatorQuery, + ): IPythonEnvsIterator { + await this.activate(); if (query?.envPath) { let result = await iterator.next(); while (!result.done) { diff --git a/src/client/pythonEnvironments/base/locators/composite/envsCollectionService.ts b/src/client/pythonEnvironments/base/locators/composite/envsCollectionService.ts index cc37b1f82cfd..1443d269c368 100644 --- a/src/client/pythonEnvironments/base/locators/composite/envsCollectionService.ts +++ b/src/client/pythonEnvironments/base/locators/composite/envsCollectionService.ts @@ -141,12 +141,12 @@ export class EnvsCollectionService extends PythonEnvsWatcher(); if (iterator.onUpdated !== undefined) { - const listener = iterator.onUpdated(async (event) => { + iterator.onUpdated(async (event) => { if (isProgressEvent(event)) { switch (event.stage) { case ProgressReportStage.discoveryFinished: state.done = true; - listener.dispose(); + // listener.dispose(); break; case ProgressReportStage.allPathsDiscovered: if (!query) { @@ -157,13 +157,17 @@ export class EnvsCollectionService extends PythonEnvsWatcher { - state.pending += 1; + iterator.onUpdated((event) => { if (isProgressEvent(event)) { if (event.stage === ProgressReportStage.discoveryFinished) { state.done = true; - listener.dispose(); + // listener.dispose(); } else { didUpdate.fire(event); } @@ -63,19 +63,15 @@ async function* iterEnvsIterator( throw new Error( 'Unsupported behavior: `undefined` environment updates are not supported from downstream locators in reducer', ); - } else if (seen[event.index] !== undefined) { + } else if (event.index && seen[event.index] !== undefined) { const oldEnv = seen[event.index]; seen[event.index] = event.update; didUpdate.fire({ index: event.index, old: oldEnv, update: event.update }); } else { - // This implies a problem in a downstream locator - traceVerbose(`Expected already iterated env, got ${event.old} (#${event.index})`); + didUpdate.fire({ update: event.update }); } - state.pending -= 1; checkIfFinishedAndNotify(state, didUpdate); }); - } else { - didUpdate.fire({ stage: ProgressReportStage.discoveryStarted }); } let result = await iterator.next(); @@ -91,10 +87,8 @@ async function* iterEnvsIterator( } result = await iterator.next(); } - if (iterator.onUpdated === undefined) { - state.done = true; - checkIfFinishedAndNotify(state, didUpdate); - } + state.done = true; + checkIfFinishedAndNotify(state, didUpdate); } async function resolveDifferencesInBackground( @@ -128,8 +122,9 @@ function checkIfFinishedAndNotify( ) { if (state.done && state.pending === 0) { didUpdate.fire({ stage: ProgressReportStage.discoveryFinished }); - didUpdate.dispose(); + // didUpdate.dispose(); traceVerbose(`Finished with environment reducer`); + state.done = false; // No need to notify again. } } diff --git a/src/client/pythonEnvironments/base/locators/composite/envsResolver.ts b/src/client/pythonEnvironments/base/locators/composite/envsResolver.ts index 2ba54e07ed9c..ad8af59883da 100644 --- a/src/client/pythonEnvironments/base/locators/composite/envsResolver.ts +++ b/src/client/pythonEnvironments/base/locators/composite/envsResolver.ts @@ -81,13 +81,13 @@ export class PythonEnvsResolver implements IResolvingLocator { const seen: PythonEnvInfo[] = []; if (iterator.onUpdated !== undefined) { - const listener = iterator.onUpdated(async (event) => { + iterator.onUpdated(async (event) => { state.pending += 1; if (isProgressEvent(event)) { if (event.stage === ProgressReportStage.discoveryFinished) { didUpdate.fire({ stage: ProgressReportStage.allPathsDiscovered }); state.done = true; - listener.dispose(); + // listener.dispose(); } else { didUpdate.fire(event); } @@ -95,15 +95,14 @@ export class PythonEnvsResolver implements IResolvingLocator { throw new Error( 'Unsupported behavior: `undefined` environment updates are not supported from downstream locators in resolver', ); - } else if (seen[event.index] !== undefined) { + } else if (event.index && seen[event.index] !== undefined) { const old = seen[event.index]; await setKind(event.update, environmentKinds); seen[event.index] = await resolveBasicEnv(event.update); didUpdate.fire({ old, index: event.index, update: seen[event.index] }); this.resolveInBackground(event.index, state, didUpdate, seen).ignoreErrors(); } else { - // This implies a problem in a downstream locator - traceVerbose(`Expected already iterated env, got ${event.old} (#${event.index})`); + didUpdate.fire({ update: await this.resolveEnv(event.update.executablePath) }); } state.pending -= 1; checkIfFinishedAndNotify(state, didUpdate); @@ -173,7 +172,7 @@ function checkIfFinishedAndNotify( ) { if (state.done && state.pending === 0) { didUpdate.fire({ stage: ProgressReportStage.discoveryFinished }); - didUpdate.dispose(); + // didUpdate.dispose(); traceVerbose(`Finished with environment resolver`); } } diff --git a/src/test/pythonEnvironments/base/common.ts b/src/test/pythonEnvironments/base/common.ts index 847d6e752273..1303b792eb57 100644 --- a/src/test/pythonEnvironments/base/common.ts +++ b/src/test/pythonEnvironments/base/common.ts @@ -203,7 +203,7 @@ export async function getEnvsWithUpdates( } updatesDone.resolve(); listener.dispose(); - } else { + } else if (event.index) { const { index, update } = event; // We don't worry about if envs[index] is set already. envs[index] = update; From ef8558939cbdfbaf979866ec7abdb3ecf37c2cc7 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 21 Nov 2023 12:46:48 -0800 Subject: [PATCH 02/20] Lazy locate windows registry --- .../lowLevel/windowsRegistryLocator.ts | 73 +++++++++++++------ 1 file changed, 49 insertions(+), 24 deletions(-) diff --git a/src/client/pythonEnvironments/base/locators/lowLevel/windowsRegistryLocator.ts b/src/client/pythonEnvironments/base/locators/lowLevel/windowsRegistryLocator.ts index 16b2167021db..a43a93e9f71c 100644 --- a/src/client/pythonEnvironments/base/locators/lowLevel/windowsRegistryLocator.ts +++ b/src/client/pythonEnvironments/base/locators/lowLevel/windowsRegistryLocator.ts @@ -1,9 +1,17 @@ +/* eslint-disable require-yield */ /* eslint-disable no-continue */ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. +import { EventEmitter } from 'vscode'; import { PythonEnvKind, PythonEnvSource } from '../../info'; -import { BasicEnvInfo, IPythonEnvsIterator, Locator } from '../../locator'; +import { + BasicEnvInfo, + IPythonEnvsIterator, + Locator, + ProgressNotificationEvent, + PythonEnvUpdatedEvent, +} from '../../locator'; import { getRegistryInterpreters } from '../../../common/windowsUtils'; import { traceError, traceVerbose } from '../../../../logging'; import { isMicrosoftStoreDir } from '../../../common/environmentManagers/microsoftStoreEnv'; @@ -18,29 +26,46 @@ export class WindowsRegistryLocator extends Locator { _?: unknown, useWorkerThreads = inExperiment(DiscoveryUsingWorkers.experiment), ): IPythonEnvsIterator { - const iterator = async function* () { - traceVerbose('Searching for windows registry interpreters'); - const interpreters = await getRegistryInterpreters(useWorkerThreads); - for (const interpreter of interpreters) { - try { - // Filter out Microsoft Store app directories. We have a store app locator that handles this. - // The python.exe available in these directories might not be python. It can be a store install - // shortcut that takes you to microsoft store. - if (isMicrosoftStoreDir(interpreter.interpreterPath)) { - continue; - } - const env: BasicEnvInfo = { - kind: PythonEnvKind.OtherGlobal, - executablePath: interpreter.interpreterPath, - source: [PythonEnvSource.WindowsRegistry], - }; - yield env; - } catch (ex) { - traceError(`Failed to process environment: ${interpreter}`, ex); - } + const didUpdate = new EventEmitter | ProgressNotificationEvent>(); + const iterator = iterEnvsIterator(useWorkerThreads, didUpdate); + iterator.onUpdated = didUpdate.event; + return iterator; + } +} + +async function* iterEnvsIterator( + useWorkerThreads: boolean, + didUpdate: EventEmitter | ProgressNotificationEvent>, +): IPythonEnvsIterator { + updateLazily(useWorkerThreads, didUpdate).ignoreErrors(); +} + +async function updateLazily( + useWorkerThreads: boolean, + didUpdate: EventEmitter | ProgressNotificationEvent>, +) { + // Windows registry is slow and often not necessary, so notify completion while still updating lazily as we find stuff. + traceVerbose('Searching for windows registry interpreters'); + console.time('Time taken for windows registry'); + const interpreters = await getRegistryInterpreters(useWorkerThreads); + for (const interpreter of interpreters) { + try { + // Filter out Microsoft Store app directories. We have a store app locator that handles this. + // The python.exe available in these directories might not be python. It can be a store install + // shortcut that takes you to microsoft store. + if (isMicrosoftStoreDir(interpreter.interpreterPath)) { + continue; } - traceVerbose('Finished searching for windows registry interpreters'); - }; - return iterator(); + const env: BasicEnvInfo = { + kind: PythonEnvKind.OtherGlobal, + executablePath: interpreter.interpreterPath, + source: [PythonEnvSource.WindowsRegistry], + }; + didUpdate.fire({ update: env }); + } catch (ex) { + traceError(`Failed to process environment: ${interpreter}`, ex); + } } + traceVerbose('Finished searching for windows registry interpreters'); + console.timeEnd('Time taken for windows registry'); } From 5b9ae666a0db5e9cff78c5f24d88ac070818cf08 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 21 Nov 2023 13:12:35 -0800 Subject: [PATCH 03/20] Do not wait to validate all interpreters when auto-selecting --- src/client/interpreter/autoSelection/index.ts | 45 ++++++++++++++----- src/client/interpreter/contracts.ts | 4 +- src/client/interpreter/interpreterService.ts | 10 ++++- src/client/pythonEnvironments/legacyIOC.ts | 11 +++-- 4 files changed, 55 insertions(+), 15 deletions(-) diff --git a/src/client/interpreter/autoSelection/index.ts b/src/client/interpreter/autoSelection/index.ts index 7714c487ed30..9259a76415d9 100644 --- a/src/client/interpreter/autoSelection/index.ts +++ b/src/client/interpreter/autoSelection/index.ts @@ -6,11 +6,13 @@ import { inject, injectable } from 'inversify'; import { Event, EventEmitter, Uri } from 'vscode'; import { IWorkspaceService } from '../../common/application/types'; +import { DiscoveryUsingWorkers } from '../../common/experiments/groups'; import '../../common/extensions'; import { IFileSystem } from '../../common/platform/types'; -import { IPersistentState, IPersistentStateFactory, Resource } from '../../common/types'; +import { IExperimentService, IPersistentState, IPersistentStateFactory, Resource } from '../../common/types'; import { createDeferred, Deferred } from '../../common/utils/async'; import { compareSemVerLikeVersions } from '../../pythonEnvironments/base/info/pythonVersion'; +import { ProgressReportStage } from '../../pythonEnvironments/base/locator'; import { PythonEnvironment } from '../../pythonEnvironments/info'; import { sendTelemetryEvent } from '../../telemetry'; import { EventName } from '../../telemetry/constants'; @@ -44,6 +46,7 @@ export class InterpreterAutoSelectionService implements IInterpreterAutoSelectio @inject(IInterpreterComparer) private readonly envTypeComparer: IInterpreterComparer, @inject(IInterpreterAutoSelectionProxyService) proxy: IInterpreterAutoSelectionProxyService, @inject(IInterpreterHelper) private readonly interpreterHelper: IInterpreterHelper, + @inject(IExperimentService) private readonly experimentService: IExperimentService, ) { proxy.registerInstance!(this); } @@ -199,22 +202,44 @@ export class InterpreterAutoSelectionService implements IInterpreterAutoSelectio private async autoselectInterpreterWithLocators(resource: Resource): Promise { // Do not perform a full interpreter search if we already have cached interpreters for this workspace. const queriedState = this.getAutoSelectionInterpretersQueryState(resource); - if (queriedState.value !== true && resource) { + const globalQueriedState = this.getAutoSelectionQueriedOnceState(); + if (globalQueriedState.value && queriedState.value !== true && resource) { await this.interpreterService.triggerRefresh({ searchLocations: { roots: [resource], doNotIncludeNonRooted: true }, }); } - const globalQueriedState = this.getAutoSelectionQueriedOnceState(); - if (!globalQueriedState.value) { - // Global interpreters are loaded the first time an extension loads, after which we don't need to - // wait on global interpreter promise refresh. - await this.interpreterService.refreshPromise; - } - const interpreters = this.interpreterService.getInterpreters(resource); + const inExperiment = this.experimentService.inExperimentSync(DiscoveryUsingWorkers.experiment); const workspaceUri = this.interpreterHelper.getActiveWorkspaceUri(resource); + let recommendedInterpreter: PythonEnvironment | undefined; + if (inExperiment) { + if (!globalQueriedState.value) { + // Global interpreters are loaded the first time an extension loads, after which we don't need to + // wait on global interpreter promise refresh. + // Do not wait for validation of all interpreters to finish, we only need to validate the recommended interpreter. + await this.interpreterService.getRefreshPromise({ stage: ProgressReportStage.allPathsDiscovered }); + } + let interpreters = this.interpreterService.getInterpreters(resource); + + recommendedInterpreter = this.envTypeComparer.getRecommended(interpreters, workspaceUri?.folderUri); + const details = recommendedInterpreter + ? await this.interpreterService.getInterpreterDetails(recommendedInterpreter.path) + : undefined; + if (!details || !recommendedInterpreter) { + await this.interpreterService.refreshPromise; // Interpreter is invalid, wait for all of validation to finish. + interpreters = this.interpreterService.getInterpreters(resource); + recommendedInterpreter = this.envTypeComparer.getRecommended(interpreters, workspaceUri?.folderUri); + } + } else { + if (!globalQueriedState.value) { + // Global interpreters are loaded the first time an extension loads, after which we don't need to + // wait on global interpreter promise refresh. + await this.interpreterService.refreshPromise; + } + const interpreters = this.interpreterService.getInterpreters(resource); - const recommendedInterpreter = this.envTypeComparer.getRecommended(interpreters, workspaceUri?.folderUri); + recommendedInterpreter = this.envTypeComparer.getRecommended(interpreters, workspaceUri?.folderUri); + } if (!recommendedInterpreter) { return; } diff --git a/src/client/interpreter/contracts.ts b/src/client/interpreter/contracts.ts index bfaebd235f19..30a05c140249 100644 --- a/src/client/interpreter/contracts.ts +++ b/src/client/interpreter/contracts.ts @@ -4,6 +4,7 @@ import { FileChangeType } from '../common/platform/fileSystemWatcher'; import { Resource } from '../common/types'; import { PythonEnvSource } from '../pythonEnvironments/base/info'; import { + GetRefreshEnvironmentsOptions, ProgressNotificationEvent, PythonLocatorQuery, TriggerRefreshOptions, @@ -22,7 +23,7 @@ export const IComponentAdapter = Symbol('IComponentAdapter'); export interface IComponentAdapter { readonly onProgress: Event; triggerRefresh(query?: PythonLocatorQuery, options?: TriggerRefreshOptions): Promise; - getRefreshPromise(): Promise | undefined; + getRefreshPromise(options?: GetRefreshEnvironmentsOptions): Promise | undefined; readonly onChanged: Event; // VirtualEnvPrompt onDidCreate(resource: Resource, callback: () => void): Disposable; @@ -74,6 +75,7 @@ export const IInterpreterService = Symbol('IInterpreterService'); export interface IInterpreterService { triggerRefresh(query?: PythonLocatorQuery, options?: TriggerRefreshOptions): Promise; readonly refreshPromise: Promise | undefined; + getRefreshPromise(options?: GetRefreshEnvironmentsOptions): Promise | undefined; readonly onDidChangeInterpreters: Event; onDidChangeInterpreterConfiguration: Event; onDidChangeInterpreter: Event; diff --git a/src/client/interpreter/interpreterService.ts b/src/client/interpreter/interpreterService.ts index c97a35c4a973..e9829d978fb6 100644 --- a/src/client/interpreter/interpreterService.ts +++ b/src/client/interpreter/interpreterService.ts @@ -38,7 +38,11 @@ import { Interpreters } from '../common/utils/localize'; import { sendTelemetryEvent } from '../telemetry'; import { EventName } from '../telemetry/constants'; import { cache } from '../common/utils/decorators'; -import { PythonLocatorQuery, TriggerRefreshOptions } from '../pythonEnvironments/base/locator'; +import { + GetRefreshEnvironmentsOptions, + PythonLocatorQuery, + TriggerRefreshOptions, +} from '../pythonEnvironments/base/locator'; import { sleep } from '../common/utils/async'; type StoredPythonEnvironment = PythonEnvironment & { store?: boolean }; @@ -59,6 +63,10 @@ export class InterpreterService implements Disposable, IInterpreterService { return this.pyenvs.getRefreshPromise(); } + public getRefreshPromise(options?: GetRefreshEnvironmentsOptions): Promise | undefined { + return this.pyenvs.getRefreshPromise(options); + } + public get onDidChangeInterpreter(): Event { return this.didChangeInterpreterEmitter.event; } diff --git a/src/client/pythonEnvironments/legacyIOC.ts b/src/client/pythonEnvironments/legacyIOC.ts index f116bdb63a89..31de503a0f3c 100644 --- a/src/client/pythonEnvironments/legacyIOC.ts +++ b/src/client/pythonEnvironments/legacyIOC.ts @@ -9,7 +9,12 @@ import { Resource } from '../common/types'; import { IComponentAdapter, ICondaService, PythonEnvironmentsChangedEvent } from '../interpreter/contracts'; import { IServiceManager } from '../ioc/types'; import { PythonEnvInfo, PythonEnvKind, PythonEnvSource } from './base/info'; -import { IDiscoveryAPI, PythonLocatorQuery, TriggerRefreshOptions } from './base/locator'; +import { + GetRefreshEnvironmentsOptions, + IDiscoveryAPI, + PythonLocatorQuery, + TriggerRefreshOptions, +} from './base/locator'; import { isMacDefaultPythonPath } from './common/environmentManagers/macDefault'; import { isParentPath } from './common/externalDependencies'; import { EnvironmentType, PythonEnvironment } from './info'; @@ -102,8 +107,8 @@ class ComponentAdapter implements IComponentAdapter { return this.api.triggerRefresh(query, options); } - public getRefreshPromise() { - return this.api.getRefreshPromise(); + public getRefreshPromise(options?: GetRefreshEnvironmentsOptions) { + return this.api.getRefreshPromise(options); } public get onProgress() { From 73cc0272bb940f959576babeec4b8849a4553508 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 21 Nov 2023 13:31:32 -0800 Subject: [PATCH 04/20] Enhance windows reg --- .../lowLevel/windowsRegistryLocator.ts | 42 ++++++++++++++----- 1 file changed, 31 insertions(+), 11 deletions(-) diff --git a/src/client/pythonEnvironments/base/locators/lowLevel/windowsRegistryLocator.ts b/src/client/pythonEnvironments/base/locators/lowLevel/windowsRegistryLocator.ts index a43a93e9f71c..0cf974d308e3 100644 --- a/src/client/pythonEnvironments/base/locators/lowLevel/windowsRegistryLocator.ts +++ b/src/client/pythonEnvironments/base/locators/lowLevel/windowsRegistryLocator.ts @@ -27,27 +27,25 @@ export class WindowsRegistryLocator extends Locator { useWorkerThreads = inExperiment(DiscoveryUsingWorkers.experiment), ): IPythonEnvsIterator { const didUpdate = new EventEmitter | ProgressNotificationEvent>(); - const iterator = iterEnvsIterator(useWorkerThreads, didUpdate); + const iterator = useWorkerThreads ? iterEnvsIterator(didUpdate) : oldIterEnvsIterator(); iterator.onUpdated = didUpdate.event; return iterator; } } +/** + * Windows registry is slow and often not necessary, so notify completion immediately, while still updating lazily as we find stuff. + * To accomplish this, use an empty iterator while lazily firing environments using updates. + */ async function* iterEnvsIterator( - useWorkerThreads: boolean, didUpdate: EventEmitter | ProgressNotificationEvent>, ): IPythonEnvsIterator { - updateLazily(useWorkerThreads, didUpdate).ignoreErrors(); + updateLazily(didUpdate).ignoreErrors(); } -async function updateLazily( - useWorkerThreads: boolean, - didUpdate: EventEmitter | ProgressNotificationEvent>, -) { - // Windows registry is slow and often not necessary, so notify completion while still updating lazily as we find stuff. +async function updateLazily(didUpdate: EventEmitter | ProgressNotificationEvent>) { traceVerbose('Searching for windows registry interpreters'); - console.time('Time taken for windows registry'); - const interpreters = await getRegistryInterpreters(useWorkerThreads); + const interpreters = await getRegistryInterpreters(true); for (const interpreter of interpreters) { try { // Filter out Microsoft Store app directories. We have a store app locator that handles this. @@ -67,5 +65,27 @@ async function updateLazily( } } traceVerbose('Finished searching for windows registry interpreters'); - console.timeEnd('Time taken for windows registry'); +} + +async function* oldIterEnvsIterator(): IPythonEnvsIterator { + const interpreters = await getRegistryInterpreters(false); + for (const interpreter of interpreters) { + try { + // Filter out Microsoft Store app directories. We have a store app locator that handles this. + // The python.exe available in these directories might not be python. It can be a store install + // shortcut that takes you to microsoft store. + if (isMicrosoftStoreDir(interpreter.interpreterPath)) { + continue; + } + const env: BasicEnvInfo = { + kind: PythonEnvKind.OtherGlobal, + executablePath: interpreter.interpreterPath, + source: [PythonEnvSource.WindowsRegistry], + }; + yield env; + } catch (ex) { + traceError(`Failed to process environment: ${interpreter}`, ex); + } + } + traceVerbose('Finished searching for windows registry interpreters'); } From cee63428fe328a3d80e5ca5c186a70af2910c540 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 21 Nov 2023 13:35:25 -0800 Subject: [PATCH 05/20] fix tests --- src/test/interpreters/autoSelection/index.unit.test.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/test/interpreters/autoSelection/index.unit.test.ts b/src/test/interpreters/autoSelection/index.unit.test.ts index d96d590628e0..914c67bb4a65 100644 --- a/src/test/interpreters/autoSelection/index.unit.test.ts +++ b/src/test/interpreters/autoSelection/index.unit.test.ts @@ -14,7 +14,7 @@ import { WorkspaceService } from '../../../client/common/application/workspace'; import { PersistentState, PersistentStateFactory } from '../../../client/common/persistentState'; import { FileSystem } from '../../../client/common/platform/fileSystem'; import { IFileSystem } from '../../../client/common/platform/types'; -import { IPersistentStateFactory, Resource } from '../../../client/common/types'; +import { IExperimentService, IPersistentStateFactory, Resource } from '../../../client/common/types'; import { createDeferred } from '../../../client/common/utils/async'; import { InterpreterAutoSelectionService } from '../../../client/interpreter/autoSelection'; import { InterpreterAutoSelectionProxyService } from '../../../client/interpreter/autoSelection/proxy'; @@ -41,6 +41,7 @@ suite('Interpreters - Auto Selection', () => { let helper: IInterpreterHelper; let proxy: IInterpreterAutoSelectionProxyService; let interpreterService: IInterpreterService; + let experimentService: IExperimentService; let sendTelemetryEventStub: sinon.SinonStub; let telemetryEvents: { eventName: string; properties: Record }[] = []; class InterpreterAutoSelectionServiceTest extends InterpreterAutoSelectionService { @@ -64,6 +65,8 @@ suite('Interpreters - Auto Selection', () => { helper = mock(InterpreterHelper); proxy = mock(InterpreterAutoSelectionProxyService); interpreterService = mock(InterpreterService); + experimentService = mock(IExperimentService); + when(experimentService.inExperimentSync(anything())).thenReturn(false); const interpreterComparer = new EnvironmentTypeComparer(instance(helper)); @@ -75,6 +78,7 @@ suite('Interpreters - Auto Selection', () => { interpreterComparer, instance(proxy), instance(helper), + instance(experimentService), ); when(interpreterService.refreshPromise).thenReturn(undefined); @@ -236,6 +240,7 @@ suite('Interpreters - Auto Selection', () => { interpreterComparer, instance(proxy), instance(helper), + instance(experimentService), ); autoSelectionService.initializeStore = () => Promise.resolve(); @@ -275,6 +280,7 @@ suite('Interpreters - Auto Selection', () => { interpreterComparer, instance(proxy), instance(helper), + instance(experimentService), ); autoSelectionService.initializeStore = () => Promise.resolve(); @@ -309,6 +315,7 @@ suite('Interpreters - Auto Selection', () => { interpreterComparer, instance(proxy), instance(helper), + instance(experimentService), ); autoSelectionService.initializeStore = () => Promise.resolve(); @@ -352,6 +359,7 @@ suite('Interpreters - Auto Selection', () => { interpreterComparer, instance(proxy), instance(helper), + instance(experimentService), ); autoSelectionService.initializeStore = () => Promise.resolve(); From 0a2fb39c573de44e7e329690060c832898ed820e Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 21 Nov 2023 14:18:03 -0800 Subject: [PATCH 06/20] Fix tests --- src/client/interpreter/autoSelection/index.ts | 2 +- .../autoSelection/index.unit.test.ts | 19 ++++++++++++++++++- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/src/client/interpreter/autoSelection/index.ts b/src/client/interpreter/autoSelection/index.ts index 9259a76415d9..4310374fc00f 100644 --- a/src/client/interpreter/autoSelection/index.ts +++ b/src/client/interpreter/autoSelection/index.ts @@ -186,7 +186,7 @@ export class InterpreterAutoSelectionService implements IInterpreterAutoSelectio private getAutoSelectionQueriedOnceState(): IPersistentState { const key = `autoSelectionInterpretersQueriedOnce`; - return this.stateFactory.createWorkspacePersistentState(key, undefined); + return this.stateFactory.createGlobalPersistentState(key, undefined); } /** diff --git a/src/test/interpreters/autoSelection/index.unit.test.ts b/src/test/interpreters/autoSelection/index.unit.test.ts index 914c67bb4a65..6c5473546614 100644 --- a/src/test/interpreters/autoSelection/index.unit.test.ts +++ b/src/test/interpreters/autoSelection/index.unit.test.ts @@ -65,7 +65,7 @@ suite('Interpreters - Auto Selection', () => { helper = mock(InterpreterHelper); proxy = mock(InterpreterAutoSelectionProxyService); interpreterService = mock(InterpreterService); - experimentService = mock(IExperimentService); + experimentService = mock(); when(experimentService.inExperimentSync(anything())).thenReturn(false); const interpreterComparer = new EnvironmentTypeComparer(instance(helper)); @@ -144,6 +144,12 @@ suite('Interpreters - Auto Selection', () => { undefined, ), ).thenReturn(instance(state)); + when( + stateFactory.createGlobalPersistentState( + 'autoSelectionInterpretersQueriedOnce', + undefined, + ), + ).thenReturn(instance(state)); when(workspaceService.getWorkspaceFolderIdentifier(anything(), '')).thenReturn('workspaceIdentifier'); autoSelectionService.onDidChangeAutoSelectedInterpreter(() => { @@ -212,6 +218,13 @@ suite('Interpreters - Auto Selection', () => { test('getInterpreters is called with ignoreCache at true if there is no value set in the workspace persistent state', async () => { const interpreterComparer = new EnvironmentTypeComparer(instance(helper)); + + const globalQueriedState = mock(PersistentState) as PersistentState; + when(globalQueriedState.value).thenReturn(true); + when(stateFactory.createGlobalPersistentState(anyString(), undefined)).thenReturn( + instance(globalQueriedState), + ); + const queryState = mock(PersistentState) as PersistentState; when(queryState.value).thenReturn(undefined); @@ -393,6 +406,10 @@ suite('Interpreters - Auto Selection', () => { when(stateFactory.createWorkspacePersistentState(anyString(), undefined)).thenReturn( instance(queryState), ); + when(queryState.value).thenReturn(undefined); + when(stateFactory.createGlobalPersistentState(anyString(), undefined)).thenReturn( + instance(queryState), + ); let initialize = false; let eventFired = false; From 44dc2e5a33d9df5ac86e93f76f08d8d4ac053fcd Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 21 Nov 2023 16:04:19 -0800 Subject: [PATCH 07/20] Fix up windows path locator --- .../lowLevel/windowsKnownPathsLocator.ts | 22 +++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/src/client/pythonEnvironments/base/locators/lowLevel/windowsKnownPathsLocator.ts b/src/client/pythonEnvironments/base/locators/lowLevel/windowsKnownPathsLocator.ts index b2f5123069b0..5a80f9a08b62 100644 --- a/src/client/pythonEnvironments/base/locators/lowLevel/windowsKnownPathsLocator.ts +++ b/src/client/pythonEnvironments/base/locators/lowLevel/windowsKnownPathsLocator.ts @@ -7,7 +7,7 @@ import { Event } from 'vscode'; import { IDisposable } from '../../../../common/types'; import { getSearchPathEntries } from '../../../../common/utils/exec'; import { Disposables } from '../../../../common/utils/resourceLifecycle'; -import { iterPythonExecutablesInDir, looksLikeBasicGlobalPython } from '../../../common/commonUtils'; +import * as path from 'path'; import { isPyenvShimDir } from '../../../common/environmentManagers/pyenv'; import { isMicrosoftStoreDir } from '../../../common/environmentManagers/microsoftStoreEnv'; import { PythonEnvKind, PythonEnvSource } from '../../info'; @@ -17,6 +17,10 @@ import { getEnvs } from '../../locatorUtils'; import { PythonEnvsChangedEvent } from '../../watcher'; import { DirFilesLocator } from './filesLocator'; import { traceVerbose } from '../../../../logging'; +import { pathExists } from 'fs-extra'; +import { inExperiment } from '../../../common/externalDependencies'; +import { DiscoveryUsingWorkers } from '../../../../common/experiments/groups'; +import { iterPythonExecutablesInDir, looksLikeBasicGlobalPython } from '../../../common/commonUtils'; /** * A locator for Windows locators found under the $PATH env var. @@ -34,6 +38,7 @@ export class WindowsPathEnvVarLocator implements ILocator, IDispos private readonly disposables = new Disposables(); constructor() { + const inExp = inExperiment(DiscoveryUsingWorkers.experiment); const dirLocators: (ILocator & IDisposable)[] = getSearchPathEntries() .filter( (dirname) => @@ -48,7 +53,7 @@ export class WindowsPathEnvVarLocator implements ILocator, IDispos !isMicrosoftStoreDir(dirname) && !isPyenvShimDir(dirname), ) // Build a locator for each directory. - .map((dirname) => getDirFilesLocator(dirname, PythonEnvKind.System, [PythonEnvSource.PathEnvVar])); + .map((dirname) => getDirFilesLocator(dirname, PythonEnvKind.System, [PythonEnvSource.PathEnvVar], inExp)); this.disposables.push(...dirLocators); this.locators = new Locators(dirLocators); this.onChanged = this.locators.onChanged; @@ -74,7 +79,7 @@ export class WindowsPathEnvVarLocator implements ILocator, IDispos } } -async function* getExecutables(dirname: string): AsyncIterableIterator { +async function* oldGetExecutables(dirname: string): AsyncIterableIterator { for await (const entry of iterPythonExecutablesInDir(dirname)) { if (await looksLikeBasicGlobalPython(entry)) { yield entry.filename; @@ -82,17 +87,26 @@ async function* getExecutables(dirname: string): AsyncIterableIterator { } } +async function* getExecutables(dirname: string): AsyncIterableIterator { + const executable = path.join(dirname, 'python.exe'); + if (await pathExists(executable)) { + yield executable; + } +} + function getDirFilesLocator( // These are passed through to DirFilesLocator. dirname: string, kind: PythonEnvKind, source?: PythonEnvSource[], + inExp?: boolean, ): ILocator & IDisposable { // For now we do not bother using a locator that watches for changes // in the directory. If we did then we would use // `DirFilesWatchingLocator`, but only if not \\windows\system32 and // the `isDirWatchable()` (from fsWatchingLocator.ts) returns true. - const locator = new DirFilesLocator(dirname, kind, getExecutables, source); + const executableFunc = inExp ? getExecutables : oldGetExecutables; + const locator = new DirFilesLocator(dirname, kind, executableFunc, source); const dispose = async () => undefined; // Really we should be checking for symlinks or something more From e3afdc715ac5c917d435f88ed1218559cc94a431 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Tue, 21 Nov 2023 19:10:23 -0800 Subject: [PATCH 08/20] hEY --- .../base/locators/lowLevel/windowsKnownPathsLocator.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/client/pythonEnvironments/base/locators/lowLevel/windowsKnownPathsLocator.ts b/src/client/pythonEnvironments/base/locators/lowLevel/windowsKnownPathsLocator.ts index 5a80f9a08b62..850f97774227 100644 --- a/src/client/pythonEnvironments/base/locators/lowLevel/windowsKnownPathsLocator.ts +++ b/src/client/pythonEnvironments/base/locators/lowLevel/windowsKnownPathsLocator.ts @@ -17,8 +17,7 @@ import { getEnvs } from '../../locatorUtils'; import { PythonEnvsChangedEvent } from '../../watcher'; import { DirFilesLocator } from './filesLocator'; import { traceVerbose } from '../../../../logging'; -import { pathExists } from 'fs-extra'; -import { inExperiment } from '../../../common/externalDependencies'; +import { inExperiment, pathExists } from '../../../common/externalDependencies'; import { DiscoveryUsingWorkers } from '../../../../common/experiments/groups'; import { iterPythonExecutablesInDir, looksLikeBasicGlobalPython } from '../../../common/commonUtils'; From 87e9709f3144156f4b2e2c3c966c17603c606a3d Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Wed, 22 Nov 2023 12:33:26 -0800 Subject: [PATCH 09/20] Add telemetry --- src/client/activation/activationManager.ts | 9 +++++---- src/client/activation/types.ts | 5 +++-- src/client/extension.ts | 3 ++- src/client/extensionActivation.ts | 8 +++++--- src/client/languageServer/watcher.ts | 19 +++++++++++++++---- .../lowLevel/windowsKnownPathsLocator.ts | 2 +- src/client/telemetry/constants.ts | 1 + src/client/telemetry/index.ts | 9 +++++++++ 8 files changed, 41 insertions(+), 15 deletions(-) diff --git a/src/client/activation/activationManager.ts b/src/client/activation/activationManager.ts index 763ce1ae8819..9e97c5c48857 100644 --- a/src/client/activation/activationManager.ts +++ b/src/client/activation/activationManager.ts @@ -11,6 +11,7 @@ import { PYTHON_LANGUAGE } from '../common/constants'; import { IFileSystem } from '../common/platform/types'; import { IDisposable, IInterpreterPathService, Resource } from '../common/types'; import { Deferred } from '../common/utils/async'; +import { StopWatch } from '../common/utils/stopWatch'; import { IInterpreterAutoSelectionService } from '../interpreter/autoSelection/types'; import { traceDecoratorError } from '../logging'; import { sendActivationTelemetry } from '../telemetry/envFileTelemetry'; @@ -69,7 +70,7 @@ export class ExtensionActivationManager implements IExtensionActivationManager { } } - public async activate(): Promise { + public async activate(startupStopWatch: StopWatch): Promise { this.filterServices(); await this.initialize(); @@ -77,12 +78,12 @@ export class ExtensionActivationManager implements IExtensionActivationManager { await Promise.all([ ...this.singleActivationServices.map((item) => item.activate()), - this.activateWorkspace(this.activeResourceService.getActiveResource()), + this.activateWorkspace(this.activeResourceService.getActiveResource(), startupStopWatch), ]); } @traceDecoratorError('Failed to activate a workspace') - public async activateWorkspace(resource: Resource): Promise { + public async activateWorkspace(resource: Resource, startupStopWatch?: StopWatch): Promise { const folder = this.workspaceService.getWorkspaceFolder(resource); resource = folder ? folder.uri : undefined; const key = this.getWorkspaceKey(resource); @@ -97,7 +98,7 @@ export class ExtensionActivationManager implements IExtensionActivationManager { await this.interpreterPathService.copyOldInterpreterStorageValuesToNew(resource); } await sendActivationTelemetry(this.fileSystem, this.workspaceService, resource); - await Promise.all(this.activationServices.map((item) => item.activate(resource))); + await Promise.all(this.activationServices.map((item) => item.activate(resource, startupStopWatch))); await this.appDiagnostics.performPreStartupHealthCheck(resource); } diff --git a/src/client/activation/types.ts b/src/client/activation/types.ts index 2a177bb570b8..3a949e480d4b 100644 --- a/src/client/activation/types.ts +++ b/src/client/activation/types.ts @@ -6,6 +6,7 @@ import { Event } from 'vscode'; import { LanguageClient, LanguageClientOptions } from 'vscode-languageclient/node'; import type { IDisposable, ILogOutputChannel, Resource } from '../common/types'; +import { StopWatch } from '../common/utils/stopWatch'; import { PythonEnvironment } from '../pythonEnvironments/info'; export const IExtensionActivationManager = Symbol('IExtensionActivationManager'); @@ -23,7 +24,7 @@ export interface IExtensionActivationManager extends IDisposable { * @returns {Promise} * @memberof IExtensionActivationManager */ - activate(): Promise; + activate(startupStopWatch: StopWatch): Promise; /** * Method invoked when a workspace is loaded. * This is where we place initialization scripts for each workspace. @@ -47,7 +48,7 @@ export const IExtensionActivationService = Symbol('IExtensionActivationService') */ export interface IExtensionActivationService { supportedWorkspaceTypes: { untrustedWorkspace: boolean; virtualWorkspace: boolean }; - activate(resource: Resource): Promise; + activate(resource: Resource, startupStopWatch?: StopWatch): Promise; } export enum LanguageServerType { diff --git a/src/client/extension.ts b/src/client/extension.ts index a4f5bd5dbfd0..a4c31d91eb3b 100644 --- a/src/client/extension.ts +++ b/src/client/extension.ts @@ -110,6 +110,7 @@ async function activateUnsafe( const activationDeferred = createDeferred(); displayProgress(activationDeferred.promise); startupDurations.startActivateTime = startupStopWatch.elapsedTime; + const activationStopWatch = new StopWatch(); //=============================================== // activation starts here @@ -127,7 +128,7 @@ async function activateUnsafe( const components = await initializeComponents(ext); // Then we finish activating. - const componentsActivated = await activateComponents(ext, components); + const componentsActivated = await activateComponents(ext, components, activationStopWatch); activateFeatures(ext, components); const nonBlocking = componentsActivated.map((r) => r.fullyReady); diff --git a/src/client/extensionActivation.ts b/src/client/extensionActivation.ts index cd9f99d400db..91497ea3ccdc 100644 --- a/src/client/extensionActivation.ts +++ b/src/client/extensionActivation.ts @@ -51,11 +51,13 @@ import { registerCreateEnvironmentTriggers } from './pythonEnvironments/creation import { initializePersistentStateForTriggers } from './common/persistentState'; import { logAndNotifyOnLegacySettings } from './logging/settingLogs'; import { DebuggerTypeName } from './debugger/constants'; +import { StopWatch } from './common/utils/stopWatch'; export async function activateComponents( // `ext` is passed to any extra activation funcs. ext: ExtensionState, components: Components, + startupStopWatch: StopWatch, ): Promise { // Note that each activation returns a promise that resolves // when that activation completes. However, it might have started @@ -73,7 +75,7 @@ export async function activateComponents( // activate them in parallel with the other components. // https://github.com/microsoft/vscode-python/issues/15380 // These will go away eventually once everything is refactored into components. - const legacyActivationResult = await activateLegacy(ext); + const legacyActivationResult = await activateLegacy(ext, startupStopWatch); const workspaceService = new WorkspaceService(); if (!workspaceService.isTrusted) { return [legacyActivationResult]; @@ -105,7 +107,7 @@ export function activateFeatures(ext: ExtensionState, _components: Components): // init and activation: move them to activateComponents(). // See https://github.com/microsoft/vscode-python/issues/10454. -async function activateLegacy(ext: ExtensionState): Promise { +async function activateLegacy(ext: ExtensionState, startupStopWatch: StopWatch): Promise { const { legacyIOC } = ext; const { serviceManager, serviceContainer } = legacyIOC; @@ -183,7 +185,7 @@ async function activateLegacy(ext: ExtensionState): Promise { const manager = serviceContainer.get(IExtensionActivationManager); disposables.push(manager); - const activationPromise = manager.activate(); + const activationPromise = manager.activate(startupStopWatch); return { fullyReady: activationPromise }; } diff --git a/src/client/languageServer/watcher.ts b/src/client/languageServer/watcher.ts index d3eccb71144c..72f7514c5676 100644 --- a/src/client/languageServer/watcher.ts +++ b/src/client/languageServer/watcher.ts @@ -29,6 +29,7 @@ import { PylanceLSExtensionManager } from './pylanceLSExtensionManager'; import { ILanguageServerExtensionManager, ILanguageServerWatcher } from './types'; import { sendTelemetryEvent } from '../telemetry'; import { EventName } from '../telemetry/constants'; +import { StopWatch } from '../common/utils/stopWatch'; @injectable() /** @@ -73,14 +74,18 @@ export class LanguageServerWatcher implements IExtensionActivationService, ILang // IExtensionActivationService - public async activate(resource?: Resource): Promise { + public async activate(resource?: Resource, startupStopWatch?: StopWatch): Promise { this.register(); - await this.startLanguageServer(this.languageServerType, resource); + await this.startLanguageServer(this.languageServerType, resource, startupStopWatch); } // ILanguageServerWatcher - public async startLanguageServer(languageServerType: LanguageServerType, resource?: Resource): Promise { - await this.startAndGetLanguageServer(languageServerType, resource); + public async startLanguageServer( + languageServerType: LanguageServerType, + resource?: Resource, + startupStopWatch?: StopWatch, + ): Promise { + await this.startAndGetLanguageServer(languageServerType, resource, startupStopWatch); } public register(): void { @@ -124,6 +129,7 @@ export class LanguageServerWatcher implements IExtensionActivationService, ILang private async startAndGetLanguageServer( languageServerType: LanguageServerType, resource?: Resource, + startupStopWatch?: StopWatch, ): Promise { const lsResource = this.getWorkspaceUri(resource); const currentInterpreter = this.workspaceInterpreters.get(lsResource.fsPath); @@ -170,6 +176,11 @@ export class LanguageServerWatcher implements IExtensionActivationService, ILang if (languageServerExtensionManager.canStartLanguageServer(interpreter)) { // Start the language server. + if (startupStopWatch) { + console.log('Time taken to trigger LS', startupStopWatch.elapsedTime); + // It means that startup is triggering this code, track time it takes since startup to activate this code. + sendTelemetryEvent(EventName.LANGUAGE_SERVER_TRIGGER_DURATION, startupStopWatch.elapsedTime); + } await languageServerExtensionManager.startLanguageServer(lsResource, interpreter); logStartup(languageServerType, lsResource); diff --git a/src/client/pythonEnvironments/base/locators/lowLevel/windowsKnownPathsLocator.ts b/src/client/pythonEnvironments/base/locators/lowLevel/windowsKnownPathsLocator.ts index 850f97774227..d158d1da156c 100644 --- a/src/client/pythonEnvironments/base/locators/lowLevel/windowsKnownPathsLocator.ts +++ b/src/client/pythonEnvironments/base/locators/lowLevel/windowsKnownPathsLocator.ts @@ -4,10 +4,10 @@ /* eslint-disable max-classes-per-file */ import { Event } from 'vscode'; +import * as path from 'path'; import { IDisposable } from '../../../../common/types'; import { getSearchPathEntries } from '../../../../common/utils/exec'; import { Disposables } from '../../../../common/utils/resourceLifecycle'; -import * as path from 'path'; import { isPyenvShimDir } from '../../../common/environmentManagers/pyenv'; import { isMicrosoftStoreDir } from '../../../common/environmentManagers/microsoftStoreEnv'; import { PythonEnvKind, PythonEnvSource } from '../../info'; diff --git a/src/client/telemetry/constants.ts b/src/client/telemetry/constants.ts index 072537619224..3321653c3bd7 100644 --- a/src/client/telemetry/constants.ts +++ b/src/client/telemetry/constants.ts @@ -64,6 +64,7 @@ export enum EventName { EXTENSION_SURVEY_PROMPT = 'EXTENSION_SURVEY_PROMPT', LANGUAGE_SERVER_ENABLED = 'LANGUAGE_SERVER.ENABLED', + LANGUAGE_SERVER_TRIGGER_DURATION = 'LANGUAGE_SERVER.TRIGGER_DURATION', LANGUAGE_SERVER_STARTUP = 'LANGUAGE_SERVER.STARTUP', LANGUAGE_SERVER_READY = 'LANGUAGE_SERVER.READY', LANGUAGE_SERVER_TELEMETRY = 'LANGUAGE_SERVER.EVENT', diff --git a/src/client/telemetry/index.ts b/src/client/telemetry/index.ts index 42a73fb06e07..50464414c2ed 100644 --- a/src/client/telemetry/index.ts +++ b/src/client/telemetry/index.ts @@ -1325,6 +1325,15 @@ export interface IEventNamePropertyMapping { [EventName.LANGUAGE_SERVER_READY]: { lsVersion?: string; }; + /** + * Track how long it takes to trigger language server activation code, after Python extension starts activating. + */ + /* __GDPR__ + "language_server_trigger_duration" : { + "duration" : { "classification": "SystemMetaData", "purpose": "FeatureInsight", "owner": "karrtikr" } + } + */ + [EventName.LANGUAGE_SERVER_TRIGGER_DURATION]: unknown; /** * Telemetry event sent when starting Node.js server */ From c554561eb79d6b62602a21d1258502436886c888 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Wed, 22 Nov 2023 13:14:16 -0800 Subject: [PATCH 10/20] sad --- src/client/languageServer/watcher.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/client/languageServer/watcher.ts b/src/client/languageServer/watcher.ts index 72f7514c5676..9f12dc89df0e 100644 --- a/src/client/languageServer/watcher.ts +++ b/src/client/languageServer/watcher.ts @@ -177,7 +177,6 @@ export class LanguageServerWatcher implements IExtensionActivationService, ILang if (languageServerExtensionManager.canStartLanguageServer(interpreter)) { // Start the language server. if (startupStopWatch) { - console.log('Time taken to trigger LS', startupStopWatch.elapsedTime); // It means that startup is triggering this code, track time it takes since startup to activate this code. sendTelemetryEvent(EventName.LANGUAGE_SERVER_TRIGGER_DURATION, startupStopWatch.elapsedTime); } From 0bec32e1377614775a0eb862fbbf23831ea4a276 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Mon, 22 Jan 2024 04:44:16 +0000 Subject: [PATCH 11/20] fix some unit tests --- .../pythonEnvironments/base/locatorUtils.ts | 2 +- .../composite/envsCollectionService.ts | 2 +- .../base/locators/composite/envsReducer.ts | 4 +-- src/test/pythonEnvironments/base/common.ts | 2 +- .../composite/envsReducer.unit.test.ts | 25 ------------------- 5 files changed, 5 insertions(+), 30 deletions(-) diff --git a/src/client/pythonEnvironments/base/locatorUtils.ts b/src/client/pythonEnvironments/base/locatorUtils.ts index dcbf2b1b5877..6af8c0ee1b69 100644 --- a/src/client/pythonEnvironments/base/locatorUtils.ts +++ b/src/client/pythonEnvironments/base/locatorUtils.ts @@ -85,7 +85,7 @@ export async function getEnvs(iterator: IPythonEnvsIterator( } updatesDone.resolve(); listener.dispose(); - } else if (event.index) { + } else if (event.index !== undefined) { const { index, update } = event; // We don't worry about if envs[index] is set already. envs[index] = update; diff --git a/src/test/pythonEnvironments/base/locators/composite/envsReducer.unit.test.ts b/src/test/pythonEnvironments/base/locators/composite/envsReducer.unit.test.ts index 592586118d14..8724c9a903b5 100644 --- a/src/test/pythonEnvironments/base/locators/composite/envsReducer.unit.test.ts +++ b/src/test/pythonEnvironments/base/locators/composite/envsReducer.unit.test.ts @@ -65,31 +65,6 @@ suite('Python envs locator - Environments Reducer', () => { assertBasicEnvsEqual(envs, expected); }); - test('Updates to environments from the incoming iterator replaces the previous info', async () => { - // Arrange - const env = createBasicEnv(PythonEnvKind.Poetry, path.join('path', 'to', 'exec1')); - const updatedEnv = createBasicEnv(PythonEnvKind.Venv, path.join('path', 'to', 'exec1')); - const envsReturnedByParentLocator = [env]; - const didUpdate = new EventEmitter | ProgressNotificationEvent>(); - const parentLocator = new SimpleLocator(envsReturnedByParentLocator, { - onUpdated: didUpdate.event, - }); - const reducer = new PythonEnvsReducer(parentLocator); - - // Act - const iterator = reducer.iterEnvs(); - - const iteratorUpdateCallback = () => { - didUpdate.fire({ index: 0, old: env, update: updatedEnv }); - didUpdate.fire({ stage: ProgressReportStage.discoveryFinished }); // It is essential for the incoming iterator to fire "null" event signifying it's done - }; - const envs = await getEnvsWithUpdates(iterator, iteratorUpdateCallback); - - // Assert - assertBasicEnvsEqual(envs, [updatedEnv]); - didUpdate.dispose(); - }); - test('Ensure progress updates are emitted correctly', async () => { // Arrange const env1 = createBasicEnv(PythonEnvKind.Venv, path.join('path', 'to', 'exec1')); From 8e8f109adf81d5a58f668658c4d4c19901f16249 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Mon, 22 Jan 2024 16:28:07 +0000 Subject: [PATCH 12/20] fix tests --- .../base/locators/lowLevel/windowsRegistryLocator.ts | 4 +++- .../locators/lowLevel/windowsRegistryLocator.unit.test.ts | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/client/pythonEnvironments/base/locators/lowLevel/windowsRegistryLocator.ts b/src/client/pythonEnvironments/base/locators/lowLevel/windowsRegistryLocator.ts index 0cf974d308e3..725407976a74 100644 --- a/src/client/pythonEnvironments/base/locators/lowLevel/windowsRegistryLocator.ts +++ b/src/client/pythonEnvironments/base/locators/lowLevel/windowsRegistryLocator.ts @@ -28,7 +28,9 @@ export class WindowsRegistryLocator extends Locator { ): IPythonEnvsIterator { const didUpdate = new EventEmitter | ProgressNotificationEvent>(); const iterator = useWorkerThreads ? iterEnvsIterator(didUpdate) : oldIterEnvsIterator(); - iterator.onUpdated = didUpdate.event; + if (useWorkerThreads) { + iterator.onUpdated = didUpdate.event; + } return iterator; } } diff --git a/src/test/pythonEnvironments/base/locators/lowLevel/windowsRegistryLocator.unit.test.ts b/src/test/pythonEnvironments/base/locators/lowLevel/windowsRegistryLocator.unit.test.ts index a69a643f52d4..69d31cf59241 100644 --- a/src/test/pythonEnvironments/base/locators/lowLevel/windowsRegistryLocator.unit.test.ts +++ b/src/test/pythonEnvironments/base/locators/lowLevel/windowsRegistryLocator.unit.test.ts @@ -222,7 +222,7 @@ suite('Windows Registry', () => { createBasicEnv(PythonEnvKind.OtherGlobal, path.join(regTestRoot, 'python38', 'python.exe')), ].map((e) => ({ ...e, source: [PythonEnvSource.WindowsRegistry] })); - const iterator = locator.iterEnvs(); + const iterator = locator.iterEnvs(undefined, false); const actualEnvs = await getEnvs(iterator); assertBasicEnvsEqual(actualEnvs, expectedEnvs); From 6c3b1559d6d0dd8a84cf356b57d61b16c5a4946a Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Mon, 29 Jan 2024 02:39:35 +0000 Subject: [PATCH 13/20] fix compile errors --- .../base/locators/composite/envsReducer.unit.test.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/test/pythonEnvironments/base/locators/composite/envsReducer.unit.test.ts b/src/test/pythonEnvironments/base/locators/composite/envsReducer.unit.test.ts index 8724c9a903b5..a7f44abbbf94 100644 --- a/src/test/pythonEnvironments/base/locators/composite/envsReducer.unit.test.ts +++ b/src/test/pythonEnvironments/base/locators/composite/envsReducer.unit.test.ts @@ -3,16 +3,13 @@ import { assert, expect } from 'chai'; import * as path from 'path'; -import { EventEmitter } from 'vscode'; import { PythonEnvKind, PythonEnvSource } from '../../../../../client/pythonEnvironments/base/info'; import { PythonEnvsReducer } from '../../../../../client/pythonEnvironments/base/locators/composite/envsReducer'; import { PythonEnvsChangedEvent } from '../../../../../client/pythonEnvironments/base/watcher'; import { assertBasicEnvsEqual } from '../envTestUtils'; import { createBasicEnv, getEnvs, getEnvsWithUpdates, SimpleLocator } from '../../common'; import { - PythonEnvUpdatedEvent, BasicEnvInfo, - ProgressNotificationEvent, ProgressReportStage, isProgressEvent, } from '../../../../../client/pythonEnvironments/base/locator'; From daad39a84fb1c862a51f2c90ec1ebdb4df052a57 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Mon, 29 Jan 2024 03:19:22 +0000 Subject: [PATCH 14/20] Run tests when experiment is on --- .../windowsKnownPathsLocator.functional.test.ts | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/test/pythonEnvironments/discovery/locators/windowsKnownPathsLocator.functional.test.ts b/src/test/pythonEnvironments/discovery/locators/windowsKnownPathsLocator.functional.test.ts index cd7715a1bf75..e8228f96a066 100644 --- a/src/test/pythonEnvironments/discovery/locators/windowsKnownPathsLocator.functional.test.ts +++ b/src/test/pythonEnvironments/discovery/locators/windowsKnownPathsLocator.functional.test.ts @@ -3,6 +3,7 @@ import { assert } from 'chai'; import * as path from 'path'; +import * as sinon from 'sinon'; import { getOSType, OSType } from '../../../../client/common/utils/platform'; import { PythonEnvKind, PythonEnvSource } from '../../../../client/pythonEnvironments/base/info'; import { BasicEnvInfo, PythonLocatorQuery } from '../../../../client/pythonEnvironments/base/locator'; @@ -10,6 +11,7 @@ import { WindowsPathEnvVarLocator } from '../../../../client/pythonEnvironments/ import { ensureFSTree } from '../../../utils/fs'; import { assertBasicEnvsEqual } from '../../base/locators/envTestUtils'; import { createBasicEnv, getEnvs } from '../../base/common'; +import * as externalDependencies from '../../../../client/pythonEnvironments/common/externalDependencies'; const IS_WINDOWS = getOSType() === OSType.Windows; @@ -66,22 +68,20 @@ suite('Python envs locator - WindowsPathEnvVarLocator', async () => { `.trimEnd(); - suiteSetup(async function () { + suiteSetup(async () => { + await ensureFSTree(dataTree, __dirname); + }); + setup(async function () { if (!IS_WINDOWS) { if (!process.env.PVSC_TEST_FORCE) { this.skip(); } // eslint-disable-next-line global-require - const sinon = require('sinon'); - // eslint-disable-next-line global-require const platformAPI = require('../../../../../client/common/utils/platform'); const stub = sinon.stub(platformAPI, 'getOSType'); stub.returns(OSType.Windows); } - - await ensureFSTree(dataTree, __dirname); - }); - setup(() => { + sinon.stub(externalDependencies, 'inExperiment').returns(true); cleanUps = []; const oldSearchPath = process.env[ENV_VAR]; @@ -97,6 +97,7 @@ suite('Python envs locator - WindowsPathEnvVarLocator', async () => { console.log(err); } }); + sinon.restore(); }); function getActiveLocator(...roots: string[]): WindowsPathEnvVarLocator { From 9feabf9c33f031085457171be5b3688705d612b9 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Mon, 29 Jan 2024 03:23:51 +0000 Subject: [PATCH 15/20] s --- .../windowsKnownPathsLocator.functional.test.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/test/pythonEnvironments/discovery/locators/windowsKnownPathsLocator.functional.test.ts b/src/test/pythonEnvironments/discovery/locators/windowsKnownPathsLocator.functional.test.ts index e8228f96a066..ebebf2a8220e 100644 --- a/src/test/pythonEnvironments/discovery/locators/windowsKnownPathsLocator.functional.test.ts +++ b/src/test/pythonEnvironments/discovery/locators/windowsKnownPathsLocator.functional.test.ts @@ -68,14 +68,16 @@ suite('Python envs locator - WindowsPathEnvVarLocator', async () => { `.trimEnd(); - suiteSetup(async () => { - await ensureFSTree(dataTree, __dirname); - }); - setup(async function () { + suiteSetup(async function () { if (!IS_WINDOWS) { if (!process.env.PVSC_TEST_FORCE) { this.skip(); } + } + await ensureFSTree(dataTree, __dirname); + }); + setup(async () => { + if (!IS_WINDOWS) { // eslint-disable-next-line global-require const platformAPI = require('../../../../../client/common/utils/platform'); const stub = sinon.stub(platformAPI, 'getOSType'); From 6b8f19f36c17d5872584f4592dd8dfea6abb8b6d Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Mon, 29 Jan 2024 14:20:12 +0000 Subject: [PATCH 16/20] fix tests --- src/client/pythonEnvironments/base/locatorUtils.ts | 2 ++ .../base/locators/lowLevel/windowsRegistryLocator.ts | 2 ++ .../locators/lowLevel/windowsRegistryLocator.unit.test.ts | 6 +++--- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/client/pythonEnvironments/base/locatorUtils.ts b/src/client/pythonEnvironments/base/locatorUtils.ts index 6af8c0ee1b69..faeaa84bedf4 100644 --- a/src/client/pythonEnvironments/base/locatorUtils.ts +++ b/src/client/pythonEnvironments/base/locatorUtils.ts @@ -95,6 +95,8 @@ export async function getEnvs(iterator: IPythonEnvsIterator { createBasicEnv(PythonEnvKind.OtherGlobal, path.join(regTestRoot, 'python38', 'python.exe')), ].map((e) => ({ ...e, source: [PythonEnvSource.WindowsRegistry] })); - const iterator = locator.iterEnvs(undefined, false); + const iterator = locator.iterEnvs(undefined, true); const actualEnvs = await getEnvs(iterator); assertBasicEnvsEqual(actualEnvs, expectedEnvs); @@ -233,7 +233,7 @@ suite('Windows Registry', () => { throw Error(); }); - const iterator = locator.iterEnvs(); + const iterator = locator.iterEnvs(undefined, true); const actualEnvs = await getEnvs(iterator); assert.deepStrictEqual(actualEnvs, []); @@ -252,7 +252,7 @@ suite('Windows Registry', () => { createBasicEnv(PythonEnvKind.OtherGlobal, path.join(regTestRoot, 'python38', 'python.exe')), ].map((e) => ({ ...e, source: [PythonEnvSource.WindowsRegistry] })); - const iterator = locator.iterEnvs(); + const iterator = locator.iterEnvs(undefined, true); const actualEnvs = await getEnvs(iterator); assertBasicEnvsEqual(actualEnvs, expectedEnvs); From c86c03e89341b05465c09946ff45bb46dade8870 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Thu, 1 Feb 2024 17:42:29 +0000 Subject: [PATCH 17/20] Add comments --- .../pythonEnvironments/base/locators/composite/envsReducer.ts | 3 ++- .../pythonEnvironments/base/locators/composite/envsResolver.ts | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/client/pythonEnvironments/base/locators/composite/envsReducer.ts b/src/client/pythonEnvironments/base/locators/composite/envsReducer.ts index 1c3fa3e8336b..8a342a0386ef 100644 --- a/src/client/pythonEnvironments/base/locators/composite/envsReducer.ts +++ b/src/client/pythonEnvironments/base/locators/composite/envsReducer.ts @@ -55,6 +55,8 @@ async function* iterEnvsIterator( if (isProgressEvent(event)) { if (event.stage === ProgressReportStage.discoveryFinished) { state.done = true; + // For super slow locators such as Windows registry, we expect updates even after discovery + // is "officially" finished, hence do not dispose listeners. // listener.dispose(); } else { didUpdate.fire(event); @@ -122,7 +124,6 @@ function checkIfFinishedAndNotify( ) { if (state.done && state.pending === 0) { didUpdate.fire({ stage: ProgressReportStage.discoveryFinished }); - // didUpdate.dispose(); traceVerbose(`Finished with environment reducer`); state.done = false; // No need to notify again. } diff --git a/src/client/pythonEnvironments/base/locators/composite/envsResolver.ts b/src/client/pythonEnvironments/base/locators/composite/envsResolver.ts index ad8af59883da..752f5778c73c 100644 --- a/src/client/pythonEnvironments/base/locators/composite/envsResolver.ts +++ b/src/client/pythonEnvironments/base/locators/composite/envsResolver.ts @@ -87,6 +87,8 @@ export class PythonEnvsResolver implements IResolvingLocator { if (event.stage === ProgressReportStage.discoveryFinished) { didUpdate.fire({ stage: ProgressReportStage.allPathsDiscovered }); state.done = true; + // For super slow locators such as Windows registry, we expect updates even after discovery + // is "officially" finished, hence do not dispose listeners. // listener.dispose(); } else { didUpdate.fire(event); @@ -172,7 +174,6 @@ function checkIfFinishedAndNotify( ) { if (state.done && state.pending === 0) { didUpdate.fire({ stage: ProgressReportStage.discoveryFinished }); - // didUpdate.dispose(); traceVerbose(`Finished with environment resolver`); } } From fcd547e124ef84cd2184dc0c2662bbec8335135d Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Mon, 5 Feb 2024 18:53:00 +0530 Subject: [PATCH 18/20] Fix worker threads conda --- .../common/environmentManagers/conda.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/client/pythonEnvironments/common/environmentManagers/conda.ts b/src/client/pythonEnvironments/common/environmentManagers/conda.ts index bc98e57829e9..5c2f1b187ff8 100644 --- a/src/client/pythonEnvironments/common/environmentManagers/conda.ts +++ b/src/client/pythonEnvironments/common/environmentManagers/conda.ts @@ -10,6 +10,7 @@ import { readFile, onDidChangePythonSetting, exec, + inExperiment, } from '../externalDependencies'; import { PythonVersion, UNKNOWN_PYTHON_VERSION } from '../../base/info'; @@ -24,6 +25,7 @@ import { OUTPUT_MARKER_SCRIPT } from '../../../common/process/internal/scripts'; import { splitLines } from '../../../common/stringUtils'; import { SpawnOptions } from '../../../common/process/types'; import { sleep } from '../../../common/utils/async'; +import { DiscoveryUsingWorkers } from '../../../common/experiments/groups'; export const AnacondaCompanyName = 'Anaconda, Inc.'; export const CONDAPATH_SETTING_KEY = 'condaPath'; @@ -269,7 +271,7 @@ export class Conda { readonly command: string, shellCommand?: string, private readonly shellPath?: string, - private readonly useWorkerThreads = true, + private readonly useWorkerThreads = inExperiment(DiscoveryUsingWorkers.experiment), ) { this.shellCommand = shellCommand ?? command; onDidChangePythonSetting(CONDAPATH_SETTING_KEY, () => { @@ -290,7 +292,10 @@ export class Conda { * * @return A Conda instance corresponding to the binary, if successful; otherwise, undefined. */ - private static async locate(shellPath?: string, useWorkerThreads = true): Promise { + private static async locate( + shellPath?: string, + useWorkerThreads = inExperiment(DiscoveryUsingWorkers.experiment), + ): Promise { traceVerbose(`Searching for conda.`); const home = getUserHomeDir(); let customCondaPath: string | undefined = 'conda'; From e1d105ddf444cf7fb06d2c086f002c05f0be3e6f Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Mon, 5 Feb 2024 19:09:59 +0530 Subject: [PATCH 19/20] fix tests --- .../common/environmentManagers/conda.ts | 21 ++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/src/client/pythonEnvironments/common/environmentManagers/conda.ts b/src/client/pythonEnvironments/common/environmentManagers/conda.ts index 5c2f1b187ff8..9c9c7ad03be8 100644 --- a/src/client/pythonEnvironments/common/environmentManagers/conda.ts +++ b/src/client/pythonEnvironments/common/environmentManagers/conda.ts @@ -271,8 +271,15 @@ export class Conda { readonly command: string, shellCommand?: string, private readonly shellPath?: string, - private readonly useWorkerThreads = inExperiment(DiscoveryUsingWorkers.experiment), + private readonly useWorkerThreads?: boolean, ) { + if (this.useWorkerThreads === undefined) { + try { + this.useWorkerThreads = inExperiment(DiscoveryUsingWorkers.experiment); + } catch { + this.useWorkerThreads = false; // Temporarily support for legacy tests + } + } this.shellCommand = shellCommand ?? command; onDidChangePythonSetting(CONDAPATH_SETTING_KEY, () => { Conda.condaPromise = new Map>(); @@ -292,10 +299,14 @@ export class Conda { * * @return A Conda instance corresponding to the binary, if successful; otherwise, undefined. */ - private static async locate( - shellPath?: string, - useWorkerThreads = inExperiment(DiscoveryUsingWorkers.experiment), - ): Promise { + private static async locate(shellPath?: string, useWorkerThreads?: boolean): Promise { + if (useWorkerThreads === undefined) { + try { + useWorkerThreads = inExperiment(DiscoveryUsingWorkers.experiment); + } catch { + useWorkerThreads = false; // Temporarily support for legacy tests + } + } traceVerbose(`Searching for conda.`); const home = getUserHomeDir(); let customCondaPath: string | undefined = 'conda'; From f72254ba54859b389f407f0b9e317bf26157d666 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Mon, 5 Feb 2024 19:13:42 +0530 Subject: [PATCH 20/20] fix lint --- .../pythonEnvironments/common/environmentManagers/conda.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/client/pythonEnvironments/common/environmentManagers/conda.ts b/src/client/pythonEnvironments/common/environmentManagers/conda.ts index 9c9c7ad03be8..1cb2e490aef8 100644 --- a/src/client/pythonEnvironments/common/environmentManagers/conda.ts +++ b/src/client/pythonEnvironments/common/environmentManagers/conda.ts @@ -299,8 +299,9 @@ export class Conda { * * @return A Conda instance corresponding to the binary, if successful; otherwise, undefined. */ - private static async locate(shellPath?: string, useWorkerThreads?: boolean): Promise { - if (useWorkerThreads === undefined) { + private static async locate(shellPath?: string, useWorkerThread?: boolean): Promise { + let useWorkerThreads: boolean; + if (useWorkerThread === undefined) { try { useWorkerThreads = inExperiment(DiscoveryUsingWorkers.experiment); } catch { @@ -411,7 +412,7 @@ export class Conda { // Probe the candidates, and pick the first one that exists and does what we need. for await (const condaPath of getCandidates()) { traceVerbose(`Probing conda binary: ${condaPath}`); - let conda = new Conda(condaPath, undefined, shellPath, useWorkerThreads); + let conda = new Conda(condaPath, undefined, shellPath); try { await conda.getInfo(); if (getOSType() === OSType.Windows && (isTestExecution() || condaPath !== customCondaPath)) {