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.

Sign up for GitHub

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 std/esm benchmark. #33

Closed
wants to merge 1 commit into from
Closed

Add std/esm benchmark. #33

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 3 additions & 0 deletions .esmrc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
Copy link
Member

Choose a reason for hiding this comment

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

This file is not bundled with the webpack build? Can we avoid external configuration files?

Copy link
Author

Choose a reason for hiding this comment

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

Yep, I can remove it and specify options via its API.

esm: "js"
}
2 changes: 2 additions & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
"license": "BSD-3-Clause",
"dependencies": {
"@babel/standalone": "7.0.0-beta.32",
"@std/esm": "^0.21.1",
Copy link
Member

Choose a reason for hiding this comment

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

We use exact dependencies.

Copy link
Author

@jdalton jdalton Feb 9, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for the pull request. There's one thing I'm wondering about though: Does @std/esm count as a web developer tool? And does it even make sense to consider outside of Node?

Not outside Node no. It's Node for sure. Is there a more appropriate suite?

The suite description states

This is a benchmark suite designed to measure the JavaScript related workloads commonly used by Web Developers nowadays, for example the core workloads in popular tools like Babel or TypeScript.

Lodash is on more than 2 million websites and impacts over 150,000 npm packages (many of those used by web devs). Since this will be used to load it and others, in the Node context, it seems like a good fit.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, good point. I do think we should have a lodash benchmark in there for sure. lodash is one of the most popular npm (frontend) dependencies (if not the most popular).

As for @std/esm, we should probably create a Node tooling benchmark for Node only/mostly packages. WDYT?

Copy link
Member

Choose a reason for hiding this comment

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

Do you have a good lodash benchmark suite already?

Copy link
Author

@jdalton jdalton Feb 9, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Do you have a good lodash benchmark suite already?

I have a pretty robust one. It allows swapping out different versions of Lodash/Underscore. It's a long running benchmark though designed to ensure the things Lodash is good at stay good and the things it's bad at stay reasonable.

As for @std/esm, we should probably create a Node tooling benchmark for Node only/mostly packages. WDYT

That's cool too though so testing the Node flavors of Babel, Webpack, etc.?

Copy link
Member

Choose a reason for hiding this comment

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

I'm very interested in the lodash benchmark. As long as the runtime is roughly comparable to the other tests it should be fine, and if not we could reduce the iteration count a bit I guess.

That's cool too though so testing the Node flavors of Babel, Webpack, etc.?

I don't think it makes sense to have webpack there. This is definitely something that should be owned and operated by the benchmarking WG. Maybe you can join the call on Monday to discuss the idea?

Copy link
Author

Choose a reason for hiding this comment

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

As long as the runtime is roughly comparable to the other tests it should be fine, and if not we could reduce the iteration count a bit I guess.

It'd need some tweaking it's a pretty massive suite.

This is definitely something that should be owned and operated by the benchmarking WG. Maybe you can join the call on Monday to discuss the idea?

Sure thing! You can DM details if you'd like.

Copy link
Member

Choose a reason for hiding this comment

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

It'd need some tweaking it's a pretty massive suite.

It also seems to mess with the global state of benchmark.js, which needs to be fixed.

Sure thing! You can DM details if you'd like.

Added note to nodejs/benchmarking#198 for monday's meeting.

"acorn": "5.2.1",
"babylon": "7.0.0-beta.32",
"benchmark": "^2.1.4",
Expand All @@ -45,6 +46,7 @@
"esprima": "4.0.0",
"jshint": "2.9.5",
"lebab": "2.7.7",
"lodash-es": "^4.17.5",
Copy link
Member

Choose a reason for hiding this comment

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

Same here. Do we even need the explicit lodash-es dependency?

Copy link
Author

Choose a reason for hiding this comment

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

Would committing the entire package to the vendor folder be a better place?

"prepack": "0.2.11",
"prettier": "1.8.2",
"source-map": "0.6.1",
Expand Down
27 changes: 27 additions & 0 deletions src/std-esm-benchmark.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// Copyright 2017 the V8 project authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

var esmRequire = (function() {
try {
return __non_webpack_require__("@std/esm")(__non_webpack_module__);
} catch (e) {}

try {
return module.require("@std/esm")(module);
} catch (e) {}
})();

module.exports = {
name: "@std/esm"
};

if (typeof esmRequire === "function") {
module.exports.fn = function() {
for (var key in esmRequire.cache) {
delete esmRequire.cache[key];
}

return esmRequire("lodash-es");
};
}
7 changes: 7 additions & 0 deletions src/std-esm-benchmark.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// Copyright 2017 the V8 project authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

const esmBenchmark = require("./std-esm-benchmark");

it("std-esm-benchmark runs to completion", () => void esmBenchmark.fn());
5 changes: 4 additions & 1 deletion src/suite.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,14 @@ const suite = new Benchmark.Suite();
require("./prepack-benchmark"),
require("./prettier-benchmark"),
require("./source-map-benchmark"),
require("./std-esm-benchmark"),
require("./typescript-benchmark"),
require("./uglify-es-benchmark"),
require("./uglify-js-benchmark")
].forEach(options => {
suite.add(Object.assign({}, options, defaultOptions));
if (options.fn) {
suite.add(Object.assign({}, options, defaultOptions));
}
});

module.exports = suite;
3 changes: 3 additions & 0 deletions webpack.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ module.exports = [
banner:
"// Required for JavaScript engine shells.\n" +
"var global = this;\n" +
"if (typeof module !== 'undefined') {\n" +
" __non_webpack_module__ = module;\n" +
"}\n" +
"if (typeof console === 'undefined') {\n" +
" console = {log: print};\n" +
"}",
Expand Down