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

Add React benchmarking infrastructure #9465

Merged
merged 71 commits into from
May 9, 2017
Merged
Show file tree
Hide file tree
Changes from 65 commits
Commits
Show all changes
71 commits
Select commit Hold shift + click to select a range
efdd44a
Initial commit for WIP benchmarking infrastructure
trueadm Apr 20, 2017
f4bc2f7
fixed lint issues and ran prettier
trueadm Apr 20, 2017
6c02ebe
added <rootDir>/scripts/bench/ to ignore paths for Jest
trueadm Apr 20, 2017
0b09816
Merge master into remote
gaearon Apr 20, 2017
4b7ef00
tidied up code and fixed a few bugs in the runner.js
trueadm Apr 20, 2017
941745a
Merge branch 'add_benchmarking_infra' of github.com:trueadm/react int…
trueadm Apr 20, 2017
c1e34bc
fixed eslint
trueadm Apr 20, 2017
fd86c20
improved the benchmark output from the runner
trueadm Apr 20, 2017
d381fb4
fixed typo
trueadm Apr 20, 2017
c1b992f
tided up print output in runner.js
trueadm Apr 20, 2017
44395a3
throw error if chrome canary is not installed on mac
trueadm Apr 21, 2017
632274f
fix yarn.lock
trueadm Apr 21, 2017
efa63c2
added better bench stats output (tables)
trueadm Apr 21, 2017
a1b17c1
added benchmark diff to table results
trueadm Apr 22, 2017
9585a02
adds bundle size comparisons to results
trueadm Apr 24, 2017
1c768b9
Merge remote-tracking branch 'upstream/master' into add_benchmarking_…
trueadm Apr 24, 2017
76584fe
tidied up the results
trueadm Apr 24, 2017
5de7727
fixed prettier output
trueadm Apr 24, 2017
07901f9
attempt to trigger bech for circleci build
trueadm Apr 25, 2017
366e4c0
fixed merge conflict
trueadm Apr 25, 2017
2e354c0
fixes flow exlclusion for lighthouse module
trueadm Apr 25, 2017
083623e
added class components benchmark
trueadm Apr 25, 2017
2a9cb20
cleaned up stats.js
trueadm Apr 25, 2017
750f611
stability changes
trueadm Apr 25, 2017
414a714
circleci node version to 7
trueadm Apr 25, 2017
f53c359
added another benchmark
trueadm Apr 25, 2017
ab2601a
added colours to the different benchmarks to check if being cached
trueadm Apr 25, 2017
4582574
force no-cache headers
trueadm Apr 25, 2017
cadc674
added more info messages
trueadm Apr 25, 2017
e7f02d9
refactor chrome launching.
paulirish Apr 25, 2017
d486cfc
Merge pull request #1 from paulirish/add_benchmarking_infra
trueadm Apr 25, 2017
d076faa
fixed an issue where launcher.kill might fail
trueadm Apr 25, 2017
6c86fa3
Move server to runner. Launch it only once.
paulirish Apr 25, 2017
0503d5a
Merge pull request #2 from paulirish/servermove
trueadm Apr 25, 2017
0d332a6
tidy up
trueadm Apr 25, 2017
ae4ad2b
changes the logic in how the remote repo is checked out
trueadm Apr 26, 2017
d7d65d0
removes bench from circleci build
trueadm Apr 26, 2017
2c39f8f
removed colors from benchmarks (no longer needed)
trueadm Apr 26, 2017
542b015
added CI integration comment
trueadm Apr 26, 2017
d8cad69
added hacker news benchmark
trueadm Apr 26, 2017
68aa0e0
fixed merge conflict
trueadm Apr 26, 2017
1f25566
added skipBuild functionality
trueadm Apr 26, 2017
20284d5
relabelled remote
trueadm Apr 26, 2017
2c3e684
Add confidence intervals
sophiebits Apr 27, 2017
98bb3bf
Merge remote-tracking branch 'upstream/master' into add_benchmarking_…
trueadm Apr 27, 2017
c9a7a84
added first meaningful paint
trueadm Apr 27, 2017
700a9d0
removed some unused code
trueadm Apr 27, 2017
58df943
reverted code.json
trueadm Apr 27, 2017
38f5b97
updated benchmark runs back to 10
trueadm Apr 27, 2017
e06498c
no longer breaks when results contain missing bundles
trueadm Apr 27, 2017
ceb8421
adds CPU throttling
trueadm Apr 27, 2017
91aeb0d
renamed build to remote-repo
trueadm Apr 28, 2017
9320a7a
renamed build to remote-repo
trueadm Apr 28, 2017
8c5405b
small fix to build
trueadm Apr 28, 2017
49456da
Merge branch 'add_benchmarking_infra' of github.com:trueadm/react int…
trueadm Apr 28, 2017
f6a68d8
fixed bad merge
trueadm Apr 28, 2017
a09b75e
upped runs to 10 from 2 again
trueadm Apr 28, 2017
d8fa237
properly pulls master
trueadm Apr 28, 2017
b13171a
removes old-bench
trueadm Apr 28, 2017
ac3471c
fixes merge conflict
trueadm May 3, 2017
4962a4d
runs benchmarks in headless mode
trueadm May 3, 2017
586f83a
adds a --headless option
trueadm May 3, 2017
eaff2e2
improved the git build process
trueadm May 3, 2017
8fdafc5
added README
trueadm May 3, 2017
f52440b
Merge branch 'master' into add_benchmarking_infra
gaearon May 5, 2017
d377dfc
updated based feedback from review
trueadm May 5, 2017
f29856a
adds merge base commit sha
trueadm May 5, 2017
fc38448
addressing more PR feedback
trueadm May 5, 2017
272d6c7
remove built JS react files
trueadm May 5, 2017
6788752
updated .gitignore
trueadm May 5, 2017
f9443ee
added combined bundle load times to the metrics
trueadm May 9, 2017
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .eslintignore
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ fixtures/
# Ignore built files.
build/
coverage/
scripts/bench/bench-*.js
scripts/bench/benchmarks/**/*.js
scripts/old-bench/bench-*.js
Copy link
Collaborator

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.

vendor/*
**/node_modules
2 changes: 2 additions & 0 deletions .flowconfig
Original file line number Diff line number Diff line change
Expand Up @@ -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/*
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this end with .*?

<PROJECT_ROOT>/.*/__mocks__/.*
<PROJECT_ROOT>/.*/__tests__/.*

Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ static
_SpecRunner.html
__benchmarks__
build/
remote-repo/
coverage/
.module-cache
*.gem
Expand Down
2 changes: 1 addition & 1 deletion circle.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
machine:
timezone: America/Los_Angeles
node:
version: 6
version: 7
Copy link
Collaborator

Choose a reason for hiding this comment

The 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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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:
Expand Down
11 changes: 10 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Copy link
Collaborator

@gaearon gaearon May 5, 2017

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -112,7 +120,8 @@
"modulePathIgnorePatterns": [
"/.module-cache/",
"<rootDir>/build/",
"<rootDir>/scripts/rollup/shims/"
"<rootDir>/scripts/rollup/shims/",
"<rootDir>/scripts/bench/"
],
"rootDir": "",
"transform": {
Expand Down
52 changes: 21 additions & 31 deletions scripts/bench/README.md
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)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Typo: compare (here and below).
What does "(default run)" mean?

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
Copy link
Collaborator

Choose a reason for hiding this comment

The 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
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't fully understand what these two commands do. (--local and --remote)
If I run --local will it not benchmark remote at all? Or will it just skip the build for remote?

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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
Copy link
Collaborator

Choose a reason for hiding this comment

The 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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Do they recommend to use it for perf testing?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Meaning the variance was higher?

Copy link
Contributor Author

@trueadm trueadm May 9, 2017

Choose a reason for hiding this comment

The 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
```
111 changes: 0 additions & 111 deletions scripts/bench/analyze.py

This file was deleted.

Loading