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

fixed streamline benchmarks #1233

Merged
merged 2 commits into from Dec 29, 2016

Conversation

Projects
None yet
5 participants
@bjouhier
Contributor

bjouhier commented Sep 13, 2016

This PR fixes #1094. I have added the missing reference in benchmark/package.json.

I have also upgraded streamline to 2.0 and recompiled the files.

The streamline-callbacks bench is slower than before (and it is now slower than streamline-generators). This is because the streamline transform has been reimplemented with babel and the new transform uses regenerator instead of the original streamline algorithm.

@benjamingr

This comment has been minimized.

Show comment
Hide comment
@benjamingr

benjamingr Sep 13, 2016

Collaborator

Thanks! Is the code with all the switch/case the actual source code or a generated output?

Collaborator

benjamingr commented Sep 13, 2016

Thanks! Is the code with all the switch/case the actual source code or a generated output?

@bjouhier

This comment has been minimized.

Show comment
Hide comment
@bjouhier

bjouhier Sep 13, 2016

Contributor

The source is the streamline._js file. The file with the big switch/case is produced by regenerator.

The old callbacks algorithm was generating code that looked a bit better and was closer to the original source, but I got lazy when I reimplemented with babel and I just applied the regenerator transform on top of streamline's generators transform. Anyway, every JS engine supports generators these days...

Contributor

bjouhier commented Sep 13, 2016

The source is the streamline._js file. The file with the big switch/case is produced by regenerator.

The old callbacks algorithm was generating code that looked a bit better and was closer to the original source, but I got lazy when I reimplemented with babel and I just applied the regenerator transform on top of streamline's generators transform. Anyway, every JS engine supports generators these days...

@benjamingr

This comment has been minimized.

Show comment
Hide comment
@benjamingr

benjamingr Sep 13, 2016

Collaborator

LGTM

Collaborator

benjamingr commented Sep 13, 2016

LGTM

var _streamline = typeof require === 'function' ? require('streamline-runtime/lib/callbacks/runtime') : Streamline.require('streamline-runtime/lib/callbacks/runtime');
var _filename = '/Users/bruno/dev/bluebird/benchmark/doxbee-sequential-errors/streamline._js';

This comment has been minimized.

@lvivski

lvivski Sep 29, 2016

Contributor

Shouldn't it be relative, not hardcoded? There are several other places throughout the code that have it hardcoded

@lvivski

lvivski Sep 29, 2016

Contributor

Shouldn't it be relative, not hardcoded? There are several other places throughout the code that have it hardcoded

This comment has been minimized.

@bjouhier

bjouhier Sep 29, 2016

Contributor

That's right. It won't do much harm as it is only used to reconstruct stack frames.

I've entered an issue: Sage/streamlinejs#339

@bjouhier

bjouhier Sep 29, 2016

Contributor

That's right. It won't do much harm as it is only used to reconstruct stack frames.

I've entered an issue: Sage/streamlinejs#339

@bambielli

This comment has been minimized.

Show comment
Hide comment
@bambielli

bambielli Dec 4, 2016

It appears that streamline-callbacks.js is now using a deprecated module. I get the following when I run your code:

The regenerator/runtime module is deprecated; please import regenerator-runtime/runtime instead.

bambielli commented Dec 4, 2016

It appears that streamline-callbacks.js is now using a deprecated module. I get the following when I run your code:

The regenerator/runtime module is deprecated; please import regenerator-runtime/runtime instead.
@bjouhier

This comment has been minimized.

Show comment
Hide comment
@bjouhier

bjouhier Dec 21, 2016

Contributor

Should be ok now.

Contributor

bjouhier commented Dec 21, 2016

Should be ok now.

@petkaantonov petkaantonov merged commit c4b2a6e into petkaantonov:master Dec 29, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment