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
Changes from 1 commit
827a835
4fa81c5
245306e
aee567a
a8c3740
0449171
069ba34
4c4c10f
d051c44
4ee5860
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 |
---|---|---|
|
@@ -333,7 +333,12 @@ module.exports = ({ types: t }) => { | |
Function: { | ||
enter: earlyReturnTransform, | ||
|
||
exit(path) { | ||
exit(path, state) { | ||
const multiPass = state.opts.multiPass === false ? false : true; | ||
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. This is true by default which probably makes sense most of the time. 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. exit(path, { opts: { multipass = true } = {} } = {}) {
} ? 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. A bit too cryptic for my taste :) What do you think? 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. 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); | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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": "" , | ||
|
@@ -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); | ||
|
@@ -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, | ||
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. Not too happy with this duplication but as options grow, it will diverge more. Or should we create 2 proper separate presets? 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. we should enable preset options #54 and use multipass as a preset option ... 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. Ok, let's wait for that to merge then. 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. 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; | ||
}); | ||
|
@@ -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 = [ | ||
|
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.
I'm not sure if we can avoid this? Needed for benchmarks to be able to modify plugin options (such as simplify).