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

Improve benchmarks accuracy #148

Merged
merged 10 commits into from Nov 27, 2016
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 17 additions & 1 deletion package.json
Expand Up @@ -47,6 +47,22 @@
"lerna": "2.0.0-beta.26",
"lerna-changelog": "^0.2.1",
"through2": "^2.0.1",
"uglify-js": "^2.7.3"
"uglify-js": "^2.7.3",

"babel-plugin-minify-constant-folding": "^0.0.1",
"babel-plugin-minify-dead-code-elimination": "^0.0.1",
"babel-plugin-minify-flip-comparisons": "^0.0.1",
"babel-plugin-minify-guarded-expressions": "^0.0.1",
"babel-plugin-minify-infinity": "^0.0.1",
"babel-plugin-minify-mangle-names": "^0.0.2",
"babel-plugin-minify-replace": "^0.0.1",
"babel-plugin-minify-simplify": "^0.0.2",
"babel-plugin-minify-type-constructors": "^0.0.1",
"babel-plugin-transform-member-expression-literals": "^6.8.0",
"babel-plugin-transform-merge-sibling-variables": "^6.8.0",
"babel-plugin-transform-minify-booleans": "^6.8.0",
"babel-plugin-transform-property-literals": "^6.8.0",
"babel-plugin-transform-simplify-comparison-operators": "^6.8.0",
"babel-plugin-transform-undefined-to-void": "^6.8.0"
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure if we can avoid this? Needed for benchmarks to be able to modify plugin options (such as simplify).

}
}
Expand Up @@ -6,7 +6,9 @@ const unpad = require("../../../utils/unpad");

function transform(code) {
return babel.transform(code, {
plugins: [plugin],
plugins: [
[plugin, { multiPass: true }],
],
}).code;
}

Expand Down
7 changes: 6 additions & 1 deletion packages/babel-plugin-minify-simplify/src/index.js
Expand Up @@ -333,7 +333,12 @@ module.exports = ({ types: t }) => {
Function: {
enter: earlyReturnTransform,

exit(path) {
exit(path, state) {
const multiPass = state.opts.multiPass === false ? false : true;
Copy link
Member Author

Choose a reason for hiding this comment

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

This is true by default which probably makes sense most of the time.

Copy link
Member

Choose a reason for hiding this comment

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

exit(path, { opts: { multipass = true } = {} } = {}) {
}

?

Copy link
Member Author

Choose a reason for hiding this comment

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

A bit too cryptic for my taste :) What do you think?

Copy link
Member

@boopathi boopathi Sep 12, 2016

Choose a reason for hiding this comment

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

For this (putting everything in one line and just one 2-level-deep option) it looks cryptic, actually using this is much clear sometimes, We could use default destructuring assignment when we have more options.

if (!multiPass) {
return;
}

// Useful to do on enter and exit because more oppurtinties can open.
earlyReturnTransform(path);

Expand Down
77 changes: 68 additions & 9 deletions scripts/benchmark.js
Expand Up @@ -14,14 +14,16 @@ const fs = require("fs");
const path = require("path");
const compile = require("google-closure-compiler-js").compile;

const NUM_TEST_RUNS = 10;

const filename = process.argv[2];
if (!filename) {
console.error("Error: No filename specified");
process.exit(1);
}

const table = new Table({
head: ["", "raw", "raw win", "gzip", "gzip win", "parse time", "run"],
head: ["", "raw", "raw win", "gzip", "gzip win", "parse time", "run time (average)"],
chars: {
top: "",
"top-mid": "" ,
Expand Down Expand Up @@ -52,7 +54,8 @@ const code = fs.readFileSync(filename, "utf8");
const gzippedCode = zlib.gzipSync(code);

function test(name, callback) {
console.log("testing", name);

console.log('testing', name);

const start = Date.now();
const result = callback(code);
Expand All @@ -64,24 +67,82 @@ function test(name, callback) {
const gzipped = zlib.gzipSync(result);

const parseStart = Date.now();
// disable for now we have a failing test in dce
new Function(result);
const parseEnd = Date.now();
const parseNow = parseEnd - parseStart;

const runTimes = [run];

for (var i = 1; i < NUM_TEST_RUNS; i++) {
const start = Date.now();
const result = callback(code);
runTimes.push(Date.now() - start);
}

const totalTime = runTimes.reduce((a, b) => a + b, 0);
const average = parseInt(totalTime / runTimes.length, 10);

results.push({
name: name,
raw: result.length,
gzip: gzipped.length,
parse: parseNow,
run: run,
run: average,
});
}

test("babili", function (code) {
test("babili (best size)", function (code) {
return babel.transform(code, {
sourceType: "script",
presets: [require("../packages/babel-preset-babili")],
minified: true,
plugins: [
require("babel-plugin-minify-constant-folding"),
require("babel-plugin-minify-dead-code-elimination"),
require("babel-plugin-minify-flip-comparisons"),
require("babel-plugin-minify-guarded-expressions"),
require("babel-plugin-minify-infinity"),
require("babel-plugin-minify-mangle-names"),
require("babel-plugin-minify-replace"),
[
require("../packages/babel-plugin-minify-simplify/lib/index.js"),
{ multiPass: true }
],
require("babel-plugin-minify-type-constructors"),
require("babel-plugin-transform-member-expression-literals"),
require("babel-plugin-transform-merge-sibling-variables"),
require("babel-plugin-transform-minify-booleans"),
require("babel-plugin-transform-property-literals"),
require("babel-plugin-transform-simplify-comparison-operators"),
require("babel-plugin-transform-undefined-to-void"),
],
comments: false,
Copy link
Member Author

Choose a reason for hiding this comment

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

Not too happy with this duplication but as options grow, it will diverge more.

Or should we create 2 proper separate presets?

Copy link
Member

@boopathi boopathi Sep 10, 2016

Choose a reason for hiding this comment

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

we should enable preset options #54 and use multipass as a preset option ...

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, let's wait for that to merge then.

Copy link
Member

Choose a reason for hiding this comment

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

Or we can land this as two separate things. Multipass option in the plugin and benchmarks in another.

}).code;
});

test("babili (best speed)", function (code) {
return babel.transform(code, {
sourceType: "script",
minified: true,
plugins: [
require("babel-plugin-minify-constant-folding"),
require("babel-plugin-minify-dead-code-elimination"),
require("babel-plugin-minify-flip-comparisons"),
require("babel-plugin-minify-guarded-expressions"),
require("babel-plugin-minify-infinity"),
require("babel-plugin-minify-mangle-names"),
require("babel-plugin-minify-replace"),
[
require("../packages/babel-plugin-minify-simplify/lib/index.js"),
{ multiPass: false }
],
require("babel-plugin-minify-type-constructors"),
require("babel-plugin-transform-member-expression-literals"),
require("babel-plugin-transform-merge-sibling-variables"),
require("babel-plugin-transform-minify-booleans"),
require("babel-plugin-transform-property-literals"),
require("babel-plugin-transform-simplify-comparison-operators"),
require("babel-plugin-transform-undefined-to-void"),
],
comments: false,
}).code;
});
Expand All @@ -107,9 +168,7 @@ test("uglify", function (code) {
}).code;
});

results = results.sort(function (a, b) {
return a.gzip > b.gzip;
});
results = results.sort((a, b) => a.gzip > b.gzip);

results.forEach(function (result, i) {
let row = [
Expand Down