Skip to main content

Bug Tracker

Side navigation

#8065 closed bug (duplicate)

Opened January 26, 2011 06:03PM UTC

Closed February 09, 2011 03:14PM UTC

Last modified March 13, 2012 05:59PM UTC

Significant Performance Difference in 1.5RC1

Reported by: mhildreth@chainbridgetech.com Owned by: mhildreth@chainbridgetech.com
Priority: undecided Milestone: 1.next
Component: unfiled Version: 1.5rc1
Keywords: Cc:
Blocked by: Blocking:
Description

I'm not sure if you would categorize this as a bug, but I did see a significant degredation in performance when switching from 1.4.2 to 1.5.1. I saw the same thing when switching from 1.4.2 to 1.4.4 also.

We have a menu comprised of UL's and LI's that we use jquery to animate and provide other behaviors for. In two cases, I'm seeing that the calls chain to the function uniquesort, which for wrapped sets that have many items, takes much longer (like 6x). The specific calls include the wrapped set's .add and .parent functions. These functions don't seem to call uniquesort in 1.4.2.

I've changed the code to work around this issue by changing something that looked like this:

var subItems = $("#menu>ul>li>ul>li>a");

subItems.parent().hover(...);

to something like this:

var subItems = $("#menu>ul>li>ul>li>a");

var subItems.each(function(){$(this).parent().hover(...);});

By calling parent() on each of the items in the subItems wrapped set, rather than chaining it on to the subItems wrapped set, no uniquesort is performed against the result. This code is about 1.5x slower when using 1.4.2, but the execution times are at least consistent when using 1.5rc1.

Attachments (0)
Change History (12)

Changed January 26, 2011 08:37PM UTC by rwaldron comment:1

owner: → mhildreth@chainbridgetech.com
status: newpending

Thanks for taking the time to contribute to the jQuery project! Please provide a reduced jsFiddle test case to help us assess your ticket!

Additionally, test against the jQuery 0 GIT version to ensure the issue still exists.

In the case of performance issues, we recommend creating a perf test at http://jsperf.com (be sure to use http://code.jquery.com/jquery-git.js & jQuery.noConflict() to test against the most recent jQuery ).

Be Excellent to eachother!

Changed February 09, 2011 07:49AM UTC by SDraconis comment:2

This sounds like it is related to #7341, which was unfortunately closed as "wontfix" despite the fact that the performance implications are significant.

Changed February 09, 2011 02:30PM UTC by mhildreth@chainbridgetech.com comment:3

status: pendingnew

Yes, too bad that's listed as wontfix.

I understand the comments that say that the old way had a bunch of errors and the new way is correct, but I think a good suggestion is in comment#11. Since the wrapped set subItems is already known to be sorted and unique, it seems redundant to redo the unique sort when calling .parent().

I wasn't running into the "long running script dialog" in IE, but I only tested IE8, not IE6 or 7. The work-around I put in place doesn't mangle the the code that badly, and even through that section of the code is 1.5x slower, the menu initialization overall is still faster (reduced to 0.8x), so I'd call it a wash.

Changed February 09, 2011 03:14PM UTC by snover comment:4

resolution: → duplicate
status: newclosed

I’m going to merge this into #7341 since this seems to be the same issue; if you can provide a patch to improve performance here without using sourceIndex we would be happy to accept it.

Changed February 09, 2011 03:14PM UTC by snover comment:5

Duplicate of #7341.

Changed February 09, 2011 03:46PM UTC by rwaldron comment:6

In addition to snover's comments, I think it's important to point out that your selectors could use to optimization, at the very least, you can use the jQuery( selector, context ) signature to improve speed:

http://jsperf.com/awful-selector

Changed February 09, 2011 04:10PM UTC by mhildreth comment:7

Well, the code I submitted with the ticket was just pseudo code. The actual code looks more like this:

var menuWrapper = $("menu");

var topMenu = menuWrapper.children("ul");

var topMenuItems = topMenu.children();

var topMenuItemLinks = topMenuItems.children("a");

var subMenus = topMenu.find("ul");

var subMenuItems = subMenus.children();

var subMenuItemLinks = subMenuItems.children("a");

/*Snip*/

function initHovers() {

subMenuItemLinks.parent().hover(function(){/*Snip*/});

}();

Now the first part is just as fast if not faster. All indicators point to the slow down in calling initHovers() where it calls .parent(). There are about 400 items in the subMenuItemLinks set at that point, so the uniqueSort operation gets really exaggerated. I'm open to suggestions if you have any as to how I could improve the selectors in this case.

Changed February 09, 2011 04:17PM UTC by rwaldron comment:8

Just curious, are you using the HTML5

menu
element?

Changed February 09, 2011 04:24PM UTC by rwaldron comment:9

Also, take a look at this and tell me what you think...

http://jsfiddle.net/rwaldron/ZjV3N/2/

Changed February 09, 2011 04:34PM UTC by mhildreth comment:10

No. Just a bunch of ul's, li's, and a's. As you can probably guess, they form a typical horizontal navigation menu. We use jquery to traverse it to wire up events and provide the animation effects for the drop-downs. This is for an application that has a broad range of consumers (which unforunately includes a decent amount of IE6 users), so HTML5 is not really an option for us and likely won't be any time soon.

Looking at your jsfiddle test. You are correct, for the sample markup included, they are the same. However, the menu allows for "break" items in it which are li's that do not contain a's, but instead and hr. The "break" items just serve to separate groups of li's in a sinlge nested ul (submenu). In this case, we don't want to have those li's light up when hovered over because there's nothing clickable inside them - make sense? That's why I'm finding the a's and then asking for parents. Those parents are the ones that should light up when hovered over.

Changed February 09, 2011 05:21PM UTC by rwaldron comment:11

I broke down the var refs to the actual method chain, and it's insightful. Take a look at this...

http://jsperf.com/traversing-menu-awful-selector

Changed February 09, 2011 05:46PM UTC by mhildreth comment:12

OK, so I revised the test to include more links. Check out revision 2:

http://jsperf.com/traversing-menu-awful-selector/2

As the number of links grows larger, the difference in selectors narrows. However in this example 100, but in the one that started this whole thing, it's more like 400. You can see that the call to .parent() causes somewhere between a 30x and 100x burden.

[Caveat - I get that having 100 items in the menu is a bit silly, but clients want what they want :-)]