use instanceof instead of .jquery for speed + portability #3

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
5 participants
@akre54

akre54 commented Jan 31, 2013

I was curious about the use of the .jquery property lookup on the node variable in component.js as compared to using instanceof $, so I set up a jsperf to compare the two.

instanceof is faster than .jquery in all cases (bare element, string selector, or wrapped jQuery element) in Safari, Chrome, and Android 4.0.4, but much slower for the wrapped jQuery element case in Firefox (maybe something to do with V8's hidden classes?).

My main reason for this pull however, is portability. It's the only place in the code that specifically requires the jQuery API (zepto and ender dont have .jquery properties on their instances).

@cpb

This comment has been minimized.

Show comment Hide comment
@cpb

cpb Jan 31, 2013

👍

cpb commented Jan 31, 2013

👍

@angus-c

This comment has been minimized.

Show comment Hide comment
@angus-c

angus-c Feb 1, 2013

Collaborator

@akre54 thanks for this. Will verify performance across browsers shortly.

Collaborator

angus-c commented Feb 1, 2013

@akre54 thanks for this. Will verify performance across browsers shortly.

@ericelliott

This comment has been minimized.

Show comment Hide comment
@ericelliott

ericelliott Feb 5, 2013

instanceof isn't safe across iframe boundaries. I haven't seen the broader context of this change, but if that is potentially an issue, I would suggest a different approach.

instanceof isn't safe across iframe boundaries. I haven't seen the broader context of this change, but if that is potentially an issue, I would suggest a different approach.

@akre54

This comment has been minimized.

Show comment Hide comment
@akre54

akre54 Feb 5, 2013

@dilvie the solution might be for Flight to hold on to a reference to its DOM manipulation lib object, e.g. via Flight.$, if there is a concern about differing prototypes across iframe boundaries. And if this is the case, there's probably a bigger concern if the $ instances don't share a common prototype in that $element.on and $element.off would refer to a different $ object.

akre54 commented Feb 5, 2013

@dilvie the solution might be for Flight to hold on to a reference to its DOM manipulation lib object, e.g. via Flight.$, if there is a concern about differing prototypes across iframe boundaries. And if this is the case, there's probably a bigger concern if the $ instances don't share a common prototype in that $element.on and $element.off would refer to a different $ object.

@ghost ghost assigned angus-c Feb 17, 2013

@marcobarbosa

This comment has been minimized.

Show comment Hide comment
@marcobarbosa

marcobarbosa Feb 21, 2013

Any updates on this?

I personally feel that I'd have to load jQuery just because of Flight. But maybe that's just my case scenario...

Any updates on this?

I personally feel that I'd have to load jQuery just because of Flight. But maybe that's just my case scenario...

@ericelliott

This comment has been minimized.

Show comment Hide comment
@ericelliott

ericelliott Feb 21, 2013

Why not do a different type of feature detection?

Why not do a different type of feature detection?

@angus-c

This comment has been minimized.

Show comment Hide comment
@angus-c

angus-c Mar 5, 2013

Collaborator

@dilvie although the iframe thing can be dodgy for testing instanceof in the case of $ being passed between iframes, I think it's highly unlikely you'd be running flight in one frame and installing jquery in another

@akre54 FWIW my tests show instance of to be slightly slower in chrome and significantly slower in firefox and IE. But I'm not really concerned about performance in either case. It's just not called often enough for 100,000 times per second to be considered a concern.

It would be nice if flight were decoupled from jQuery, but that is a huge project and this change would be the tip of the iceberg (actually that test might no longer even be required if we had no jQuery)

Collaborator

angus-c commented Mar 5, 2013

@dilvie although the iframe thing can be dodgy for testing instanceof in the case of $ being passed between iframes, I think it's highly unlikely you'd be running flight in one frame and installing jquery in another

@akre54 FWIW my tests show instance of to be slightly slower in chrome and significantly slower in firefox and IE. But I'm not really concerned about performance in either case. It's just not called often enough for 100,000 times per second to be considered a concern.

It would be nice if flight were decoupled from jQuery, but that is a huge project and this change would be the tip of the iceberg (actually that test might no longer even be required if we had no jQuery)

@angus-c angus-c closed this Mar 5, 2013

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