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

Fix for V8 disables optimization for dispatch #1

Closed
wants to merge 1 commit into from

Conversation

Hypercubed
Copy link

I posted this issue on the master (millermedeiros#67) but submitting a PR here because I can build and test this version.

I haven't checked the actual performance impact of this change vs. previous but suspect enabling optimization is better.

@mhofman
Copy link

mhofman commented Jun 4, 2015

Interesting. It's baffling V8 disables optimizations when copying arguments in this quite common way, especially since named args are not used at all in this function.

You might want to open a pull request on jvmartins/js-signals instead. This repo incorporates their build system changes as well as a change in how dispatch itself works (by catching and rethrowing exceptions asynchronously)

@Hypercubed
Copy link
Author

Here is a list of "optimization killers": https://github.com/petkaantonov/bluebird/wiki/Optimization-killers . Not my work so I don't know how much this impact performance.

I made the PR because it appeared to be the most ahead. It would be nice if there was a true active fork of js-signals.

@Hypercubed
Copy link
Author

From what I can tell there is a significant difference: http://jsperf.com/shotgun-js-vs-js-signals/4

@Hypercubed Hypercubed closed this Mar 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants