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

Re-optimize ES.TypeIsObject. #388

Merged
merged 1 commit into from
Dec 16, 2015

Conversation

cscott
Copy link
Collaborator

@cscott cscott commented Dec 16, 2015

Caching the typeof value in a local variable defeats the typeof x === <constant string> peephole optimization, and slows down es6-shim on the doxbee benchmark (on my machine, 20000 iterations) from 5870ms (with this patch) to 6062ms (without it).

@ljharb
Copy link
Collaborator

ljharb commented Dec 16, 2015

lulz @ js engine optimization

// http://www.ecma-international.org/ecma-262/6.0/#sec-toobject
// but is not well optimized by runtimes and creates an object
// whenever it returns false, and thus is very slow.
return typeof x === 'function' || typeof x === 'object';
Copy link
Collaborator

Choose a reason for hiding this comment

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

fwiw i think we could do || Object(x) === x here and only new primitives would hit the slow path, and only until we'd attempted to account for them - that might be better than leaving just the comment?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's an interesting proposal. You'd have to add back the string/number/symbol checks in that case, though, to ensure that only "exotics" hit the || Object(x) case. Not sure the extra complexity is worth it, given that I can't actually identify any valid "exotics" in any implementation that don't return 'object' or 'function'. Presumably if there really is some instance in the wild someone will file a bug complaining that es6-shim throws a TypeError in some case when it shouldn't, and we can tweak the check then.

Copy link
Collaborator

Choose a reason for hiding this comment

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

ah yes, i missed that you removed those.

That's a fair point, so I think this is OK

Caching the typeof value in a local variable defeats the
`typeof x === <constant string>` peephole optimization, and slows down
es6-shim on the doxbee benchmark (on my machine, 20000 iterations) from
5870ms (with this patch) to 6062ms (without it).
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