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

WIP events: optimize emit() #533

Closed
wants to merge 1 commit into from

Conversation

trevnorris
Copy link
Contributor

EventEmitter#emit() can be called with many different arguments, but
there are a few that are most common. Increase performance by optimizing
those most common cases. It also drastically decreases the amount of
output from --trace_deopt.

This is a hyper optimization and will probably on save 100ns or so at actual runtime. One thing I find most advantageous is that --trace_deopt will show far less output.

There is still plenty that I would like to do. For example, implement a sort of IC in JS that will preempt what will be called. Would like feedback during development. Also one of the "message" tests is failing because of the change in the call stack. Will take care of that before WIP is removed.

R=@indutny

} else if (er instanceof Error) {
throw er; // Unhandled 'error' event
} else {
throw Error('Uncaught, unspecified "error" event.');
Copy link
Member

Choose a reason for hiding this comment

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

Is there any difference between throw new Error and throw Error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Think the specification states that will always return a new Error object regardless. But since I'm there might as well switch it up to use new.

@indutny
Copy link
Member

indutny commented Jan 21, 2015

Mostly LGTM if it boosts performance or at least does not kill it ;)

@trevnorris
Copy link
Contributor Author

@indutny Thanks. I'm going to sleep on it. There's still more to do. I would like to further implement an in JS IC that can preemptively call the correct emit function.

@trevnorris trevnorris force-pushed the ee-awesomeness-all-around branch 2 times, most recently from 5c1ba98 to 4672003 Compare January 21, 2015 00:40
return true;
}

function $emitError(type, er) {
Copy link
Member

Choose a reason for hiding this comment

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

I feel like the function name should reflect that you should !this._events.error before calling. Perhaps $emitUncaughtError?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dougwilson good point. thanks.

EventEmitter#emit() can be called with many different arguments, but
there are a few that are most common. Increase performance by optimizing
those most common cases. It also drastically decreases the amount of
output from --trace_deopt.
@charmander
Copy link
Contributor

Is there a useful benchmark available?

@trevnorris
Copy link
Contributor Author

Superseded by #601

@trevnorris trevnorris closed this Feb 6, 2015
@trevnorris trevnorris deleted the ee-awesomeness-all-around branch February 6, 2015 19:52
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.

4 participants