Skip to content

Commit

Permalink
Improve the perf of functional tests running with real jupyter (#10242)
Browse files Browse the repository at this point in the history
* 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)
  • Loading branch information
rchiodo authored Feb 21, 2020
1 parent 71ba3bb commit d084c47
Show file tree
Hide file tree
Showing 7 changed files with 255 additions and 5 deletions.
9 changes: 9 additions & 0 deletions build/.mocha.functional.perf.opts
Original file line number Diff line number Diff line change
@@ -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
137 changes: 137 additions & 0 deletions build/ci/vscode-python-nightly-flake-ci.yaml
Original file line number Diff line number Diff line change
@@ -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
1 change: 1 addition & 0 deletions news/3 Code Health/7997.md
Original file line number Diff line number Diff line change
@@ -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.
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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[]>>();
Expand All @@ -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;
}
Expand All @@ -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);
Expand Down
23 changes: 20 additions & 3 deletions src/test/datascience/dataScienceIocContainer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
Event,
EventEmitter,
FileSystemWatcher,
Memento,
Uri,
WorkspaceConfiguration,
WorkspaceFolder,
Expand Down Expand Up @@ -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';
Expand Down Expand Up @@ -130,6 +130,7 @@ import {
} from '../../client/common/terminal/types';
import {
BANNER_NAME_LS_SURVEY,
GLOBAL_MEMENTO,
IAsyncDisposableRegistry,
IConfigurationService,
ICryptoUtils,
Expand All @@ -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';
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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>(
Expand Down
53 changes: 53 additions & 0 deletions src/test/datascience/testPersistentStateFactory.ts
Original file line number Diff line number Diff line change
@@ -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);
}
}

0 comments on commit d084c47

Please sign in to comment.