Skip to content

Commit

Permalink
Perf improvements - avoid persisting haste map / processing files whe…
Browse files Browse the repository at this point in the history 
…n not changed. (#8153)

* Haste map - avoid persisting haste map if not changed / processing files if not changed

* Fix lint error.

* Use await for watch to keep code readable and consistent.

* Update CHANGELOG.md

* Add assertions for watchman changed files.
  • Loading branch information
@scotthovestadt
scotthovestadt committed Mar 19, 2019
1 parent 23e66a2 commit b48576a
Show file tree
Hide file tree
Showing 4 changed files with 102 additions and 51 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Expand Up @@ -25,6 +25,8 @@

### Performance

- `[jest-haste-map]` Avoid persisting haste map or processing files when not changed ([#8153](https://github.com/facebook/jest/pull/8153))

## 24.5.0

### Features
Expand Down
27 changes: 21 additions & 6 deletions packages/jest-haste-map/src/crawlers/__tests__/watchman.test.js
View file
Expand Up @@ -128,7 +128,7 @@ describe('watchman watch', () => {
ignore: pearMatcher,
rootDir: ROOT_MOCK,
roots: ROOTS,
}).then(({hasteMap, removedFiles}) => {
}).then(({changedFiles, hasteMap, removedFiles}) => {
const client = watchman.Client.mock.instances[0];
const calls = client.command.mock.calls;

Expand Down Expand Up @@ -165,6 +165,8 @@ describe('watchman watch', () => {
}),
);

expect(changedFiles).toEqual(undefined);

expect(hasteMap.files).toEqual(mockFiles);

expect(removedFiles).toEqual(new Map());
Expand Down Expand Up @@ -210,7 +212,8 @@ describe('watchman watch', () => {
: null,
rootDir: ROOT_MOCK,
roots: ROOTS,
}).then(({hasteMap, removedFiles}) => {
}).then(({changedFiles, hasteMap, removedFiles}) => {
expect(changedFiles).toEqual(undefined);
expect(hasteMap.files).toEqual(
createMap({
[path.join(DURIAN_RELATIVE, 'foo.1.js')]: ['', 33, 43, 0, [], null],
Expand Down Expand Up @@ -265,7 +268,7 @@ describe('watchman watch', () => {
ignore: pearMatcher,
rootDir: ROOT_MOCK,
roots: ROOTS,
}).then(({hasteMap, removedFiles}) => {
}).then(({changedFiles, hasteMap, removedFiles}) => {
// The object was reused.
expect(hasteMap.files).toBe(mockFiles);

Expand All @@ -275,6 +278,12 @@ describe('watchman watch', () => {
}),
);

expect(changedFiles).toEqual(
createMap({
[KIWI_RELATIVE]: ['', 42, 40, 0, [], null],
}),
);

expect(hasteMap.files).toEqual(
createMap({
[KIWI_RELATIVE]: ['', 42, 40, 0, [], null],
Expand Down Expand Up @@ -349,7 +358,7 @@ describe('watchman watch', () => {
ignore: pearMatcher,
rootDir: ROOT_MOCK,
roots: ROOTS,
}).then(({hasteMap, removedFiles}) => {
}).then(({changedFiles, hasteMap, removedFiles}) => {
// The file object was *not* reused.
expect(hasteMap.files).not.toBe(mockFiles);

Expand All @@ -359,6 +368,8 @@ describe('watchman watch', () => {
}),
);

expect(changedFiles).toEqual(undefined);

// strawberry and melon removed from the file list.
expect(hasteMap.files).toEqual(
createMap({
Expand Down Expand Up @@ -443,14 +454,16 @@ describe('watchman watch', () => {
ignore: pearMatcher,
rootDir: ROOT_MOCK,
roots: ROOTS,
}).then(({hasteMap, removedFiles}) => {
}).then(({changedFiles, hasteMap, removedFiles}) => {
expect(hasteMap.clocks).toEqual(
createMap({
[FRUITS_RELATIVE]: 'c:fake-clock:3',
[VEGETABLES_RELATIVE]: 'c:fake-clock:4',
}),
);

expect(changedFiles).toEqual(undefined);

expect(hasteMap.files).toEqual(
createMap({
[KIWI_RELATIVE]: ['', 42, 52, 0, [], null],
Expand Down Expand Up @@ -506,7 +519,7 @@ describe('watchman watch', () => {
ignore: pearMatcher,
rootDir: ROOT_MOCK,
roots: [...ROOTS, ROOT_MOCK],
}).then(({hasteMap, removedFiles}) => {
}).then(({changedFiles, hasteMap, removedFiles}) => {
const client = watchman.Client.mock.instances[0];
const calls = client.command.mock.calls;

Expand Down Expand Up @@ -538,6 +551,8 @@ describe('watchman watch', () => {
}),
);

expect(changedFiles).toEqual(new Map());

expect(hasteMap.files).toEqual(new Map());

expect(removedFiles).toEqual(new Map());
Expand Down
5 changes: 5 additions & 0 deletions packages/jest-haste-map/src/crawlers/watchman.ts
View file
Expand Up @@ -33,6 +33,7 @@ function WatchmanError(error: Error): Error {
export = async function watchmanCrawl(
options: CrawlerOptions,
): Promise<{
changedFiles?: FileData;
removedFiles: FileData;
hasteMap: InternalHasteMap;
}> {
Expand Down Expand Up @@ -148,6 +149,7 @@ export = async function watchmanCrawl(

let files = data.files;
let removedFiles = new Map();
const changedFiles = new Map();
let watchmanFiles: Map<string, any>;
let isFresh = false;
try {
Expand Down Expand Up @@ -243,17 +245,20 @@ export = async function watchmanCrawl(
absoluteVirtualFilePath,
);
files.set(relativeVirtualFilePath, nextData);
changedFiles.set(relativeVirtualFilePath, nextData);
}
}
} else {
files.set(relativeFilePath, nextData);
changedFiles.set(relativeFilePath, nextData);
}
}
}
}

data.files = files;
return {
changedFiles: isFresh ? undefined : changedFiles,
hasteMap: data,
removedFiles,
};
Expand Down
119 changes: 74 additions & 45 deletions packages/jest-haste-map/src/index.ts
View file
Expand Up @@ -339,30 +339,43 @@ class HasteMap extends EventEmitter {

build(): Promise<InternalHasteMapObject> {
if (!this._buildPromise) {
this._buildPromise = this._buildFileMap()
.then(data => this._buildHasteMap(data))
.then(hasteMap => {
this._buildPromise = (async () => {
const data = await this._buildFileMap();

// Persist when we don't know if files changed (changedFiles undefined)
// or when we know a file was changed or deleted.
let hasteMap: InternalHasteMap;
if (
data.changedFiles === undefined ||
data.changedFiles.size > 0 ||
data.removedFiles.size > 0
) {
hasteMap = await this._buildHasteMap(data);
this._persist(hasteMap);
} else {
hasteMap = data.hasteMap;
}

const rootDir = this._options.rootDir;
const hasteFS = new HasteFS({
files: hasteMap.files,
rootDir,
});
const moduleMap = new HasteModuleMap({
duplicates: hasteMap.duplicates,
map: hasteMap.map,
mocks: hasteMap.mocks,
rootDir,
});
const __hasteMapForTest =
(process.env.NODE_ENV === 'test' && hasteMap) || null;
return this._watch(hasteMap).then(() => ({
__hasteMapForTest,
hasteFS,
moduleMap,
}));
const rootDir = this._options.rootDir;
const hasteFS = new HasteFS({
files: hasteMap.files,
rootDir,
});
const moduleMap = new HasteModuleMap({
duplicates: hasteMap.duplicates,
map: hasteMap.map,
mocks: hasteMap.mocks,
rootDir,
});
const __hasteMapForTest =
(process.env.NODE_ENV === 'test' && hasteMap) || null;
await this._watch(hasteMap);
return {
__hasteMapForTest,
hasteFS,
moduleMap,
};
})();
}
return this._buildPromise;
}
Expand Down Expand Up @@ -395,16 +408,19 @@ class HasteMap extends EventEmitter {
/**
* 2. crawl the file system.
*/
private _buildFileMap(): Promise<{
private async _buildFileMap(): Promise<{
removedFiles: FileData;
changedFiles?: FileData;
hasteMap: InternalHasteMap;
}> {
const read = this._options.resetCache ? this._createEmptyMap : this.read;

return Promise.resolve()
.then(() => read.call(this))
.catch(() => this._createEmptyMap())
.then(hasteMap => this._crawl(hasteMap));
let hasteMap: InternalHasteMap;
try {
const read = this._options.resetCache ? this._createEmptyMap : this.read;
hasteMap = await read.call(this);
} catch {
hasteMap = this._createEmptyMap();
}
return this._crawl(hasteMap);
}

/**
Expand Down Expand Up @@ -618,20 +634,34 @@ class HasteMap extends EventEmitter {
.then(workerReply, workerError);
}

private _buildHasteMap(data: {
private async _buildHasteMap(data: {
removedFiles: FileData;
changedFiles?: FileData;
hasteMap: InternalHasteMap;
}): Promise<InternalHasteMap> {
const {removedFiles, hasteMap} = data;
const map = new Map();
const mocks = new Map();
const promises = [];
const {removedFiles, changedFiles, hasteMap} = data;

// If any files were removed or we did not track what files changed, process
// every file looking for changes. Otherwise, process only changed files.
let map: ModuleMapData;
let mocks: MockData;
let filesToProcess: FileData;
if (changedFiles === undefined || removedFiles.size) {
map = new Map();
mocks = new Map();
filesToProcess = hasteMap.files;
} else {
map = hasteMap.map;
mocks = hasteMap.mocks;
filesToProcess = changedFiles;
}

for (const [relativeFilePath, fileMetadata] of removedFiles) {
this._recoverDuplicates(hasteMap, relativeFilePath, fileMetadata[H.ID]);
}

for (const relativeFilePath of hasteMap.files.keys()) {
const promises = [];
for (const relativeFilePath of filesToProcess.keys()) {
if (
this._options.skipPackageJson &&
relativeFilePath.endsWith(PACKAGE_JSON)
Expand All @@ -649,17 +679,16 @@ class HasteMap extends EventEmitter {
}
}

return Promise.all(promises)
.then(() => {
this._cleanup();
hasteMap.map = map;
hasteMap.mocks = mocks;
return hasteMap;
})
.catch(error => {
this._cleanup();
return Promise.reject(error);
});
try {
await Promise.all(promises);
this._cleanup();
hasteMap.map = map;
hasteMap.mocks = mocks;
return hasteMap;
} catch (error) {
this._cleanup();
throw error;
}
}

private _cleanup() {
Expand Down

0 comments on commit b48576a

Please sign in to comment.