From dbb020e6d90aa9669c1613e61551225b787568da Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Fri, 1 Mar 2024 14:45:41 +0530 Subject: [PATCH 1/8] Make `known` API faster --- src/client/environmentApi.ts | 34 ++++++++++++++++++++------ src/client/environmentKnownCache.ts | 37 +++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 7 deletions(-) create mode 100644 src/client/environmentKnownCache.ts diff --git a/src/client/environmentApi.ts b/src/client/environmentApi.ts index da6a132b2b44..bb9c00335b47 100644 --- a/src/client/environmentApi.ts +++ b/src/client/environmentApi.ts @@ -32,6 +32,8 @@ import { Resource, } from './api/types'; import { buildEnvironmentCreationApi } from './pythonEnvironments/creation/createEnvApi'; +import { EnvironmentKnownCache } from './environmentKnownCache'; +import { StopWatch } from './common/utils/stopWatch'; type ActiveEnvironmentChangeEvent = { resource: WorkspaceFolder | undefined; @@ -120,6 +122,15 @@ export function buildEnvironmentApi( const disposables = serviceContainer.get(IDisposableRegistry); const extensions = serviceContainer.get(IExtensions); const envVarsProvider = serviceContainer.get(IEnvironmentVariablesProvider); + let knownCache: EnvironmentKnownCache; + + function initKnownCache() { + const knownEnvs = discoveryApi + .getEnvs() + .filter((e) => filterUsingVSCodeContext(e)) + .map((e) => convertEnvInfoAndGetReference(e)); + return new EnvironmentKnownCache(knownEnvs); + } function sendApiTelemetry(apiName: string, args?: unknown) { extensions .determineExtensionFromCallStack() @@ -139,10 +150,15 @@ export function buildEnvironmentApi( // Filter out environments that are not in the current workspace. return; } + if (!knownCache) { + knownCache = initKnownCache(); + } if (e.old) { if (e.new) { + const newEnv = convertEnvInfoAndGetReference(e.new); + knownCache.updateEnv(convertEnvInfoAndGetReference(e.old), newEnv); traceVerbose('Python API env change detected', env.id, 'update'); - onEnvironmentsChanged.fire({ type: 'update', env: convertEnvInfoAndGetReference(e.new) }); + onEnvironmentsChanged.fire({ type: 'update', env: newEnv }); reportInterpretersChanged([ { path: getEnvPath(e.new.executable.filename, e.new.location).path, @@ -150,8 +166,10 @@ export function buildEnvironmentApi( }, ]); } else { + const oldEnv = convertEnvInfoAndGetReference(e.old); + knownCache.updateEnv(oldEnv, undefined); traceVerbose('Python API env change detected', env.id, 'remove'); - onEnvironmentsChanged.fire({ type: 'remove', env: convertEnvInfoAndGetReference(e.old) }); + onEnvironmentsChanged.fire({ type: 'remove', env: oldEnv }); reportInterpretersChanged([ { path: getEnvPath(e.old.executable.filename, e.old.location).path, @@ -160,8 +178,10 @@ export function buildEnvironmentApi( ]); } } else if (e.new) { + const newEnv = convertEnvInfoAndGetReference(e.new); + knownCache.addEnv(newEnv); traceVerbose('Python API env change detected', env.id, 'add'); - onEnvironmentsChanged.fire({ type: 'add', env: convertEnvInfoAndGetReference(e.new) }); + onEnvironmentsChanged.fire({ type: 'add', env: newEnv }); reportInterpretersChanged([ { path: getEnvPath(e.new.executable.filename, e.new.location).path, @@ -179,6 +199,9 @@ export function buildEnvironmentApi( onEnvironmentsChanged, onEnvironmentVariablesChanged, ); + if (!knownCache!) { + knownCache = initKnownCache(); + } const environmentApi: PythonExtension['environments'] = { getEnvironmentVariables: (resource?: Resource) => { @@ -235,10 +258,7 @@ export function buildEnvironmentApi( }, get known(): Environment[] { sendApiTelemetry('known'); - return discoveryApi - .getEnvs() - .filter((e) => filterUsingVSCodeContext(e)) - .map((e) => convertEnvInfoAndGetReference(e)); + return knownCache.envs; }, async refreshEnvironments(options?: RefreshOptions) { if (!workspace.isTrusted) { diff --git a/src/client/environmentKnownCache.ts b/src/client/environmentKnownCache.ts new file mode 100644 index 000000000000..c13b937d118a --- /dev/null +++ b/src/client/environmentKnownCache.ts @@ -0,0 +1,37 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +import { Environment } from './api/types'; + +/** + * Environment info cache using persistent storage to save and retrieve pre-cached env info. + */ +export class EnvironmentKnownCache { + private _envs: Environment[] = []; + + constructor(envs: Environment[]) { + this._envs = envs; + } + + public get envs(): Environment[] { + return this._envs; + } + + public addEnv(env: Environment): void { + const found = this._envs.find((e) => env.id === e.id); + if (!found) { + this._envs.push(env); + } + } + + public updateEnv(oldValue: Environment, newValue: Environment | undefined): void { + const index = this._envs.findIndex((e) => oldValue.id === e.id); + if (index !== -1) { + if (newValue === undefined) { + this._envs.splice(index, 1); + } else { + this._envs[index] = newValue; + } + } + } +} From a26bc67affde53021610ebed03107b0b25b5bfad Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Fri, 1 Mar 2024 14:50:16 +0530 Subject: [PATCH 2/8] add comment --- src/client/environmentKnownCache.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/environmentKnownCache.ts b/src/client/environmentKnownCache.ts index c13b937d118a..287f5bab343f 100644 --- a/src/client/environmentKnownCache.ts +++ b/src/client/environmentKnownCache.ts @@ -4,7 +4,7 @@ import { Environment } from './api/types'; /** - * Environment info cache using persistent storage to save and retrieve pre-cached env info. + * Workaround temp cache until types are consolidated. */ export class EnvironmentKnownCache { private _envs: Environment[] = []; From df64d5d67e8227a82375bd7d0888c3f2906198d6 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Fri, 1 Mar 2024 14:50:45 +0530 Subject: [PATCH 3/8] fix --- src/client/environmentApi.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/client/environmentApi.ts b/src/client/environmentApi.ts index bb9c00335b47..3f04b227835d 100644 --- a/src/client/environmentApi.ts +++ b/src/client/environmentApi.ts @@ -33,7 +33,6 @@ import { } from './api/types'; import { buildEnvironmentCreationApi } from './pythonEnvironments/creation/createEnvApi'; import { EnvironmentKnownCache } from './environmentKnownCache'; -import { StopWatch } from './common/utils/stopWatch'; type ActiveEnvironmentChangeEvent = { resource: WorkspaceFolder | undefined; From 9e2b39ef142a9aa8a1296694bf969925d656efaa Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Fri, 1 Mar 2024 18:34:58 +0530 Subject: [PATCH 4/8] Add tests --- src/client/environmentApi.ts | 2 +- src/test/environmentApi.unit.test.ts | 15 +++++++++++---- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/client/environmentApi.ts b/src/client/environmentApi.ts index 3f04b227835d..d65ec2a55ab1 100644 --- a/src/client/environmentApi.ts +++ b/src/client/environmentApi.ts @@ -155,7 +155,7 @@ export function buildEnvironmentApi( if (e.old) { if (e.new) { const newEnv = convertEnvInfoAndGetReference(e.new); - knownCache.updateEnv(convertEnvInfoAndGetReference(e.old), newEnv); + knownCache.updateEnv(convertEnvInfo(e.old), newEnv); traceVerbose('Python API env change detected', env.id, 'update'); onEnvironmentsChanged.fire({ type: 'update', env: newEnv }); reportInterpretersChanged([ diff --git a/src/test/environmentApi.unit.test.ts b/src/test/environmentApi.unit.test.ts index 1d8dc3e5c847..9681473cdfc0 100644 --- a/src/test/environmentApi.unit.test.ts +++ b/src/test/environmentApi.unit.test.ts @@ -94,6 +94,7 @@ suite('Python Environment API', () => { discoverAPI.setup((d) => d.onProgress).returns(() => onDidChangeRefreshState.event); discoverAPI.setup((d) => d.onChanged).returns(() => onDidChangeEnvironments.event); + discoverAPI.setup((d) => d.getEnvs()).returns(() => []); environmentApi = buildEnvironmentApi(discoverAPI.object, serviceContainer.object); }); @@ -325,6 +326,7 @@ suite('Python Environment API', () => { }, ]; discoverAPI.setup((d) => d.getEnvs()).returns(() => envs); + environmentApi = buildEnvironmentApi(discoverAPI.object, serviceContainer.object); const actual = environmentApi.known; const actualEnvs = actual?.map((a) => (a as EnvironmentReference).internal); assert.deepEqual( @@ -409,10 +411,10 @@ suite('Python Environment API', () => { // Update events events = []; expectedEvents = []; - const updatedEnv = cloneDeep(envs[0]); - updatedEnv.arch = Architecture.x86; - onDidChangeEnvironments.fire({ old: envs[0], new: updatedEnv }); - expectedEvents.push({ env: convertEnvInfo(updatedEnv), type: 'update' }); + const updatedEnv0 = cloneDeep(envs[0]); + updatedEnv0.arch = Architecture.x86; + onDidChangeEnvironments.fire({ old: envs[0], new: updatedEnv0 }); + expectedEvents.push({ env: convertEnvInfo(updatedEnv0), type: 'update' }); eventValues = events.map((e) => ({ env: (e.env as EnvironmentReference).internal, type: e.type })); assert.deepEqual(eventValues, expectedEvents); @@ -423,6 +425,11 @@ suite('Python Environment API', () => { expectedEvents.push({ env: convertEnvInfo(envs[2]), type: 'remove' }); eventValues = events.map((e) => ({ env: (e.env as EnvironmentReference).internal, type: e.type })); assert.deepEqual(eventValues, expectedEvents); + + const expectedEnvs = [convertEnvInfo(updatedEnv0), convertEnvInfo(envs[1])].sort(); + const knownEnvs = environmentApi.known.map((e) => (e as EnvironmentReference).internal).sort(); + + assert.deepEqual(expectedEnvs, knownEnvs); }); test('updateActiveEnvironmentPath: no resource', async () => { From a5b7b2b2bb35a8517810581400dddd8c5bf97056 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Fri, 1 Mar 2024 18:36:11 +0530 Subject: [PATCH 5/8] Update name --- src/client/environmentApi.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/client/environmentApi.ts b/src/client/environmentApi.ts index d65ec2a55ab1..b7d705e777c1 100644 --- a/src/client/environmentApi.ts +++ b/src/client/environmentApi.ts @@ -127,7 +127,7 @@ export function buildEnvironmentApi( const knownEnvs = discoveryApi .getEnvs() .filter((e) => filterUsingVSCodeContext(e)) - .map((e) => convertEnvInfoAndGetReference(e)); + .map((e) => updateReference(e)); return new EnvironmentKnownCache(knownEnvs); } function sendApiTelemetry(apiName: string, args?: unknown) { @@ -154,7 +154,7 @@ export function buildEnvironmentApi( } if (e.old) { if (e.new) { - const newEnv = convertEnvInfoAndGetReference(e.new); + const newEnv = updateReference(e.new); knownCache.updateEnv(convertEnvInfo(e.old), newEnv); traceVerbose('Python API env change detected', env.id, 'update'); onEnvironmentsChanged.fire({ type: 'update', env: newEnv }); @@ -165,7 +165,7 @@ export function buildEnvironmentApi( }, ]); } else { - const oldEnv = convertEnvInfoAndGetReference(e.old); + const oldEnv = updateReference(e.old); knownCache.updateEnv(oldEnv, undefined); traceVerbose('Python API env change detected', env.id, 'remove'); onEnvironmentsChanged.fire({ type: 'remove', env: oldEnv }); @@ -177,7 +177,7 @@ export function buildEnvironmentApi( ]); } } else if (e.new) { - const newEnv = convertEnvInfoAndGetReference(e.new); + const newEnv = updateReference(e.new); knownCache.addEnv(newEnv); traceVerbose('Python API env change detected', env.id, 'add'); onEnvironmentsChanged.fire({ type: 'add', env: newEnv }); @@ -370,7 +370,7 @@ export function convertEnvInfo(env: PythonEnvInfo): Environment { return convertedEnv as Environment; } -function convertEnvInfoAndGetReference(env: PythonEnvInfo): Environment { +function updateReference(env: PythonEnvInfo): Environment { return getEnvReference(convertEnvInfo(env)); } From e0b7374431961b3628271437e9c60c20bdfc33bf Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Fri, 1 Mar 2024 18:41:46 +0530 Subject: [PATCH 6/8] fix functional tests --- src/test/api.functional.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/api.functional.test.ts b/src/test/api.functional.test.ts index 74293f55256c..d99bcfde81ab 100644 --- a/src/test/api.functional.test.ts +++ b/src/test/api.functional.test.ts @@ -37,6 +37,7 @@ suite('Extension API', () => { interpreterService = mock(InterpreterService); environmentVariablesProvider = mock(); discoverAPI = mock(); + when(discoverAPI.getEnvs()).thenReturn([]); when(serviceContainer.get(IConfigurationService)).thenReturn( instance(configurationService), From f288980dfc0fd01da344714af4c52703cba96623 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Fri, 1 Mar 2024 19:16:42 +0530 Subject: [PATCH 7/8] Do not send telemetry for known --- src/client/environmentApi.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/client/environmentApi.ts b/src/client/environmentApi.ts index b7d705e777c1..65430920d015 100644 --- a/src/client/environmentApi.ts +++ b/src/client/environmentApi.ts @@ -256,7 +256,8 @@ export function buildEnvironmentApi( return resolveEnvironment(path, discoveryApi); }, get known(): Environment[] { - sendApiTelemetry('known'); + // Do not send telemetry for "known", as this may be called 1000s of times so it can significant: + // sendApiTelemetry('known'); return knownCache.envs; }, async refreshEnvironments(options?: RefreshOptions) { From bd34e5eff7838f14d86041c748498bcd01ffeb25 Mon Sep 17 00:00:00 2001 From: Kartik Raj Date: Fri, 1 Mar 2024 19:37:52 +0530 Subject: [PATCH 8/8] Fix tests --- src/test/environmentApi.unit.test.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/test/environmentApi.unit.test.ts b/src/test/environmentApi.unit.test.ts index 9681473cdfc0..012e1a0bfc69 100644 --- a/src/test/environmentApi.unit.test.ts +++ b/src/test/environmentApi.unit.test.ts @@ -74,8 +74,7 @@ suite('Python Environment API', () => { envVarsProvider = typemoq.Mock.ofType(); extensions .setup((e) => e.determineExtensionFromCallStack()) - .returns(() => Promise.resolve({ extensionId: 'id', displayName: 'displayName', apiName: 'apiName' })) - .verifiable(typemoq.Times.atLeastOnce()); + .returns(() => Promise.resolve({ extensionId: 'id', displayName: 'displayName', apiName: 'apiName' })); interpreterPathService = typemoq.Mock.ofType(); configService = typemoq.Mock.ofType(); onDidChangeRefreshState = new EventEmitter(); @@ -100,8 +99,6 @@ suite('Python Environment API', () => { }); teardown(() => { - // Verify each API method sends telemetry regarding who called the API. - extensions.verifyAll(); sinon.restore(); });