Speed up interpreter locators (#676)

performance improvements (fixes #666)

Author: DonJayamanne

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';

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: {

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);
     }
 }
 

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');

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
         };
     }

src/client/interpreter/contracts.ts

@@ -62,6 +62,7 @@ export type PythonInterpreter = {
     type: InterpreterType;
     envName?: string;
     envPath?: string;
+    cachedEntry?: boolean;
 };
 
 export type WorkspacePythonPath = {

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();
 

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) {

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)

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);
+    }
+}