-
Notifications
You must be signed in to change notification settings - Fork 69
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
Run the benchmark on various engines as part of CI #29
base: master
Are you sure you want to change the base?
Conversation
.travis.yml
Outdated
- "8" | ||
script: | ||
- npm test | ||
- node dist/cli.js | ||
- jsvu --os=linux64 --engines=all | ||
- chakra dist/cli.js | ||
- # javascriptcore dist/cli.js |
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.
Note: jsc
doesn’t offer Linux builds yet: https://github.com/GoogleChromeLabs/jsvu#supported-engines
9e43838
to
1606ce0
Compare
.travis.yml
Outdated
- "8" | ||
script: | ||
- npm test | ||
- node dist/cli.js | ||
- jsvu --os=linux64 --engines=all | ||
- ~/.jsvu/chakra dist/cli.js | ||
- # ~/.jsvu/javascriptcore dist/cli.js |
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.
😢
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.
@jfbastien Did you see https://github.com/GoogleChromeLabs/jsvu#supported-engines, and specifically https://bugs.webkit.org/show_bug.cgi?id=179945? I’d love to support JSC on Linux, but I can’t seem to find any downloads for it! Can you help?
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.
It's available on wasm-stat.us, which is built from https://github.com/webassembly/waterfall
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.
jsvu has a policy of only trusting installers that come straight from the source, i.e. from Apple directly.
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.
@jfbastien Even so, it looks like the Linux builder doesn’t include the “JSC” step :(
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.
@mathiasbynens that should be super easy to fix on https://github.com/webassembly/waterfall. I'm sure @dschuff is happy to take the PR for it.
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.
WebAssembly/waterfall#299, but this does not mean I am okay with changing jsvu’s policy! ;)
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.
@jfbastien In fact, if Apple could provide these downloads directly, WebAssembly/waterfall could just use jsvu
to get them instead of compiling everything from source. (Same for all the other JS engines.) 👍
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.
As discussed on your webkit-dev post, the GTK folks maintain the Linux port. Lucas says we're happy to host the binary.
Apple provides an official jsc with MacOS.
.travis.yml
Outdated
@@ -1,11 +1,14 @@ | |||
language: node_js | |||
node_js: | |||
- "6" |
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'd love to keep 6 and 8, and add 9 instead, so we have coverage for all (relevant) LTS versions and for the current node version.
458af1d
to
aa633cd
Compare
bf0edd6
to
5442220
Compare
I noticed the travis-ci test failed (seemed one-off-ish). Update: Ah yep! See travis_retry. |
@jdalton Wanna submit a PR? 😎 |
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.
Sorry, this fell off my plate somehow. Changes LGTM. Restarted the CI.
Sure, I'll do a follow up. |
a6b034d
to
f03b110
Compare
c289d6a
to
164db67
Compare
No description provided.