Bug Tracker

Ticket #8065 (closed bug: duplicate)

Opened 4 years ago

Last modified 3 years ago

Significant Performance Difference in 1.5RC1

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

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.

Change History

comment:1 Changed 4 years ago by rwaldron

  • Owner set to mhildreth@…
  • Status changed from new to pending

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!

comment:2 Changed 4 years ago by SDraconis

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

comment:3 Changed 4 years ago by mhildreth@…

  • Status changed from pending to new

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.

comment:4 Changed 4 years ago by snover

  • Status changed from new to closed
  • Resolution set to duplicate

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.

comment:5 Changed 4 years ago by snover

Duplicate of #7341.

comment:6 Changed 4 years ago by rwaldron

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

comment:7 Changed 4 years ago by mhildreth

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.

comment:8 Changed 4 years ago by rwaldron

Just curious, are you using the HTML5 menu element?

comment:9 Changed 4 years ago by rwaldron

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

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

comment:10 Changed 4 years ago by mhildreth

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.

comment:11 Changed 4 years ago by rwaldron

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

comment:12 Changed 4 years ago by mhildreth

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 :-)]

Note: See TracTickets for help on using tickets.