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 butternut to benchmarks #527

Merged
merged 3 commits into from
May 11, 2017
Merged

Add butternut to benchmarks #527

merged 3 commits into from
May 11, 2017

Conversation

boopathi
Copy link
Member

@boopathi boopathi commented May 10, 2017

@codecov
Copy link

codecov bot commented May 10, 2017

Codecov Report

Merging #527 into master will increase coverage by 0.03%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #527      +/-   ##
==========================================
+ Coverage   83.22%   83.26%   +0.03%     
==========================================
  Files          34       34              
  Lines        2539     2539              
  Branches      908      908              
==========================================
+ Hits         2113     2114       +1     
+ Misses        255      254       -1     
  Partials      171      171
Impacted Files Coverage Δ
packages/babel-plugin-minify-simplify/src/index.js 80.74% <0%> (+0.12%) ⬆️

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 673c617...de0b1b5. Read the comment docs.

@boopathi boopathi added the Tag: Internal Pull Request changing project internals - code that is NOT published label May 10, 2017
README.md Outdated
| **closureCompiler** | 34.77kB | 72% | 11.96kB | 60% | 2.62 | 3940.01 |
| **closureCompilerJs** | 65.41kB | 48% | 15.83kB | 47% | 2.96 | 1049.21 |
| **babili** | 35.86kB | 71% | 12.47kB | 58% | 8.14 | 1418.56 |
| **uglify** | 35.74kB | 71% | 11.97kB | 60% | 2.60 | 676.57 |
Copy link
Member

Choose a reason for hiding this comment

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

lets add uglify-es as well?

@@ -128,6 +130,9 @@ class Benchmark {
const out = compile(flags);
return out.compiledCode;
}
butternut(code) {
return butternut.squash(code, { check: true }).code;

Choose a reason for hiding this comment

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

Using the check: true option is a good idea to check that Butternut is producing valid code, but it shouldn't be used for benchmarking as it causes the output to be parsed again.

Also, Butternut generates a sourcemap by default. Since Babili and Uglify don't, we should probably disable sourcemaps for Butternut too to remove that from the equation:

-return butternut.squash(code, { check: true }).code;
+return butternut.squash(code, { sourceMap: false }).code;

Copy link
Member Author

Choose a reason for hiding this comment

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

👍

+ Remove check:true as it re-runs
+ Add sourceMaps false as by default others don't generate sourcemaps,
but butternut does
+ Update README
@boopathi boopathi merged commit e2710cf into master May 11, 2017
@boopathi boopathi deleted the butternut-0 branch May 11, 2017 13:35
@darahak
Copy link

darahak commented May 11, 2017

What is the rule for using bold text in the benchmark tables? Highlight best results?
It seems inconsistent.

For example, in the first table for React: Closure Compiler has the best results for output size, but not for parse time and minify time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Tag: Internal Pull Request changing project internals - code that is NOT published
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants