jquery superfish performance question

[grabnerandi-2]

I am analyzing some performance issues at a website and ran into a
piece of code in superfish where i was wondering if this could be
optimized.
in the superfish method i can see the following code snippet:

var $a = $('a',this);
$a.each(function(i){
  var $li = $a.eq(i).parents('li');
  $a.eq(i).focus(function(){over.call($li);}).blur(function(){out.call
($li);});
});

I used the dynaTrace AJAX Edition to analyze the performance of the
superfish method. It turns out the the .eq(i) on $a takes about 20% of
the total execution time. As we are in the each method - wouldnt it be
better to just use this instead of accessing the current object in the
each loop via the eq method? Couldn't the above code be changed to

var $a = $('a',this);
$a.each(function(i){
  var $li = this.parents('li');
  this.focus(function(){over.call($li);}).blur(function(){out.call
($li);});
});

Thoughts?

[Joel Birch]

Hello,

You would still need to wrap that "this" in a jQuery object, otherwise
it refers to a DOM object. At that point, I would think that referring
to an existing jQuery object, as Superfish currently does, would be
preferrable. Are you able to test this, to be sure?

Thanks
Joel Birch

[grabnerandi-2]

I am not able to test it - at least not on the page I've found the
problem as the page is not under my control.
I am doing some web page analysis and discovered this issue and
believe that this change can significantly improve the performance.

Once I find some time I can try it out on a local web-site that I have
under control
But thanks for agreeing to my conclusion - and yes - you are right -
we would need to wrap the "this" object - but that should still be
fater than calling the .eq method

On Nov 2, 9:42 am, Joel Birch <joeldbi...@...> wrote:
> Hello,
>
> You would still need to wrap that "this" in a jQuery object, otherwise
> it refers to a DOM object. At that point, I would think that referring
> to an existing jQuery object, as Superfish currently does, would be
> preferrable. Are you able to test this, to be sure?
>
> Thanks
> Joel Birch

[grabnerandi-2]

Hi Joel

I did some more testing. I wrote a small sample with 3 different
menus: 50 items, 100 items and 500 items (I know - 500 is a lot - but
the page that I've analyzed actually had that many)
I made the change to the library. For the 500 elements the superfish
method takes 3459ms on my laptop with a total of 152222 JavaScript/DOM
calls. After I changed the code to what I proposed the call took
3228ms and 146922 JavaScript/DOM calls. So - the change in my example
improved my 500 items example by 220ms and saved about 5000 calls.

I used http://ajax.dynatrace.com for the analysis

what do you think - worth making the change in your library?

On Nov 2, 3:43 pm, grabnerandi <grabnera...@...> wrote:

> I am not able to test it - at least not on the page I've found the
> problem as the page is not under my control.
> I am doing some web page analysis and discovered this issue and
> believe that this change can significantly improve the performance.
>
> Once I find some time I can try it out on a local web-site that I have
> under control
> But thanks for agreeing to my conclusion - and yes - you are right -
> we would need to wrap the "this" object - but that should still be
> fater than calling the .eq method
>
> On Nov 2, 9:42 am, Joel Birch <joeldbi...@...> wrote:
>
>
>
> > Hello,
>
> > You would still need to wrap that "this" in a jQuery object, otherwise
> > it refers to a DOM object. At that point, I would think that referring
> > to an existing jQuery object, as Superfish currently does, would be
> > preferrable. Are you able to test this, to be sure?
>
> > Thanks
> > Joel Birch- Hide quoted text -
>
> - Show quoted text -

[Joel Birch]

Thanks so much for doing that! It does look like an improvement.
Probably not an urgent one, but definitely worth doing for the next
version.

It seems unintuitive that creating a new jQuery object would be less
costly than selecting from an existing collection, but there you go.
Can't argue with solid evidence!

Great feedback, cheers.

Joel.

[grabnerandi-2]

I did some further performance testing on the menus - will soon blog
about it on http://blog.dynatrace.com
I ran tests with 50, 100 and 500 menu items. It seems like 50 elements
take roughly 300ms to process. Performance in that case scales up
linear meaning that 100 take 600ms and 500 take 3s. the page that I
was testing actually had 500 menu items - thats why this whole thing
caught my attention as the page spent 3s in the superfish method

cheers

On Nov 3, 9:49 pm, Joel Birch <joeldbi...@...> wrote:

> Thanks so much for doing that! It does look like an improvement.
> Probably not an urgent one, but definitely worth doing for the next
> version.
>
> It seems unintuitive that creating a new jQuery object would be less
> costly than selecting from an existing collection, but there you go.
> Can't argue with solid evidence!
>
> Great feedback, cheers.
>
> Joel.