Skip to main content

Bug Tracker

Side navigation

#7341 closed bug (fixed)

Opened October 28, 2010 07:34AM UTC

Closed May 10, 2011 04:17PM UTC

Last modified August 21, 2013 05:40PM UTC

Slow .add() in IE

Reported by: markus@kilke.net Owned by: rwaldron
Priority: high Milestone: 1.6.1
Component: selector Version: 1.4.3
Keywords: Cc: rwaldron
Blocked by: Blocking:
Description

The .add()-function performance is significantly poorer in IE (7 & 8), when using jQuery 1.4.3 vs. older 1.4.2.

Just adding to a collection of a few hundred elements can cause the "Stop running script" alert.

Demo:

http://jsfiddle.net/ZNf8b/6/

Attachments (0)
Change History (41)

Changed October 29, 2010 05:04AM UTC by SlexAxton comment:1

Can someone with a native IE verify?

Changed October 29, 2010 12:43PM UTC by rwaldron comment:2

component: unfiledmanipulation
owner: → rwaldron
status: newassigned

Alex, I'll take a look at this.

Changed November 17, 2010 02:53AM UTC by addyosmani comment:3

cc: → rwaldron

Did we manage to take another look at this? :)

Changed November 24, 2010 09:28PM UTC by rajiv.delwadia@versionone.com comment:4

This is because add() does a uniqueSort(), and the sortOrder comparison that is used in IE changed in 1.4.3.

The old comparison used .sourceIndex, but the new one builds 2 arrays of ancestors for practically every comparison.

Arguably, $("*",node).add(node) should be cheap, not requiring sorting and uniquifying.

Changed December 16, 2010 08:42PM UTC by anonymous comment:5

I just ran into this and had to revert to jquery 1.4.2.

Changed December 17, 2010 06:47PM UTC by DonJaime comment:6

This is not just a problem when add() is used explicitly, it also gets in the way with addSelf(), remove(), and replaceWith(). Acording to the profiler helpfully supplied along with IE8, add() calls uniqueSort(), which calls Array.sort(), which then calls Array.sort(), which calls an internal function thousands or tens of thousands of times, and that internal function calls Array.unshift tens or hundreds of thousands of times.

I had to make code significantly clunkier to keep some parts of our site usable in IE. The problem with remove() and replaceWith() mean that performance of AJAX page updates is still noticeably degraded in IE. I suggest you treat this with rather more urgency than 'did we manage to take another look at this'.

Changed December 17, 2010 09:55PM UTC by anonymous comment:7

The problem is not just Array.shift/unshift. It was the change (in 1.4.3) for sortOrder() in IE from using sourceIndex to walking to the top of the DOM in every comparison.

Changed December 29, 2010 06:20PM UTC by khs4473@gmail.com comment:8

I've forked Sizzle and added sourceIndex support back in. Check it out - I can send a pull request or whatever if you like.

https://github.com/khs4473/sizzle/commit/6cbad8d304af05cf15c1d4365c4a791c45087d49

Changed January 17, 2011 09:06PM UTC by john comment:9

milestone: 1.51.next
priority: undecidedhigh
resolution: → wontfix
status: assignedclosed

We're not planning on moving back to using sourceIndex - sourceIndex was fraught with problems and caused all sorts of compatibility issues. While this particular technique is slower it's also correct - which is important.

Changed January 17, 2011 09:48PM UTC by anonymous comment:10

I'm sorry, John, but I disagree; getting a long-running script error is ''not'' correct. Here is another approach to speeding up add():

When adding n arbitrary elements to a set of size m, concatenating and resorting takes (m+n)*log(m+n), whereas doing an insertion of the new elements take n*log(m). This would work because the first set is already known to be sorted. Given that m can be arbitrarily large, and n is usually 1, the insertion will invariably win.

In the case of a comma selector, it gets even better: both sets are known to be already sorted, so they just need a merge. n+m

Changed January 20, 2011 03:41PM UTC by anonymous comment:11

sourceIndex is pretty thoroughly validated in my patch, but hey, it's your engine! : )

Changed January 21, 2011 09:39AM UTC by anonymous comment:12

If something doesn't work properly then it is not correct, even if it is 'correct'.

Changed January 25, 2011 11:48PM UTC by David Tong comment:13

just to add to the discussion, any app that lists a bunch of items with mouseover and mouseout events will trigger withinElement that ultimately calls andSelf() that calls Array.sort. 1.4.4 is slower than previous versions as indicated in the thread above, and on more complex apps, the performance is noticeably slower now.

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

#8065 is a duplicate of this ticket.

Changed February 15, 2011 09:41AM UTC by ratibus@gmail.com comment:15

This performance issue in IE is major in my opinion. It broked parts of our applications because of this bug #8233 which is caused by this particular .add() issue.

Please reopen this ticket. It could not be acceptable to have jquery users facing "slow script" issue in IE because of internal implementation decisions.

Changed February 16, 2011 07:11AM UTC by twatson@gisplanning.com comment:16

I have to agree, with ratibus. We see this issue when placing a large when calling $("#element").empty().html("LARGE CHUCNK OF HTML WITH SCRIPTS"). We do this in order to remove any previous handlers in elements in #element and to run any required scripts in the new HTML. Since 1.4.2 we have been able to update because our IE users are now seeing the long running script warning. Please reopen and provide a workaround. Or if you can suggest a better way to do what we have been doing I will take that! I will happily create fiddler cases to demonstrate that although you are doing the "correct" thing, you are infact making the library unusable.

Changed March 01, 2011 06:07PM UTC by scottgonzalez comment:17

There are multiple bugs filed against jQuery UI which are caused by this.

Changed March 02, 2011 01:25AM UTC by anonymous comment:18

With all due respect, I have to add that this issue shouldn't be disregarded. What makes jQuery such a great and essential javascript library is how seamlessly it makes web development work across the legion of different browsers. This is especially true when having to deal with Internet Explorer which continues to be the thorn in the side of developers. Now if you make a fundamental change to the jQuery core that has not just a slight, but significant performance hit for IE then you are assailing one of jQuery's fundamental strengths. Also not only are you forcing people to have to stay at jquery 1.4.2 but you're preventing people from taking advantage of jQuery UI as well because of the decision to not find a better solution for this issue. I would hope that developers of such caliber to have been able to create jQuery wouldn't accept this sub par situation.

Changed March 07, 2011 04:10AM UTC by anonymous comment:19

Agree with comment 19.

We've also hit this issue in an MS MVC3 project using the unobtrusive validation framework. The jquery.validate.js plugin is hitting .Add() and performance is so bad in IE8 that a production release is not an option. MS MVC framework seems to be getting good traction so I'm sure many others will run into this issue.

As such we are also roadblocked from moving past v1.4.2.

Changed March 18, 2011 06:24PM UTC by nsegura3 comment:20

This is also hitting the Dialog widget of jquery UI as the code that tries to give focus to the first focusable element uses the andSelf() function. We are getting many complains from our users getting the "script taking too long" warning when opening a dialog.

Changed March 28, 2011 10:32PM UTC by anonymous comment:21

"''While this particular technique is slower it's also correct - which is important.''"

John, this is about the dumbest unilateral stance I've ever seen someone take.

When we upgraded jQuery from 1.4.2 to 1.4.4, a simple function on a complex DOM that took 67ms in IE8 now takes over 3.4s. jQuery is now 50 times slower in IE8, which is hardly a metric that should be so lightly dismissed.

No one who runs a data-heavy production website can upgrade beyond 1.4.2 until this issue is fixed for IE8. You're shooting your own project in the foot.

Changed March 28, 2011 10:33PM UTC by anonymous comment:22

"''While this particular technique is slower it's also correct - which is important.''"

John, this is about the dumbest unilateral stance I've ever seen someone take.

When we upgraded jQuery from 1.4.2 to 1.4.4, a simple function on a complex DOM that took 67ms in IE8 now takes over 3.4s. jQuery is now 50 times slower in IE8, which is hardly a metric that should be so lightly dismissed.

No one who runs a data-heavy production website can upgrade beyond 1.4.2 until this issue is fixed for IE8. You're shooting your own project in the foot.

Changed March 30, 2011 04:00PM UTC by ajpiano comment:23

#8561 is a duplicate of this ticket.

Changed April 04, 2011 10:25AM UTC by asherh comment:24

Replying to [comment:22 anonymous]:

We are also stuck with 1.4.2 because of this checkSibling() IE problem. We have a page with 1.4.3+, UI 1.8.11, and a dialog with a large multiple select (hundreds of options). If you click on an option, IE 7/8 freezes in endless JS calls to checkSibling(). Please re-open, many users will be stuck with 1.4.2 until it is fixed.

See also:

http://bugs.jquery.com/ticket/7367

http://forum.jquery.com/topic/is-this-a-regression-or-a-known-issue-in-1-4-3-and-ie7-8

http://bugs.jqueryui.com/ticket/6644

http://core.trac.wordpress.org/ticket/16643

Changed April 05, 2011 02:37PM UTC by anonymous comment:25

Also stuck on 1.4.2. I hope it gets resolved.

Changed April 08, 2011 03:33PM UTC by justbeez comment:26

I completely understand that doing things correctly is important. However, there has to be a way to improve the performance of the "correct" way.

IE8 still has too much market share to just have broken interfaces that used to work. (That's a regression.)

I'm using jQuery 5.1.2 (can't revert due to dependencies) to filter and search data based on user input that triggers AJAX calls and returns a list of matching elements from a database. Even with caching all the wrapped sets and optimizing the selectors and setting contexts where possible, the filtering of those sets is just too slow in IE8 and results in the slow script warnings. And I'm only dealing with two sets of ~75 siblings each!

I completely understand that sourceIndex isn't the answer--I've run into some of it's issues before. But we need ''something'' that works in IE8 proficiently. If not, this can't be considered a cross-browser library anymore.

Changed April 19, 2011 04:10PM UTC by cmcnulty comment:27

For future reference, here's the commit that has caused the performance regression:

https://github.com/jquery/sizzle/commit/bb0828d44b6c9d360e6077187e1607076b6e67ab#sizzle.js

Changed April 25, 2011 08:58PM UTC by brewdente@gmail.com comment:28

If anyone wants a quick and easy way to get around this issue, just include the code pasted here:

http://codepaste.net/sygep8

after you include your jQuery file and you'll be back in business. Finally, you can update your jQuery!

Changed May 03, 2011 01:07PM UTC by anonymous comment:29

This has caught our production environment out and we're looking to revert to 1.4.2 - a workable solution going forward would be great.

Changed May 04, 2011 11:35AM UTC by anonymous comment:30

has the "correct" way been "fixed" in 1.6?

Changed May 04, 2011 11:28PM UTC by anonymous comment:31

Has anybody tested 1.6 yet to see if this issue has been resolved?

Changed May 05, 2011 05:10AM UTC by cmcnulty comment:32

_comment0: 1.6 has the same performance problem. I've tried to consolidate all of the info I've gathered about this issue into a perftest to demonstrate the severity of the issue once and for all. \ \ http://jsperf.com/jquery-1-4-3-perf-degrade/4 \ \ I've also created a jsfiddle to attempt to debunk the idea that the commit that caused these problems actually fixed the issue of disconnected nodes: \ \ http://jsfiddle.net/P8aPL/2/ \ \ I'm off to jquery forums to try to see if anyone is interested in taking another look at this issue.1304612780484598

1.6 has the same performance problem. I've tried to consolidate all of the info I've gathered about this issue into a perftest to demonstrate the severity of the issue once and for all.

http://jsperf.com/jquery-1-4-3-perf-degrade/5

I've also created a jsfiddle to attempt to debunk the idea that the commit that caused these problems actually fixed the issue of disconnected nodes:

http://jsfiddle.net/P8aPL/2/

I'm off to jquery forums to try to see if anyone is interested in taking another look at this issue.

Changed May 05, 2011 05:36AM UTC by cmcnulty comment:33

I've opened a discussion of this topic here.

Hopefully, we can suss out some better test cases for improving a plugin, and maybe, hopefully, someday, improving the sort performance of IE6,7,8 back to 1.4.3 levels.

Changed May 10, 2011 04:13PM UTC by john comment:34

milestone: 1.next1.6.1
resolution: wontfix
status: closedreopened

Changed May 10, 2011 04:17PM UTC by john comment:35

component: manipulationselector
resolution: → fixed
status: reopenedclosed

Fixed. https://github.com/jquery/sizzle/commit/2f96e523f77c69fa99138e49d923c44c9bdfae51

I realized that we could use sourceIndex in a very limited case which should help the performance for .add(), overall. Note that we can't use sourceIndex exclusively (as was being asked for) but only as a side to the actual sorting algorithm.

Changed May 10, 2011 05:05PM UTC by john comment:36

Performance test with data here:

http://jsperf.com/jquery-add-performance-2

Right now it's only about 6-10% slower in 1.6.1 vs. 1.4.2 - but substantially faster than 1.4.3 - 1.6 (around 12-20x).

Changed May 10, 2011 08:23PM UTC by cmcnulty comment:37

_comment0: John, you can also close [http://bugs.jquery.com/ticket/8778] as a dupe1305227086320639

John, you can also close #8778 as a dupe

Changed May 16, 2011 04:18PM UTC by john comment:38

#8778 is a duplicate of this ticket.

Changed July 11, 2011 03:28AM UTC by roviury@gmail.com comment:39

http://closure-compiler.appspot.com/code/jsc6b0c3922d3ded1527f0deda155d1abcd/default.js

This is a minified, modified function jQuery.fn.add

(all jQuery versions, all browsers, 476 bytes)

it can speed up at least 2.0x

PS. two array of elements must in DOM order before

Changed July 25, 2013 07:21PM UTC by cseppy@gmail.com comment:40

Replying to [comment:29 brewdente@…]:

We have tons of users stuck in IE 7 and 8 and this is a great workaround, thanks!

Changed August 21, 2013 05:40PM UTC by Jamie comment:41

Replying to [comment:36 john]:

Fixed. https://github.com/jquery/sizzle/commit/2f96e523f77c69fa99138e49d923c44c9bdfae51

John, I have jquery-1.9.1, and I notice that this change isn't in there. When will it be released to the mainline?