Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dramatically improve watch mode performance. #8201

Merged
merged 5 commits into from Mar 24, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -36,6 +36,7 @@
- `[jest-haste-map]` Avoid persisting haste map or processing files when not changed ([#8153](https://github.com/facebook/jest/pull/8153))
- `[jest-core]` Improve performance of SearchSource.findMatchingTests by 15% ([#8184](https://github.com/facebook/jest/pull/8184))
- `[jest-resolve]` Optimize internal cache lookup performance ([#8183](https://github.com/facebook/jest/pull/8183))
- `[jest-core]` Dramatically improve watch mode performance ([#8201](https://github.com/facebook/jest/pull/8201))

## 24.5.0

Expand Down
2 changes: 1 addition & 1 deletion packages/jest-config/src/__tests__/getMaxWorkers.test.ts
Expand Up @@ -32,7 +32,7 @@ describe('getMaxWorkers', () => {

it('Returns based on the number of cpus', () => {
expect(getMaxWorkers({})).toBe(3);
expect(getMaxWorkers({watch: true})).toBe(2);
expect(getMaxWorkers({watch: true})).toBe(3);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

was there any reason this was 1 less? @rogeliog do you know?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe the rationale for cpus/2 was that watch mode is typically used while doing other things (mostly editing files) and this was to make it less likely that the editor becomes slow / freezes?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we want to make it 1 less that's fine, but before it was halved! Just happened to be 1 less in this case.

On my machine, it was running 6 workers instead of 11.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know, stumbled over the formula as well some time ago and wondered if there's any experimental basis for it ^^
I'm fine with using cpus - 1 for watch mode as well. It's a good heuristic for what performs best when Jest is the only thing running, we can't really do much about sharing resources with other applications.
If you want your editor to remain responsive while running an expensive task, nice yarn jest --watch is a good idea anyway.

});

describe('% based', () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/jest-config/src/getMaxWorkers.ts
Expand Up @@ -32,6 +32,6 @@ export default function getMaxWorkers(
return parsed > 0 ? parsed : 1;
} else {
const cpus = os.cpus() ? os.cpus().length : 1;
return Math.max(argv.watch ? Math.floor(cpus / 2) : cpus - 1, 1);
return Math.max(cpus - 1, 1);
}
}
18 changes: 11 additions & 7 deletions packages/jest-haste-map/src/ModuleMap.ts
Expand Up @@ -32,8 +32,9 @@ export type SerializableModuleMap = {
};

export default class ModuleMap {
private readonly _raw: RawModuleMap;
static DuplicateHasteCandidatesError: typeof DuplicateHasteCandidatesError;
private readonly _raw: RawModuleMap;
private json: SerializableModuleMap | undefined;

constructor(raw: RawModuleMap) {
this._raw = raw;
Expand Down Expand Up @@ -84,12 +85,15 @@ export default class ModuleMap {
}

toJSON(): SerializableModuleMap {
return {
duplicates: Array.from(this._raw.duplicates),
map: Array.from(this._raw.map),
mocks: Array.from(this._raw.mocks),
rootDir: this._raw.rootDir,
};
if (!this.json) {
SimenB marked this conversation as resolved.
Show resolved Hide resolved
this.json = {
duplicates: Array.from(this._raw.duplicates),
map: Array.from(this._raw.map),
mocks: Array.from(this._raw.mocks),
rootDir: this._raw.rootDir,
};
}
return this.json;
}

static fromJSON(serializableModuleMap: SerializableModuleMap) {
Expand Down
4 changes: 0 additions & 4 deletions packages/jest-runner/src/__tests__/testRunner.test.js
Expand Up @@ -51,7 +51,6 @@ test('injects the serializable module map into each worker in watch mode', () =>
context: runContext,
globalConfig,
path: './file.test.js',
serializableModuleMap,
},
],
[
Expand All @@ -60,7 +59,6 @@ test('injects the serializable module map into each worker in watch mode', () =>
context: runContext,
globalConfig,
path: './file2.test.js',
serializableModuleMap,
},
],
]);
Expand Down Expand Up @@ -90,7 +88,6 @@ test('does not inject the serializable module map in serial mode', () => {
context: runContext,
globalConfig,
path: './file.test.js',
serializableModuleMap: null,
},
],
[
Expand All @@ -99,7 +96,6 @@ test('does not inject the serializable module map in serial mode', () => {
context: runContext,
globalConfig,
path: './file2.test.js',
serializableModuleMap: null,
},
],
]);
Expand Down
25 changes: 21 additions & 4 deletions packages/jest-runner/src/index.ts
Expand Up @@ -11,7 +11,7 @@ import exit from 'exit';
import throat from 'throat';
import Worker from 'jest-worker';
import runTest from './runTest';
import {worker} from './testWorker';
import {worker, SerializableResolver} from './testWorker';
import {
OnTestFailure,
OnTestStart,
Expand Down Expand Up @@ -103,11 +103,31 @@ class TestRunner {
onResult: OnTestSuccess,
onFailure: OnTestFailure,
) {
let resolvers: Map<string, SerializableResolver> | undefined = undefined;
if (watcher.isWatchMode()) {
resolvers = new Map();
for (const test of tests) {
if (!resolvers.has(test.context.config.name)) {
resolvers.set(test.context.config.name, {
config: test.context.config,
serializableModuleMap: test.context.moduleMap.toJSON(),
});
}
}
}

const worker = new Worker(TEST_WORKER_PATH, {
exposedMethods: ['worker'],
forkOptions: {stdio: 'pipe'},
maxRetries: 3,
numWorkers: this._globalConfig.maxWorkers,
setupArgs: resolvers
? [
{
serializableResolvers: Array.from(resolvers.values()),
},
]
: undefined,
}) as WorkerInterface;

if (worker.getStdout()) worker.getStdout().pipe(process.stdout);
Expand Down Expand Up @@ -135,9 +155,6 @@ class TestRunner {
},
globalConfig: this._globalConfig,
path: test.path,
serializableModuleMap: watcher.isWatchMode()
? test.context.moduleMap.toJSON()
: null,
});
});

Expand Down
62 changes: 36 additions & 26 deletions packages/jest-runner/src/testWorker.ts
Expand Up @@ -8,18 +8,23 @@

import {Config} from '@jest/types';
import {SerializableError, TestResult} from '@jest/test-result';
import HasteMap, {SerializableModuleMap, ModuleMap} from 'jest-haste-map';
import HasteMap, {ModuleMap, SerializableModuleMap} from 'jest-haste-map';
import exit from 'exit';
import {separateMessageFromStack} from 'jest-message-util';
import Runtime from 'jest-runtime';
import Resolver from 'jest-resolve';
import {ErrorWithCode, TestRunnerSerializedContext} from './types';
import runTest from './runTest';

export type SerializableResolver = {
config: Config.ProjectConfig;
serializableModuleMap: SerializableModuleMap;
};

type WorkerData = {
config: Config.ProjectConfig;
globalConfig: Config.GlobalConfig;
path: Config.Path;
serializableModuleMap: SerializableModuleMap | null;
context?: TestRunnerSerializedContext;
};

Expand Down Expand Up @@ -47,45 +52,50 @@ const formatError = (error: string | ErrorWithCode): SerializableError => {
};
};

const resolvers = Object.create(null);
const getResolver = (
config: Config.ProjectConfig,
moduleMap: ModuleMap | null,
) => {
// In watch mode, the raw module map with all haste modules is passed from
// the test runner to the watch command. This is because jest-haste-map's
// watch mode does not persist the haste map on disk after every file change.
// To make this fast and consistent, we pass it from the TestRunner.
if (moduleMap) {
return Runtime.createResolver(config, moduleMap);
} else {
const name = config.name;
if (!resolvers[name]) {
resolvers[name] = Runtime.createResolver(
const resolvers = new Map<string, Resolver>();
const getResolver = (config: Config.ProjectConfig, moduleMap?: ModuleMap) => {
const name = config.name;
if (moduleMap || !resolvers.has(name)) {
resolvers.set(
name,
Runtime.createResolver(
config,
Runtime.createHasteMap(config).readModuleMap(),
);
}
return resolvers[name];
moduleMap || Runtime.createHasteMap(config).readModuleMap(),
),
);
}
return resolvers.get(name)!;
};

export function setup(setupData?: {
serializableResolvers: Array<SerializableResolver>;
}) {
// Setup data is only used in watch mode to pass the latest version of all
// module maps that will be used during the test runs. Otherwise, module maps
// are loaded from disk as needed.
if (setupData) {
for (const {
config,
serializableModuleMap,
} of setupData.serializableResolvers) {
const moduleMap = HasteMap.ModuleMap.fromJSON(serializableModuleMap);
getResolver(config, moduleMap);
}
}
}

export async function worker({
config,
globalConfig,
path,
serializableModuleMap,
context,
}: WorkerData): Promise<TestResult> {
try {
const moduleMap = serializableModuleMap
? HasteMap.ModuleMap.fromJSON(serializableModuleMap)
: null;
return await runTest(
path,
globalConfig,
config,
getResolver(config, moduleMap),
getResolver(config),
context && {
...context,
changedFiles: context.changedFiles && new Set(context.changedFiles),
Expand Down