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

Further general optimizations. #389

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

cscott
Copy link
Collaborator

@cscott cscott commented Dec 16, 2015

Be careful about how arguments is accessed, and ensure that the ES.* helpers are treated as constant functions which can be inlined.

Accessing elements after the end of the arguments array triggers
deoptimization of the entire function on v8, so avoid doing so.  Also
tweak all the checks into the same standard form for consistency.
Add a bogus assignment to a function prototype, which appears to be the
magic hint to treat function assignments as constant (until proven
otherwise).  Also get rid of the after-the-fact mutation of the ES
object by making IsPromise an ordinary function.  (We could also
call `Object.freeze(ES)` but it doesn't actually improve performance
further.)
@@ -267,7 +267,9 @@

var $String = String;

var ES = {
// Assign these functions to a prototype to give v8 a hint that it
Copy link
Collaborator

Choose a reason for hiding this comment

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

how does this impact other engines? v8-specific optimizations have hurt other engines' performance in the past, with bluebird for example.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I can't imagine it would hurt performance, since it's just a throwaway assignment. I'll try to double-check, but I don't know of any equivalent low-level IR tools on the firefox side. (jsperf can be highly misleading.)

@ljharb
Copy link
Collaborator

ljharb commented Jan 3, 2016

I've cherry-picked in one of these commits - a rebase should result in only the one commit left on top

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants