Improve the perf of functional tests running with real jupyter (#10242)

* Improve the perf of subsequent tests by caching interpreters

* Add back the nightly flake

* Remove coverage

* Add news entry

* Use a static map to allow promise to be cleared on new interpreters (as it was before)

Author: rchiodo

build/.mocha.functional.perf.opts

@@ -0,0 +1,9 @@
+./out/test/**/*.functional.test.js
+--require=out/test/unittests.js
+--exclude=out/**/*.jsx
+--ui=tdd
+--recursive
+--colors
+--exit
+--timeout=180000
+--reporter spec

build/ci/vscode-python-nightly-flake-ci.yaml

@@ -0,0 +1,137 @@
+# Nightly build
+
+name: '$(Year:yyyy).$(Month).0.$(BuildID)-nightly-flake'
+
+# Not the CI build, see `vscode-python-nightly-flake-ci.yaml`.
+trigger: none
+
+# Not the PR build for merges to master and release.
+pr: none
+
+schedules:
+- cron: "0 8 * * 1-5"
+  # Daily midnight PST build, runs Monday - Friday always
+  displayName: Nightly Flake build
+  branches:
+    include:
+    - master
+    - release*
+  always: true
+
+# Variables that are available for the entire pipeline.
+variables:
+- template: templates/globals.yml
+
+stages:
+- stage: Build
+  jobs:
+  - template: templates/jobs/build_compile.yml
+
+# Each item in each matrix has a number of possible values it may
+# define.  They are detailed in templates/test_phases.yml.  The only
+# required value is "TestsToRun".
+
+- stage: Linux
+  dependsOn:
+  - Build
+  jobs:
+  - job: 'Py3x'
+    dependsOn: []
+    timeoutInMinutes: 120
+    strategy:
+      matrix:
+        'Functional':
+          TestsToRun: 'testfunctional'
+          NeedsPythonTestReqs: true
+          NeedsPythonFunctionalReqs: true
+          VSCODE_PYTHON_ROLLING: true
+    pool:
+      vmImage: 'ubuntu-16.04'
+    steps:
+      - template: templates/test_phases.yml
+
+  - job: 'Py36'
+    dependsOn: []
+    timeoutInMinutes: 120
+    strategy:
+      matrix:
+        'Functional':
+          PythonVersion: '3.6'
+          TestsToRun: 'testfunctional'
+          NeedsPythonTestReqs: true
+          NeedsPythonFunctionalReqs: true
+          VSCODE_PYTHON_ROLLING: true
+    pool:
+      vmImage: 'ubuntu-16.04'
+    steps:
+      - template: templates/test_phases.yml
+
+- stage: Mac
+  dependsOn:
+  - Build
+  jobs:
+  - job: 'Py3x'
+    dependsOn: []
+    timeoutInMinutes: 120
+    strategy:
+      matrix:
+        'Functional':
+          TestsToRun: 'testfunctional'
+          NeedsPythonTestReqs: true
+          NeedsPythonFunctionalReqs: true
+          VSCODE_PYTHON_ROLLING: true
+    pool:
+      vmImage: 'macos-10.13'
+    steps:
+      - template: templates/test_phases.yml
+
+  - job: 'Py36'
+    dependsOn: []
+    timeoutInMinutes: 120
+    strategy:
+      matrix:
+        'Functional':
+          PythonVersion: '3.6'
+          TestsToRun: 'testfunctional'
+          NeedsPythonTestReqs: true
+          NeedsPythonFunctionalReqs: true
+          VSCODE_PYTHON_ROLLING: true
+    pool:
+      vmImage: 'macos-10.13'
+    steps:
+      - template: templates/test_phases.yml
+
+- stage: Windows
+  dependsOn:
+  - Build
+  jobs:
+  - job: 'Py3x'
+    dependsOn: []
+    timeoutInMinutes: 120
+    strategy:
+      matrix:
+        'Functional':
+          TestsToRun: 'testfunctional'
+          NeedsPythonTestReqs: true
+          NeedsPythonFunctionalReqs: true
+          VSCODE_PYTHON_ROLLING: true
+    pool:
+      vmImage: 'vs2017-win2016'
+    steps:
+      - template: templates/test_phases.yml
+
+  - job: 'Py36'
+    dependsOn: []
+    timeoutInMinutes: 120
+    strategy:
+      matrix:
+        'Functional':
+          PythonVersion: '3.6'
+          TestsToRun: 'testfunctional'
+          NeedsPythonTestReqs: true
+          NeedsPythonFunctionalReqs: true
+          VSCODE_PYTHON_ROLLING: true
+    pool:
+      vmImage: 'vs2017-win2016'
+    steps:
+      - template: templates/test_phases.yml

news/3 Code Health/7997.md

@@ -0,0 +1 @@
+Functional tests using real jupyter can take 30-90 seconds each. Most of this time is searching for interpreters. Cache the interpreter search.

package.json

@@ -2844,6 +2844,7 @@
         "test:unittests": "mocha --opts ./build/.mocha.unittests.js.opts",
         "test:unittests:cover": "nyc --no-clean --nycrc-path build/.nycrc mocha --opts ./build/.mocha.unittests.ts.opts",
         "test:functional": "mocha --require source-map-support/register --opts ./build/.mocha.functional.opts",
+        "test:functional:perf": "node --inspect-brk ./node_modules/mocha/bin/_mocha --require source-map-support/register --opts ./build/.mocha.functional.perf.opts",
         "test:functional:cover": "npm run test:functional",
         "test:cover:report": "nyc --nycrc-path build/.nycrc  report --reporter=text --reporter=html --reporter=text-summary --reporter=cobertura",
         "testDebugger": "node ./out/test/testBootstrap.js ./out/test/debuggerTest.js",

src/client/interpreter/locators/services/cacheableLocatorService.ts

@@ -17,10 +17,42 @@ import { sendTelemetryEvent } from '../../../telemetry';
 import { EventName } from '../../../telemetry/constants';
 import { IInterpreterLocatorService, IInterpreterWatcher, PythonInterpreter } from '../../contracts';
 
+export class CacheableLocatorPromiseCache {
+    private static useStatic = false;
+    private static staticMap = new Map<string, Deferred<PythonInterpreter[]>>();
+    private normalMap = new Map<string, Deferred<PythonInterpreter[]>>();
+
+    public static forceUseStatic() {
+        CacheableLocatorPromiseCache.useStatic = true;
+    }
+    public get(key: string): Deferred<PythonInterpreter[]> | undefined {
+        if (CacheableLocatorPromiseCache.useStatic) {
+            return CacheableLocatorPromiseCache.staticMap.get(key);
+        }
+        return this.normalMap.get(key);
+    }
+
+    public set(key: string, value: Deferred<PythonInterpreter[]>) {
+        if (CacheableLocatorPromiseCache.useStatic) {
+            CacheableLocatorPromiseCache.staticMap.set(key, value);
+        } else {
+            this.normalMap.set(key, value);
+        }
+    }
+
+    public delete(key: string) {
+        if (CacheableLocatorPromiseCache.useStatic) {
+            CacheableLocatorPromiseCache.staticMap.delete(key);
+        } else {
+            this.normalMap.delete(key);
+        }
+    }
+}
+
 @injectable()
 export abstract class CacheableLocatorService implements IInterpreterLocatorService {
     protected readonly _hasInterpreters: Deferred<boolean>;
-    private readonly promisesPerResource = new Map<string, Deferred<PythonInterpreter[]>>();
+    private readonly promisesPerResource = new CacheableLocatorPromiseCache();
     private readonly handlersAddedToResource = new Set<string>();
     private readonly cacheKeyPrefix: string;
     private readonly locating = new EventEmitter<Promise<PythonInterpreter[]>>();
@@ -32,6 +64,7 @@ export abstract class CacheableLocatorService implements IInterpreterLocatorServ
         this._hasInterpreters = createDeferred<boolean>();
         this.cacheKeyPrefix = `INTERPRETERS_CACHE_v3_${name}`;
     }
+
     public get onLocating(): Event<Promise<PythonInterpreter[]>> {
         return this.locating.event;
     }
@@ -43,7 +76,6 @@ export abstract class CacheableLocatorService implements IInterpreterLocatorServ
     public async getInterpreters(resource?: Uri, ignoreCache?: boolean): Promise<PythonInterpreter[]> {
         const cacheKey = this.getCacheKey(resource);
         let deferred = this.promisesPerResource.get(cacheKey);
-
         if (!deferred || ignoreCache) {
             deferred = createDeferred<PythonInterpreter[]>();
             this.promisesPerResource.set(cacheKey, deferred);

src/test/datascience/dataScienceIocContainer.ts

@@ -16,6 +16,7 @@ import {
     Event,
     EventEmitter,
     FileSystemWatcher,
+    Memento,
     Uri,
     WorkspaceConfiguration,
     WorkspaceFolder,
@@ -99,7 +100,6 @@ import {
 } from '../../client/common/installer/productPath';
 import { ProductService } from '../../client/common/installer/productService';
 import { IInstallationChannelManager, IProductPathService, IProductService } from '../../client/common/installer/types';
-import { PersistentStateFactory } from '../../client/common/persistentState';
 import { IS_WINDOWS } from '../../client/common/platform/constants';
 import { PathUtils } from '../../client/common/platform/pathUtils';
 import { RegistryImplementation } from '../../client/common/platform/registry';
@@ -130,6 +130,7 @@ import {
 } from '../../client/common/terminal/types';
 import {
     BANNER_NAME_LS_SURVEY,
+    GLOBAL_MEMENTO,
     IAsyncDisposableRegistry,
     IConfigurationService,
     ICryptoUtils,
@@ -139,12 +140,14 @@ import {
     IExtensionContext,
     IExtensions,
     IInstaller,
+    IMemento,
     IOutputChannel,
     IPathUtils,
     IPersistentStateFactory,
     IPythonExtensionBanner,
     IsWindows,
-    ProductType
+    ProductType,
+    WORKSPACE_MEMENTO
 } from '../../client/common/types';
 import { Deferred, sleep } from '../../client/common/utils/async';
 import { noop } from '../../client/common/utils/misc';
@@ -283,6 +286,7 @@ import { InterpreterService } from '../../client/interpreter/interpreterService'
 import { InterpreterVersionService } from '../../client/interpreter/interpreterVersion';
 import { PythonInterpreterLocatorService } from '../../client/interpreter/locators';
 import { InterpreterLocatorHelper } from '../../client/interpreter/locators/helpers';
+import { CacheableLocatorPromiseCache } from '../../client/interpreter/locators/services/cacheableLocatorService';
 import { CondaEnvFileService } from '../../client/interpreter/locators/services/condaEnvFileService';
 import { CondaEnvService } from '../../client/interpreter/locators/services/condaEnvService';
 import {
@@ -333,6 +337,7 @@ import { MockWorkspaceConfiguration } from './mockWorkspaceConfig';
 import { blurWindow, createMessageEvent } from './reactHelpers';
 import { TestInteractiveWindowProvider } from './testInteractiveWindowProvider';
 import { TestNativeEditorProvider } from './testNativeEditorProvider';
+import { TestPersistentStateFactory } from './testPersistentStateFactory';
 
 export class DataScienceIocContainer extends UnitTestIocContainer {
     public webPanelListener: IWebPanelMessageListener | undefined;
@@ -914,7 +919,19 @@ export class DataScienceIocContainer extends UnitTestIocContainer {
             IInterpreterVersionService,
             InterpreterVersionService
         );
-        this.serviceManager.addSingleton<IPersistentStateFactory>(IPersistentStateFactory, PersistentStateFactory);
+
+        const globalStorage = this.serviceManager.get<Memento>(IMemento, GLOBAL_MEMENTO);
+        const localStorage = this.serviceManager.get<Memento>(IMemento, WORKSPACE_MEMENTO);
+
+        // Create a custom persistent state factory that remembers specific things between tests
+        this.serviceManager.addSingletonInstance<IPersistentStateFactory>(
+            IPersistentStateFactory,
+            new TestPersistentStateFactory(globalStorage, localStorage)
+        );
+
+        // Inform the cacheable locator service to use a static map so that it stays in memory in between tests
+        CacheableLocatorPromiseCache.forceUseStatic();
+
         this.serviceManager.addSingletonInstance<IInterpreterDisplay>(IInterpreterDisplay, interpreterDisplay.object);
 
         this.serviceManager.addSingleton<IPythonPathUpdaterServiceFactory>(

src/test/datascience/testPersistentStateFactory.ts

@@ -0,0 +1,53 @@
+import { Memento } from 'vscode';
+import { PersistentStateFactory } from '../../client/common/persistentState';
+import { IPersistentState, IPersistentStateFactory } from '../../client/common/types';
+
+const PrefixesToStore = ['INTERPRETERS_CACHE'];
+
+// tslint:disable-next-line: no-any
+const persistedState = new Map<string, any>();
+
+class TestPersistentState<T> implements IPersistentState<T> {
+    constructor(private key: string, defaultValue?: T | undefined) {
+        if (defaultValue) {
+            persistedState.set(key, defaultValue);
+        }
+    }
+    public get value(): T {
+        return persistedState.get(this.key);
+    }
+    public async updateValue(value: T): Promise<void> {
+        persistedState.set(this.key, value);
+    }
+}
+
+// This class is used to make certain values persist across tests.
+export class TestPersistentStateFactory implements IPersistentStateFactory {
+    private realStateFactory: PersistentStateFactory;
+    constructor(globalState: Memento, localState: Memento) {
+        this.realStateFactory = new PersistentStateFactory(globalState, localState);
+    }
+
+    public createGlobalPersistentState<T>(
+        key: string,
+        defaultValue?: T | undefined,
+        expiryDurationMs?: number | undefined
+    ): IPersistentState<T> {
+        if (PrefixesToStore.find(p => key.startsWith(p))) {
+            return new TestPersistentState(key, defaultValue);
+        }
+
+        return this.realStateFactory.createGlobalPersistentState(key, defaultValue, expiryDurationMs);
+    }
+    public createWorkspacePersistentState<T>(
+        key: string,
+        defaultValue?: T | undefined,
+        expiryDurationMs?: number | undefined
+    ): IPersistentState<T> {
+        if (PrefixesToStore.find(p => key.startsWith(p))) {
+            return new TestPersistentState(key, defaultValue);
+        }
+
+        return this.realStateFactory.createWorkspacePersistentState(key, defaultValue, expiryDurationMs);
+    }
+}