Speed up interpreter locators (#676)

performance improvements (fixes #666)
microsoft · Feb 1, 2018 · b09a421 · b09a421
1 parent d596beb
commit b09a421
Show file tree

Hide file tree

Showing 21 changed files with 192 additions and 89 deletions.
diff --git a/src/client/banner.ts b/src/client/banner.ts
@@ -17,7 +17,8 @@ export class BannerService {
         if (!this.shouldShowBanner.value) {
             return;
         }
-        this.shouldShowBanner.value = false;
+        this.shouldShowBanner.updateValue(false)
+            .catch(ex => console.error('Python Extension: Failed to update banner value', ex));
 
         const message = 'The Python extension is now published by Microsoft!';
         const yesButton = 'Read more';

diff --git a/src/client/common/featureDeprecationManager.ts b/src/client/common/featureDeprecationManager.ts
@@ -112,7 +112,7 @@ export class FeatureDeprecationManager implements IFeatureDeprecationManager {
                 break;
             }
             case doNotShowAgain: {
-                notificationPromptEnabled.value = false;
+                await notificationPromptEnabled.updateValue(false);
                 break;
             }
             default: {

diff --git a/src/client/common/persistentState.ts b/src/client/common/persistentState.ts
@@ -14,8 +14,8 @@ class PersistentState<T> implements IPersistentState<T>{
         return this.storage.get<T>(this.key, this.defaultValue);
     }
 
-    public set value(newValue: T) {
-        this.storage.update(this.key, newValue);
+    public async updateValue(newValue: T): Promise<void> {
+        await this.storage.update(this.key, newValue);
     }
 }
 

diff --git a/src/client/common/types.ts b/src/client/common/types.ts
@@ -14,7 +14,8 @@ export const GLOBAL_MEMENTO = Symbol('IGlobalMemento');
 export const WORKSPACE_MEMENTO = Symbol('IWorkspaceMemento');
 
 export interface IPersistentState<T> {
-    value: T;
+    readonly value: T;
+    updateValue(value: T): Promise<void>;
 }
 
 export const IPersistentStateFactory = Symbol('IPersistentStateFactory');

diff --git a/src/client/interpreter/configuration/interpreterSelector.ts b/src/client/interpreter/configuration/interpreterSelector.ts
@@ -43,11 +43,12 @@ export class InterpreterSelector implements Disposable {
         if (workspaceUri && suggestion.path.startsWith(workspaceUri.fsPath)) {
             detail = `.${path.sep}${path.relative(workspaceUri.fsPath, suggestion.path)}`;
         }
+        const cachedPrefix = suggestion.cachedEntry ? '(cached) ' : '';
         return {
             // tslint:disable-next-line:no-non-null-assertion
             label: suggestion.displayName!,
             description: suggestion.companyDisplayName || '',
-            detail: detail,
+            detail: `${cachedPrefix}${detail}`,
             path: suggestion.path
         };
     }

diff --git a/src/client/interpreter/contracts.ts b/src/client/interpreter/contracts.ts
@@ -62,6 +62,7 @@ export type PythonInterpreter = {
     type: InterpreterType;
     envName?: string;
     envPath?: string;
+    cachedEntry?: boolean;
 };
 
 export type WorkspacePythonPath = {

diff --git a/src/client/interpreter/index.ts b/src/client/interpreter/index.ts
@@ -53,7 +53,7 @@ export class InterpreterManager implements Disposable, IInterpreterService {
         }
         const virtualEnvMgr = this.serviceContainer.get<IVirtualEnvironmentManager>(IVirtualEnvironmentManager);
         const versionService = this.serviceContainer.get<IInterpreterVersionService>(IInterpreterVersionService);
-        const virtualEnvInterpreterProvider = new VirtualEnvService([activeWorkspace.folderUri.fsPath], virtualEnvMgr, versionService);
+        const virtualEnvInterpreterProvider = new VirtualEnvService([activeWorkspace.folderUri.fsPath], virtualEnvMgr, versionService, this.serviceContainer);
         const interpreters = await virtualEnvInterpreterProvider.getInterpreters(activeWorkspace.folderUri);
         const workspacePathUpper = activeWorkspace.folderUri.fsPath.toUpperCase();
 

diff --git a/src/client/interpreter/locators/index.ts b/src/client/interpreter/locators/index.ts
@@ -1,7 +1,7 @@
 import { inject, injectable } from 'inversify';
 import * as _ from 'lodash';
 import * as path from 'path';
-import { Disposable, Uri, workspace } from 'vscode';
+import { Disposable, Uri } from 'vscode';
 import { IPlatformService } from '../../common/platform/types';
 import { IDisposableRegistry } from '../../common/types';
 import { arePathsSame } from '../../common/utils';
@@ -21,35 +21,21 @@ import { fixInterpreterDisplayName } from './helpers';
 
 @injectable()
 export class PythonInterpreterLocatorService implements IInterpreterLocatorService {
-    private interpretersPerResource: Map<string, Promise<PythonInterpreter[]>>;
     private disposables: Disposable[] = [];
     private platform: IPlatformService;
 
     constructor( @inject(IServiceContainer) private serviceContainer: IServiceContainer) {
-        this.interpretersPerResource = new Map<string, Promise<PythonInterpreter[]>>();
         serviceContainer.get<Disposable[]>(IDisposableRegistry).push(this);
         this.platform = serviceContainer.get<IPlatformService>(IPlatformService);
     }
     public async getInterpreters(resource?: Uri) {
-        const resourceKey = this.getResourceKey(resource);
-        if (!this.interpretersPerResource.has(resourceKey)) {
-            this.interpretersPerResource.set(resourceKey, this.getInterpretersPerResource(resource));
-        }
-
-        return await this.interpretersPerResource.get(resourceKey)!;
+        return this.getInterpretersPerResource(resource);
     }
     public dispose() {
         this.disposables.forEach(disposable => disposable.dispose());
     }
-    private getResourceKey(resource?: Uri) {
-        if (!resource) {
-            return '';
-        }
-        const workspaceFolder = workspace.getWorkspaceFolder(resource);
-        return workspaceFolder ? workspaceFolder.uri.fsPath : '';
-    }
     private async getInterpretersPerResource(resource?: Uri) {
-        const locators = this.getLocators(resource);
+        const locators = this.getLocators();
         const promises = locators.map(async provider => provider.getInterpreters(resource));
         const listOfInterpreters = await Promise.all(promises);
 
@@ -73,7 +59,7 @@ export class PythonInterpreterLocatorService implements IInterpreterLocatorServi
                 return accumulator;
             }, []);
     }
-    private getLocators(resource?: Uri) {
+    private getLocators() {
         const locators: IInterpreterLocatorService[] = [];
         // The order of the services is important.
         if (this.platform.isWindows) {

diff --git a/src/client/interpreter/locators/services/KnownPathsService.ts b/src/client/interpreter/locators/services/KnownPathsService.ts
@@ -3,22 +3,26 @@ import * as _ from 'lodash';
 import * as path from 'path';
 import { Uri } from 'vscode';
 import { fsExistsAsync, IS_WINDOWS } from '../../../common/utils';
-import { IInterpreterLocatorService, IInterpreterVersionService, IKnownSearchPathsForInterpreters, InterpreterType } from '../../contracts';
+import { IServiceContainer } from '../../../ioc/types';
+import { IInterpreterVersionService, IKnownSearchPathsForInterpreters, InterpreterType, PythonInterpreter } from '../../contracts';
 import { lookForInterpretersInDirectory } from '../helpers';
+import { CacheableLocatorService } from './cacheableLocatorService';
 
 // tslint:disable-next-line:no-require-imports no-var-requires
 const untildify = require('untildify');
 
 @injectable()
-export class KnownPathsService implements IInterpreterLocatorService {
+export class KnownPathsService extends CacheableLocatorService {
     public constructor( @inject(IKnownSearchPathsForInterpreters) private knownSearchPaths: string[],
-        @inject(IInterpreterVersionService) private versionProvider: IInterpreterVersionService) { }
-    // tslint:disable-next-line:no-shadowed-variable
-    public getInterpreters(_?: Uri) {
-        return this.suggestionsFromKnownPaths();
+        @inject(IInterpreterVersionService) private versionProvider: IInterpreterVersionService,
+        @inject(IServiceContainer) serviceContainer: IServiceContainer) {
+        super('KnownPathsService', serviceContainer);
     }
     // tslint:disable-next-line:no-empty
     public dispose() { }
+    protected getInterpretersImplementation(resource?: Uri): Promise<PythonInterpreter[]> {
+        return this.suggestionsFromKnownPaths();
+    }
     private suggestionsFromKnownPaths() {
         const promises = this.knownSearchPaths.map(dir => this.getInterpretersInDirectory(dir));
         return Promise.all<string[]>(promises)

diff --git a/src/client/interpreter/locators/services/cacheableLocatorService.ts b/src/client/interpreter/locators/services/cacheableLocatorService.ts
@@ -0,0 +1,59 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+import { injectable } from 'inversify';
+import { Uri } from 'vscode';
+import { createDeferred, Deferred } from '../../../common/helpers';
+import { IPersistentStateFactory } from '../../../common/types';
+import { IServiceContainer } from '../../../ioc/types';
+import { IInterpreterLocatorService, PythonInterpreter } from '../../contracts';
+
+@injectable()
+export abstract class CacheableLocatorService implements IInterpreterLocatorService {
+    private getInterpretersPromise: Deferred<PythonInterpreter[]>;
+    private readonly cacheKey: string;
+    constructor(name: string,
+        protected readonly serviceContainer: IServiceContainer) {
+        this.cacheKey = `INTERPRETERS_CACHE_${name}`;
+    }
+    public abstract dispose();
+    public async getInterpreters(resource?: Uri): Promise<PythonInterpreter[]> {
+        if (!this.getInterpretersPromise) {
+            this.getInterpretersPromise = createDeferred<PythonInterpreter[]>();
+            this.getInterpretersImplementation(resource)
+                .then(async items => {
+                    await this.cacheInterpreters(items);
+                    this.getInterpretersPromise.resolve(items);
+                })
+                .catch(ex => this.getInterpretersPromise.reject(ex));
+        }
+        if (this.getInterpretersPromise.completed) {
+            return this.getInterpretersPromise.promise;
+        }
+
+        const cachedInterpreters = this.getCachedInterpreters();
+        return Array.isArray(cachedInterpreters) ? cachedInterpreters : this.getInterpretersPromise.promise;
+    }
+
+    protected abstract getInterpretersImplementation(resource?: Uri): Promise<PythonInterpreter[]>;
+
+    private getCachedInterpreters() {
+        const persistentFactory = this.serviceContainer.get<IPersistentStateFactory>(IPersistentStateFactory);
+        // tslint:disable-next-line:no-any
+        const globalPersistence = persistentFactory.createGlobalPersistentState<PythonInterpreter[]>(this.cacheKey, undefined as any);
+        if (!Array.isArray(globalPersistence.value)) {
+            return;
+        }
+        return globalPersistence.value.map(item => {
+            return {
+                ...item,
+                cachedEntry: true
+            };
+        });
+    }
+    private async cacheInterpreters(interpreters: PythonInterpreter[]) {
+        const persistentFactory = this.serviceContainer.get<IPersistentStateFactory>(IPersistentStateFactory);
+        const globalPersistence = persistentFactory.createGlobalPersistentState<PythonInterpreter[]>(this.cacheKey, []);
+        await globalPersistence.updateValue(interpreters);
+    }
+}
diff --git a/src/client/interpreter/locators/services/condaEnvFileService.ts b/src/client/interpreter/locators/services/condaEnvFileService.ts
@@ -3,27 +3,30 @@ import * as path from 'path';
 import { Uri } from 'vscode';
 import { IFileSystem } from '../../../common/platform/types';
 import { ILogger } from '../../../common/types';
+import { IServiceContainer } from '../../../ioc/types';
 import {
     ICondaService,
-    IInterpreterLocatorService,
     IInterpreterVersionService,
     InterpreterType,
     PythonInterpreter
 } from '../../contracts';
+import { CacheableLocatorService } from './cacheableLocatorService';
 import { AnacondaCompanyName, AnacondaCompanyNames, AnacondaDisplayName } from './conda';
 
 @injectable()
-export class CondaEnvFileService implements IInterpreterLocatorService {
+export class CondaEnvFileService extends CacheableLocatorService {
     constructor( @inject(IInterpreterVersionService) private versionService: IInterpreterVersionService,
         @inject(ICondaService) private condaService: ICondaService,
         @inject(IFileSystem) private fileSystem: IFileSystem,
+        @inject(IServiceContainer) serviceContainer: IServiceContainer,
         @inject(ILogger) private logger: ILogger) {
-    }
-    public async getInterpreters(_?: Uri) {
-        return this.getSuggestionsFromConda();
+        super('CondaEnvFileService', serviceContainer);
     }
     // tslint:disable-next-line:no-empty
     public dispose() { }
+    protected getInterpretersImplementation(resource?: Uri): Promise<PythonInterpreter[]> {
+        return this.getSuggestionsFromConda();
+    }
     private async getSuggestionsFromConda(): Promise<PythonInterpreter[]> {
         if (!this.condaService.condaEnvironmentsFile) {
             return [];

diff --git a/src/client/interpreter/locators/services/condaEnvService.ts b/src/client/interpreter/locators/services/condaEnvService.ts
@@ -5,20 +5,21 @@ import { inject, injectable } from 'inversify';
 import { Uri } from 'vscode';
 import { IFileSystem } from '../../../common/platform/types';
 import { ILogger } from '../../../common/types';
-import { CondaInfo, ICondaService, IInterpreterLocatorService, IInterpreterVersionService, InterpreterType, PythonInterpreter } from '../../contracts';
+import { IServiceContainer } from '../../../ioc/types';
+import { CondaInfo, ICondaService, IInterpreterVersionService, InterpreterType, PythonInterpreter } from '../../contracts';
+import { CacheableLocatorService } from './cacheableLocatorService';
 import { AnacondaCompanyName, AnacondaCompanyNames } from './conda';
 import { CondaHelper } from './condaHelper';
 
 @injectable()
-export class CondaEnvService implements IInterpreterLocatorService {
+export class CondaEnvService extends CacheableLocatorService {
     private readonly condaHelper = new CondaHelper();
     constructor( @inject(ICondaService) private condaService: ICondaService,
         @inject(IInterpreterVersionService) private versionService: IInterpreterVersionService,
         @inject(ILogger) private logger: ILogger,
+        @inject(IServiceContainer) serviceContainer: IServiceContainer,
         @inject(IFileSystem) private fileSystem: IFileSystem) {
-    }
-    public async getInterpreters(resource?: Uri) {
-        return this.getSuggestionsFromConda();
+        super('CondaEnvService', serviceContainer);
     }
     // tslint:disable-next-line:no-empty
     public dispose() { }
@@ -62,6 +63,9 @@ export class CondaEnvService implements IInterpreterLocatorService {
             // tslint:disable-next-line:no-non-null-assertion
             .then(interpreters => interpreters.map(interpreter => interpreter!));
     }
+    protected getInterpretersImplementation(resource?: Uri): Promise<PythonInterpreter[]> {
+        return this.getSuggestionsFromConda();
+    }
     private stripCompanyName(content: string) {
         // Strip company name from version.
         const startOfCompanyName = AnacondaCompanyNames.reduce((index, companyName) => {

diff --git a/src/client/interpreter/locators/services/condaService.ts b/src/client/interpreter/locators/services/condaService.ts
@@ -132,10 +132,10 @@ export class CondaService implements ICondaService {
             const condaFile = await this.getCondaFile();
             const envInfo = await this.processService.exec(condaFile, ['env', 'list']).then(output => output.stdout);
             const environments = this.condaHelper.parseCondaEnvironmentNames(envInfo);
-            globalPersistence.value = { data: environments };
+            await globalPersistence.updateValue({ data: environments });
             return environments;
         } catch (ex) {
-            globalPersistence.value = { data: undefined };
+            await globalPersistence.updateValue({ data: undefined });
             // Failed because either:
             //   1. conda is not installed.
             //   2. `conda env list has changed signature.

diff --git a/src/client/interpreter/locators/services/currentPathService.ts b/src/client/interpreter/locators/services/currentPathService.ts
@@ -4,19 +4,24 @@ import * as path from 'path';
 import { Uri } from 'vscode';
 import { PythonSettings } from '../../../common/configSettings';
 import { IProcessService } from '../../../common/process/types';
-import { IInterpreterLocatorService, IInterpreterVersionService, InterpreterType } from '../../contracts';
+import { IServiceContainer } from '../../../ioc/types';
+import { IInterpreterVersionService, InterpreterType, PythonInterpreter } from '../../contracts';
 import { IVirtualEnvironmentManager } from '../../virtualEnvs/types';
+import { CacheableLocatorService } from './cacheableLocatorService';
 
 @injectable()
-export class CurrentPathService implements IInterpreterLocatorService {
+export class CurrentPathService extends CacheableLocatorService {
     public constructor( @inject(IVirtualEnvironmentManager) private virtualEnvMgr: IVirtualEnvironmentManager,
         @inject(IInterpreterVersionService) private versionProvider: IInterpreterVersionService,
-        @inject(IProcessService) private processService: IProcessService) { }
-    public async getInterpreters(resource?: Uri) {
-        return this.suggestionsFromKnownPaths();
+        @inject(IProcessService) private processService: IProcessService,
+        @inject(IServiceContainer) serviceContainer: IServiceContainer) {
+        super('CurrentPathService', serviceContainer);
     }
     // tslint:disable-next-line:no-empty
     public dispose() { }
+    protected getInterpretersImplementation(resource?: Uri): Promise<PythonInterpreter[]> {
+        return this.suggestionsFromKnownPaths();
+    }
     private async suggestionsFromKnownPaths(resource?: Uri) {
         const currentPythonInterpreter = this.getInterpreter(PythonSettings.getInstance(resource).pythonPath, '').then(interpreter => [interpreter]);
         const python = this.getInterpreter('python', '').then(interpreter => [interpreter]);

diff --git a/src/client/interpreter/locators/services/virtualEnvService.ts b/src/client/interpreter/locators/services/virtualEnvService.ts
@@ -3,24 +3,29 @@ import * as _ from 'lodash';
 import * as path from 'path';
 import { Uri, workspace } from 'vscode';
 import { fsReaddirAsync, IS_WINDOWS } from '../../../common/utils';
-import { IInterpreterLocatorService, IInterpreterVersionService, IKnownSearchPathsForVirtualEnvironments, InterpreterType, PythonInterpreter } from '../../contracts';
+import { IServiceContainer } from '../../../ioc/types';
+import { IInterpreterVersionService, IKnownSearchPathsForVirtualEnvironments, InterpreterType, PythonInterpreter } from '../../contracts';
 import { IVirtualEnvironmentManager } from '../../virtualEnvs/types';
 import { lookForInterpretersInDirectory } from '../helpers';
 import * as settings from './../../../common/configSettings';
+import { CacheableLocatorService } from './cacheableLocatorService';
 
 // tslint:disable-next-line:no-require-imports no-var-requires
 const untildify = require('untildify');
 
 @injectable()
-export class VirtualEnvService implements IInterpreterLocatorService {
+export class VirtualEnvService extends CacheableLocatorService {
     public constructor( @inject(IKnownSearchPathsForVirtualEnvironments) private knownSearchPaths: string[],
         @inject(IVirtualEnvironmentManager) private virtualEnvMgr: IVirtualEnvironmentManager,
-        @inject(IInterpreterVersionService) private versionProvider: IInterpreterVersionService) { }
-    public async getInterpreters(resource?: Uri) {
-        return this.suggestionsFromKnownVenvs();
+        @inject(IInterpreterVersionService) private versionProvider: IInterpreterVersionService,
+        @inject(IServiceContainer) serviceContainer: IServiceContainer) {
+        super('KnownPathsService', serviceContainer);
     }
     // tslint:disable-next-line:no-empty
     public dispose() { }
+    protected getInterpretersImplementation(resource?: Uri): Promise<PythonInterpreter[]> {
+        return this.suggestionsFromKnownVenvs();
+    }
     private async suggestionsFromKnownVenvs() {
         return Promise.all(this.knownSearchPaths.map(dir => this.lookForInterpretersInVenvs(dir)))
             // tslint:disable-next-line:underscore-consistent-invocation