HasteMap with native maps #6960

rubennorte · 2018-09-11T19:02:35Z

Summary

This changes the data structure used to store the metadata for files, modules, mocks and duplicated modules in the Haste Map from Object (without prototypes) to Map.

The performance of Map is much better than Object to implement dictionaries when they contain a large amount of entries, which is usually what we have here. After testing this change in Facebook we've seen a ~20% reduction of the time to run all tests, which is a massive performance improvement for us.

This only modifies the internal data structures used by Jest and its end users will not be affected by it (other than performance). The change in the jest-haste-map package is breaking though, as it might affect other packages depending on it (including jest packages, which have been updated here).

Test plan

I've updated all tests related to this change (including a change to make the data structures more encapsulated for the rest of packages). I've also tested this in the Facebook infrastructure (including running all tests, running a subset of the tests, watch mode and coverage reporting).

SimenB · 2018-09-11T21:04:58Z

After testing this change in Facebook we've seen a ~20% reduction of the time to run all tests

😲 That's awesome!

SimenB · 2018-09-11T21:07:41Z

Can you update the changelog?

A question: is this change observable from the outside (aka a breaking change)?

rubennorte · 2018-09-11T21:20:45Z

@SimenB sorry I was meant to talk about that in the description. I'll add it now.

cpojer · 2018-09-11T21:35:37Z

packages/jest-haste-map/src/module_map.js

@@ -132,6 +161,14 @@ export default class ModuleMap {
      set,
    );
  }
+
+  static createEmpty() {


just create. It's implied that if you create one without arguments it would be empty.

cpojer · 2018-09-11T21:36:19Z

packages/jest-haste-map/src/module_map.js

 } from 'types/HasteMap';

 import H from './constants';

 const EMPTY_MAP = {};

+export opaque type SerializableModuleMap = {
+  // There isn't an easy way to extract the type of the entries of a Map


"There is no easier way […]"

cpojer · 2018-09-11T21:39:14Z

packages/jest-haste-map/src/index.js

-    const moduleMetadata = hasteMap.map[fileMetadata[H.ID]];
+    const fileMetadata = hasteMap.files.get(filePath);
+    if (!fileMetadata) {
+      // Should never happen


this comment is superfluous.

cpojer · 2018-09-11T21:39:36Z

packages/jest-haste-map/src/index.js

+    const fileMetadata = hasteMap.files.get(filePath);
+    if (!fileMetadata) {
+      // Should never happen
+      throw new Error('File to process was not found in the haste map');


Prefix with jest-haste-map: , end with a .

cpojer · 2018-09-11T21:39:48Z

packages/jest-haste-map/src/index.js

-              files: copy(hasteMap.files),
-              map: copy(hasteMap.map),
-              mocks: copy(hasteMap.mocks),
+              clocks: new Map(hasteMap.clocks),


This creates a shallow copy, right?

Yes, like copy did.

cpojer

This is super solid work @rubennorte. For historical context, when this code was initially written, v8.serialize did not exist and plain objects were much faster than Maps. I'm glad this change now not only improves startup time but also lookup time when resolving modules. Thanks for sending this PR :)

SimenB · 2018-09-12T11:44:27Z

CHANGELOG.md

@@ -3,6 +3,7 @@
 ### Features

 - `[babel-jest]` Add support for `babel.config.js` added in Babel 7.0.0 ([#6911](https://github.com/facebook/jest/pull/6911))
+- `[jest-haste-map]` Replaced internal data structures to improve performance ([#6960](https://github.com/facebook/jest/pull/6960))


Include **breaking**?

codecov-io · 2018-09-12T13:32:22Z

Codecov Report

Merging #6960 into master will decrease coverage by 0.08%.
The diff coverage is 89.28%.

@@            Coverage Diff             @@
##           master    #6960      +/-   ##
==========================================
- Coverage   66.97%   66.89%   -0.09%     
==========================================
  Files         250      250              
  Lines       10381    10422      +41     
  Branches        4        3       -1     
==========================================
+ Hits         6953     6972      +19     
- Misses       3427     3449      +22     
  Partials        1        1

Impacted Files	Coverage Δ
packages/jest-runner/src/index.js	`47.82% <ø> (ø)`	⬆️
packages/jest-runner/src/test_worker.js	`0% <0%> (ø)`	⬆️
packages/jest-haste-map/src/crawlers/node.js	`98.78% <100%> (ø)`	⬆️
packages/jest-haste-map/src/crawlers/watchman.js	`94.8% <100%> (-0.07%)`	⬇️
packages/jest-haste-map/src/module_map.js	`88.23% <100%> (+0.73%)`	⬆️
packages/jest-haste-map/src/haste_fs.js	`57.14% <83.33%> (+5.14%)`	⬆️
packages/jest-haste-map/src/index.js	`90.02% <96%> (-5.7%)`	⬇️
packages/jest-cli/src/lib/update_global_config.js	`95.45% <0%> (-0.2%)`	⬇️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 608b711...2ee6b7d. Read the comment docs.

RobinMalfait · 2018-09-13T12:00:35Z

packages/jest-haste-map/src/crawlers/__tests__/node.test.js

@@ -68,6 +68,7 @@ jest.mock('fs', () => {
 });

 const pearMatcher = path => /pear/.test(path);
+const createMap = obj => new Map(Object.keys(obj).map(key => [key, obj[key]]));


Random question from a stranger here:

Is Object.entries compiled / usable here?

const createMap = obj => new Map(Object.entries(obj));

I used it before but we don't include polyfills and we support Node 6 (which doesn't support it), so I had to remove it. Object.entries was first available in Node 7.

Nice! Thanks for the feedback.

jdalton · 2018-09-13T16:10:53Z

@rubennorte In what versions of Node do you see perf wins in?

rubennorte · 2018-09-14T11:36:01Z

@jdalton we mainly tested this in Node v9.11.2, but we saw it's also better in v10.10.0.

mxmaxime · 2018-09-30T17:07:03Z

Very interesting.
I think it's related to the v8 hash tables optimization (https://v8.dev/blog/hash-code). If so, this perf boost is in V8 v6.3+

nickshanks · 2018-11-16T11:04:54Z

"The performance of Map is much better than Object to implement dictionaries when they contain a large amount of entries" — Are there any public data showing when Map should be used over Object?

github-actions · 2021-05-12T06:06:53Z

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

rubennorte added 2 commits September 11, 2018 14:11

Replace Object with Map in jest-haste-map

8b8cc5c

Used custom serialization for the haste map in workers

a62f729

facebook-github-bot added the cla signed label Sep 11, 2018

rubennorte requested review from mjesun, cpojer and aaronabramov and removed request for aaronabramov September 11, 2018 19:05

cpojer reviewed Sep 11, 2018

View reviewed changes

cpojer approved these changes Sep 11, 2018

View reviewed changes

Improvements in naming and comments

6ca0a99

SimenB reviewed Sep 12, 2018

View reviewed changes

Add entry in the changelog

342f76a

rubennorte force-pushed the hastemap-with-native-maps branch from 742f16d to 342f76a Compare September 12, 2018 12:37

Remove Object.entries (node 6 compat)

2ee6b7d

rubennorte merged commit 9822231 into jestjs:master Sep 12, 2018

rubennorte deleted the hastemap-with-native-maps branch September 12, 2018 13:42

RobinMalfait reviewed Sep 13, 2018

View reviewed changes

SimenB mentioned this pull request Sep 15, 2018

jest-haste-map: Fixed Haste whitelist generation for scoped modules on Windows #6980

Merged

bd82 mentioned this pull request Sep 16, 2018

Micro Optimization - Use ES2015 Maps for Lookahead function Caching. Chevrotain/chevrotain#812

Closed

SimenB mentioned this pull request Sep 18, 2018

Jest performance is at best 2x slower than Jasmine, in our case 7x slower #6694

Open

SimenB mentioned this pull request Jan 9, 2019

TestSequencer heuristic using dependency tree file sizes #7553

Closed

rubennorte mentioned this pull request Mar 21, 2019

jest-resolve 18% performance optimization #8183

Merged

github-actions bot locked as resolved and limited conversation to collaborators May 12, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

HasteMap with native maps #6960

HasteMap with native maps #6960

rubennorte commented Sep 11, 2018 •

edited

Loading

SimenB commented Sep 11, 2018

SimenB commented Sep 11, 2018

rubennorte commented Sep 11, 2018

cpojer Sep 11, 2018

cpojer Sep 11, 2018

cpojer Sep 11, 2018

cpojer Sep 11, 2018

cpojer Sep 11, 2018

rubennorte Sep 12, 2018

cpojer left a comment

SimenB Sep 12, 2018

codecov-io commented Sep 12, 2018

RobinMalfait Sep 13, 2018

rubennorte Sep 13, 2018 •

edited

Loading

RobinMalfait Sep 13, 2018

jdalton commented Sep 13, 2018

rubennorte commented Sep 14, 2018 •

edited

Loading

mxmaxime commented Sep 30, 2018 •

edited

Loading

nickshanks commented Nov 16, 2018

github-actions bot commented May 12, 2021

HasteMap with native maps #6960

HasteMap with native maps #6960

Conversation

rubennorte commented Sep 11, 2018 • edited Loading

Summary

Test plan

SimenB commented Sep 11, 2018

SimenB commented Sep 11, 2018

rubennorte commented Sep 11, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cpojer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-io commented Sep 12, 2018

Codecov Report

Choose a reason for hiding this comment

rubennorte Sep 13, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jdalton commented Sep 13, 2018

rubennorte commented Sep 14, 2018 • edited Loading

mxmaxime commented Sep 30, 2018 • edited Loading

nickshanks commented Nov 16, 2018

github-actions bot commented May 12, 2021

rubennorte commented Sep 11, 2018 •

edited

Loading

rubennorte Sep 13, 2018 •

edited

Loading

rubennorte commented Sep 14, 2018 •

edited

Loading

mxmaxime commented Sep 30, 2018 •

edited

Loading