From 0d0d2e1468bfa08c5350f4262c98cce7438ed013 Mon Sep 17 00:00:00 2001 From: Scott Hovestadt Date: Mon, 18 Mar 2019 14:30:59 -0700 Subject: [PATCH 1/5] Haste map - avoid persisting haste map if not changed / processing files if not changed --- .../jest-haste-map/src/crawlers/watchman.ts | 5 + packages/jest-haste-map/src/index.ts | 118 +++++++++++------- 2 files changed, 78 insertions(+), 45 deletions(-) diff --git a/packages/jest-haste-map/src/crawlers/watchman.ts b/packages/jest-haste-map/src/crawlers/watchman.ts index 2512efab7540..3f575101aa6f 100644 --- a/packages/jest-haste-map/src/crawlers/watchman.ts +++ b/packages/jest-haste-map/src/crawlers/watchman.ts @@ -34,6 +34,7 @@ export = async function watchmanCrawl( options: CrawlerOptions, ): Promise<{ removedFiles: FileData; + changedFiles?: FileData; hasteMap: InternalHasteMap; }> { const fields = ['name', 'exists', 'mtime_ms', 'size']; @@ -148,6 +149,7 @@ export = async function watchmanCrawl( let files = data.files; let removedFiles = new Map(); + const changedFiles = new Map(); let watchmanFiles: Map; let isFresh = false; try { @@ -243,10 +245,12 @@ export = async function watchmanCrawl( absoluteVirtualFilePath, ); files.set(relativeVirtualFilePath, nextData); + changedFiles.set(relativeVirtualFilePath, nextData); } } } else { files.set(relativeFilePath, nextData); + changedFiles.set(relativeFilePath, nextData); } } } @@ -256,5 +260,6 @@ export = async function watchmanCrawl( return { hasteMap: data, removedFiles, + changedFiles: isFresh ? undefined : changedFiles, }; }; diff --git a/packages/jest-haste-map/src/index.ts b/packages/jest-haste-map/src/index.ts index 174c5e3baca2..9df8795bf87c 100644 --- a/packages/jest-haste-map/src/index.ts +++ b/packages/jest-haste-map/src/index.ts @@ -339,30 +339,42 @@ class HasteMap extends EventEmitter { build(): Promise { 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; + return this._watch(hasteMap).then(() => ({ + __hasteMapForTest, + hasteFS, + moduleMap, + })); + })(); } return this._buildPromise; } @@ -395,16 +407,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); } /** @@ -618,20 +633,34 @@ class HasteMap extends EventEmitter { .then(workerReply, workerError); } - private _buildHasteMap(data: { + private async _buildHasteMap(data: { removedFiles: FileData; + changedFiles?: FileData; hasteMap: InternalHasteMap; }): Promise { - 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) @@ -649,17 +678,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() { From 477d9eefb88566646615d8e551ba931d4f204726 Mon Sep 17 00:00:00 2001 From: Scott Hovestadt Date: Mon, 18 Mar 2019 15:57:56 -0700 Subject: [PATCH 2/5] Fix lint error. --- packages/jest-haste-map/src/crawlers/watchman.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/jest-haste-map/src/crawlers/watchman.ts b/packages/jest-haste-map/src/crawlers/watchman.ts index 3f575101aa6f..c6d6b66766d9 100644 --- a/packages/jest-haste-map/src/crawlers/watchman.ts +++ b/packages/jest-haste-map/src/crawlers/watchman.ts @@ -33,8 +33,8 @@ function WatchmanError(error: Error): Error { export = async function watchmanCrawl( options: CrawlerOptions, ): Promise<{ - removedFiles: FileData; changedFiles?: FileData; + removedFiles: FileData; hasteMap: InternalHasteMap; }> { const fields = ['name', 'exists', 'mtime_ms', 'size']; @@ -258,8 +258,8 @@ export = async function watchmanCrawl( data.files = files; return { + changedFiles: isFresh ? undefined : changedFiles, hasteMap: data, removedFiles, - changedFiles: isFresh ? undefined : changedFiles, }; }; From d9b37df03636b29de7c09f4162537a8e7ad584bc Mon Sep 17 00:00:00 2001 From: Scott Hovestadt Date: Tue, 19 Mar 2019 07:47:17 -0700 Subject: [PATCH 3/5] Use await for watch to keep code readable and consistent. --- packages/jest-haste-map/src/index.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/jest-haste-map/src/index.ts b/packages/jest-haste-map/src/index.ts index 9df8795bf87c..d21c63e3c251 100644 --- a/packages/jest-haste-map/src/index.ts +++ b/packages/jest-haste-map/src/index.ts @@ -369,11 +369,12 @@ class HasteMap extends EventEmitter { }); const __hasteMapForTest = (process.env.NODE_ENV === 'test' && hasteMap) || null; - return this._watch(hasteMap).then(() => ({ + await this._watch(hasteMap); + return { __hasteMapForTest, hasteFS, moduleMap, - })); + }; })(); } return this._buildPromise; From 97703c07e69a6069a65bc14f1a31bf2f66c4aa70 Mon Sep 17 00:00:00 2001 From: Scott Hovestadt Date: Tue, 19 Mar 2019 08:34:40 -0700 Subject: [PATCH 4/5] Update CHANGELOG.md --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 365621c5e1a0..0dbd0615fb1c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,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 From 952202684213c511d59bda440535ca70cde58847 Mon Sep 17 00:00:00 2001 From: Scott Hovestadt Date: Tue, 19 Mar 2019 09:14:30 -0700 Subject: [PATCH 5/5] Add assertions for watchman changed files. --- .../src/crawlers/__tests__/watchman.test.js | 27 ++++++++++++++----- 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/packages/jest-haste-map/src/crawlers/__tests__/watchman.test.js b/packages/jest-haste-map/src/crawlers/__tests__/watchman.test.js index ab454171f5bd..06f2d73543ec 100644 --- a/packages/jest-haste-map/src/crawlers/__tests__/watchman.test.js +++ b/packages/jest-haste-map/src/crawlers/__tests__/watchman.test.js @@ -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; @@ -165,6 +165,8 @@ describe('watchman watch', () => { }), ); + expect(changedFiles).toEqual(undefined); + expect(hasteMap.files).toEqual(mockFiles); expect(removedFiles).toEqual(new Map()); @@ -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], @@ -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); @@ -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], @@ -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); @@ -359,6 +368,8 @@ describe('watchman watch', () => { }), ); + expect(changedFiles).toEqual(undefined); + // strawberry and melon removed from the file list. expect(hasteMap.files).toEqual( createMap({ @@ -443,7 +454,7 @@ 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', @@ -451,6 +462,8 @@ describe('watchman watch', () => { }), ); + expect(changedFiles).toEqual(undefined); + expect(hasteMap.files).toEqual( createMap({ [KIWI_RELATIVE]: ['', 42, 52, 0, [], null], @@ -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; @@ -538,6 +551,8 @@ describe('watchman watch', () => { }), ); + expect(changedFiles).toEqual(new Map()); + expect(hasteMap.files).toEqual(new Map()); expect(removedFiles).toEqual(new Map());