-
Notifications
You must be signed in to change notification settings - Fork 47.5k
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
Add React benchmarking infrastructure #9465
Changes from 65 commits
efdd44a
f4bc2f7
6c02ebe
0b09816
4b7ef00
941745a
c1e34bc
fd86c20
d381fb4
c1b992f
44395a3
632274f
efa63c2
a1b17c1
9585a02
1c768b9
76584fe
5de7727
07901f9
366e4c0
2e354c0
083623e
2a9cb20
750f611
414a714
f53c359
ab2601a
4582574
cadc674
e7f02d9
d486cfc
d076faa
6c86fa3
0503d5a
0d332a6
ae4ad2b
d7d65d0
2c39f8f
542b015
d8cad69
68aa0e0
1f25566
20284d5
2c3e684
98bb3bf
c9a7a84
700a9d0
58df943
38f5b97
e06498c
ceb8421
91aeb0d
9320a7a
8c5405b
49456da
f6a68d8
a09b75e
d8fa237
b13171a
ac3471c
4962a4d
586f83a
eaff2e2
8fdafc5
f52440b
d377dfc
f29856a
fc38448
272d6c7
6788752
f9443ee
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,8 @@ | |
<PROJECT_ROOT>/build/.* | ||
<PROJECT_ROOT>/scripts/.* | ||
<PROJECT_ROOT>/.*/node_modules/y18n/.* | ||
<PROJECT_ROOT>/node_modules/chrome-devtools-frontend/.* | ||
<PROJECT_ROOT>/node_modules/devtools-timeline-model/* | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this end with |
||
<PROJECT_ROOT>/.*/__mocks__/.* | ||
<PROJECT_ROOT>/.*/__tests__/.* | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,7 @@ static | |
_SpecRunner.html | ||
__benchmarks__ | ||
build/ | ||
remote-repo/ | ||
coverage/ | ||
.module-cache | ||
*.gem | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,7 +2,7 @@ | |
machine: | ||
timezone: America/Los_Angeles | ||
node: | ||
version: 6 | ||
version: 7 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why did we change this? We're not running benches on CI yet, are we? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Node 7 should be faster than Node 6. Not a big deal though. |
||
ruby: | ||
version: 2.2.3 | ||
environment: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -60,16 +60,22 @@ | |
"git-branch": "^0.3.0", | ||
"glob": "^6.0.4", | ||
"glob-stream": "^6.1.0", | ||
"google-closure-compiler-js": "^20170409.0.0", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it used currently, or was it added for future experimentation? If not used let's remove it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can be removed, was used for static-www build |
||
"gzip-js": "~0.3.2", | ||
"gzip-size": "^3.0.0", | ||
"http-server": "^0.9.0", | ||
"http2": "^3.3.6", | ||
"jest": "^19.0.1", | ||
"jest-config": "^19.0.1", | ||
"jest-jasmine2": "^19.0.1", | ||
"jest-runtime": "^19.0.1", | ||
"lighthouse": "^1.6.3", | ||
"loose-envify": "^1.1.0", | ||
"merge-stream": "^1.0.0", | ||
"mime": "^1.3.4", | ||
"minimist": "^1.2.0", | ||
"ncp": "^2.0.0", | ||
"nodegit": "^0.18.0", | ||
"object-assign": "^4.1.1", | ||
"platform": "^1.1.0", | ||
"prettier": "^1.2.2", | ||
|
@@ -84,6 +90,7 @@ | |
"rollup-plugin-replace": "^1.1.1", | ||
"rollup-plugin-uglify": "^1.0.1", | ||
"run-sequence": "^1.1.4", | ||
"stats-analysis": "^2.0.0", | ||
"through2": "^2.0.0", | ||
"tmp": "~0.0.28", | ||
"typescript": "~1.8.10", | ||
|
@@ -99,6 +106,7 @@ | |
"version": 7 | ||
}, | ||
"scripts": { | ||
"bench": "npm run version-check && node scripts/bench/runner.js", | ||
"build": "npm run version-check && node scripts/rollup/build.js", | ||
"linc": "git diff --name-only --diff-filter=ACMRTUB `git merge-base HEAD master` | grep '\\.js$' | xargs eslint --", | ||
"lint": "node ./scripts/tasks/eslint.js", | ||
|
@@ -112,7 +120,8 @@ | |
"modulePathIgnorePatterns": [ | ||
"/.module-cache/", | ||
"<rootDir>/build/", | ||
"<rootDir>/scripts/rollup/shims/" | ||
"<rootDir>/scripts/rollup/shims/", | ||
"<rootDir>/scripts/bench/" | ||
], | ||
"rootDir": "", | ||
"transform": { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,40 +1,30 @@ | ||
Work-in-progress benchmarks. | ||
# React Benchmarking | ||
|
||
## Running the suite | ||
## Commands | ||
|
||
You'll need two folders to compare, each of them containing `react.min.js` and `react-dom-server.min.js`. You can run `npm run build` at the repo root to get a `build` folder with these files. | ||
```bash | ||
# will comapre local repo vs remote merge base repo (default run) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Typo: compare (here and below). |
||
yarn bench | ||
|
||
For example, if you want to compare a stable verion against master, you can create folders called `build-stable` and `build-master` and use the benchmark scripts like this: | ||
# will comapre local repo vs remote merge base repo | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: type here too |
||
# this can significantly improve bench times due to no build | ||
yarn bench -- --skip-build | ||
|
||
``` | ||
$ ./measure.py build-stable stable.txt build-master master.txt | ||
$ ./analyze.py stable.txt master.txt | ||
``` | ||
# will only build and run local repo against benchmarks | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't fully understand what these two commands do. ( If it doesn't run remote, how would I interpret the benchmark? Does it mean I'll only get absolute numbers as there's nothing to compare to? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, it's only for absolute values |
||
yarn bench -- --local | ||
|
||
The test measurements (second argument to `analyze`, `master.txt` in this example) will be compared to the control measurements (first argument to `analyze`, `stable.txt` in this example). | ||
# will only build and run remote merge base repo against benchmarks | ||
yarn bench -- --remote | ||
|
||
Changes with the `-` sign in the output mean `master` is faster than `stable`. | ||
# will only build and run remote master repo against benchmarks | ||
yarn bench -- --remote=master | ||
|
||
You can name folders any way you like, this was just an example. | ||
# same as default | ||
yarn bench -- --remote --local | ||
|
||
## Running one | ||
One thing you can do with them is benchmark initial render time for a realistic hierarchy: | ||
# runs benchmarks with Chrome in headless mode | ||
yarn bench -- --headless | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it better? Should it be the default? When would you want to use it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To me, the results weren't as effective by default. Headless runs everything much faster (it seems) so it was hard to get good feedback and it seems like it's still a new feature. We can revisit adding this by default in the future. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do they recommend to use it for perf testing? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've had no official say-so for perf testing. I'll chase someone to get some feedback on using it before enabling it by default. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Meaning the variance was higher? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The variance was all over the place in some runs and better in others. It was too unreliable and no one could tell me as why this is currently the case with headless mode. |
||
|
||
``` | ||
$ which jsc | ||
/System/Library/Frameworks/JavaScriptCore.framework/Versions/Current/Resources/jsc | ||
$ jsc react-0.14.0.min.js bench-pe-es5.js -e 'var START=Date.now(); React.renderToString(React.createElement(Benchmark)); var END=Date.now(); print(END-START);' | ||
45 | ||
``` | ||
|
||
Substitute `js` or `v8` for `jsc` to use SpiderMonkey or V8, respectively, if you've installed them. | ||
|
||
## Creating one | ||
|
||
To create one, copy `extract-component.js` to your clipboard and paste it into the Chrome console on facebook.com, perhaps after changing the root ID if you don't want the tree with ID `.0`. | ||
|
||
Then to convert it to ES5: | ||
|
||
``` | ||
babel --whitelist react,react.displayName --compact false bench-pe.js >bench-pe-es5.js | ||
``` | ||
# runs only specific string matching benchmarks | ||
yarn bench -- --benchmark=hacker | ||
``` |
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like this can be removed.