Skip to main content

Bug Tracker

Side navigation

#8013 closed bug (fixed)

Opened January 19, 2011 06:30PM UTC

Closed January 21, 2011 04:21PM UTC

Last modified March 09, 2012 09:22AM UTC

Improve performance of inserting text into an HTML element

Reported by: anonymous Owned by: rwaldron
Priority: blocker Milestone: 1.5
Component: manipulation Version: 1.4.4
Keywords: Cc: rwaldron, danheberden
Blocked by: Blocking:
Description

In Firefox, I am inserting text into ~500 DIVs using this code:

$(".divs").text("default"); // ~500 DIVs

On my computer, this consistently takes 9ms if the DIVs are empty. However, this same code consistently takes 18ms if the DIVs already contain text.

Can we improve the performance of replacing text in a DIV that already contains text?

Attachments (0)
Change History (16)

Changed January 19, 2011 06:32PM UTC by rwaldron comment:1

component: unfiledmanipulation

Changed January 19, 2011 06:42PM UTC by rwaldron comment:2

priority: undecidedblocker
status: newopen

Confirmed, 1.5 is mega slow.

http://jsperf.com/insert-to-500

Changed January 19, 2011 08:21PM UTC by danheberden comment:3

More specific, it's an issue of $.fn.append - zomg is it slow now. like 80% slower.

http://jsperf.com/dh-jquery-dom-manip

Changed January 19, 2011 08:54PM UTC by addyosmani comment:4

cc: → rwaldron, danheberden

+1 on all the above - it's slow as hell. Do we have an idea yet of what change related to $.fn.append is causing this?.

Changed January 20, 2011 12:06AM UTC by jitter comment:5

The real culprit isn't append but domManip. This performance regression potentially affects a whole bunch of methods (maybe not always but: text, append, appendTo, wrap, wrapAll, wrapInner, unwrap, html, replaceWith, replaceAll, jQuery.load, prepend, prependTo, before, after, show, animate) which either directly or indirectly depend on domManip.

The regression was introduced with this commit.

Specifically the change on this line which before was fragment.cloneNode( true ) :


I also want to address that the reporter originally didn't complain about a performance drop for .text/append/domManip between 1.4.4 and 1.5, but that he wanted

$("<div>test test</div>").text("bar")

to be as fast as

$("<div></div>").text("bar")

not sure if there is something which can be done about that

Changed January 20, 2011 12:29AM UTC by rwaldron comment:6

resolution: → duplicate
status: openclosed

Changed January 20, 2011 12:29AM UTC by rwaldron comment:7

Duplicate of #7992.

Changed January 20, 2011 02:02AM UTC by brad@assistly.com comment:8

So #7992 is really about remove() being slow and it was introduced starting in 1.4.3

Changed January 20, 2011 03:36AM UTC by rwaldron comment:9

_comment0: Patched. For some reason the browser scope tests arent updating correctly, so I grabbed screenshots of the tests along the way \ \ Chrome: http://gyazo.com/c40de84ba91c1602838ca532f32da0a1.png \ Opera: http://gyazo.com/e40704f30e7798f2fa6bae79c4d65722.png \ Safari: http://gyazo.com/8cb924594e5f74e428ebb8904c8b111a.png \ Firefox 3.6.13: http://gyazo.com/275f394bbc9e60fd06dbed0556f0f51b.png \ Firefox 4b9: http://gyazo.com/dfc05f1da4413d41c7650194619de116.png \ \ \ The IE patch that introduced the issue is still in place, instead I'm only running it through that path if its absolutely nec. IE requires the run through clone, so its still slow.1295495415358824
_comment1: Patched. For some reason the browser scope tests arent updating correctly, so I grabbed screenshots of the tests along the way \ \ Chrome: http://gyazo.com/c40de84ba91c1602838ca532f32da0a1.png[[BR]] \ \ Opera: http://gyazo.com/e40704f30e7798f2fa6bae79c4d65722.png[[BR]] \ Safari: http://gyazo.com/8cb924594e5f74e428ebb8904c8b111a.png[[BR]] \ Firefox 3.6.13: http://gyazo.com/275f394bbc9e60fd06dbed0556f0f51b.png[[BR]] \ Firefox 4b9: http://gyazo.com/dfc05f1da4413d41c7650194619de116.png \ \ \ The IE patch that introduced the issue is still in place, instead I'm only running it through that path if its absolutely nec. IE requires the run through clone, so its still slow.1295495429584051
resolution: duplicate
status: closedreopened

Patched. For some reason the browser scope tests arent updating correctly, so I grabbed screenshots of the tests along the way

Chrome: http://gyazo.com/c40de84ba91c1602838ca532f32da0a1.png

Opera: http://gyazo.com/e40704f30e7798f2fa6bae79c4d65722.png

Safari: http://gyazo.com/8cb924594e5f74e428ebb8904c8b111a.png

Firefox 3.6.13: http://gyazo.com/275f394bbc9e60fd06dbed0556f0f51b.png

Firefox 4b9: http://gyazo.com/dfc05f1da4413d41c7650194619de116.png

The IE patch that introduced the issue is still in place, instead I'm only running it through that path if its absolutely nec. IE requires the run through clone, so its still slow.

Changed January 20, 2011 03:14PM UTC by jitter comment:10

owner: → rwaldron
status: reopenedassigned

pull request

rwaldron can you please include a link to the jsperf test you made. Also wouldn't this be better suited on #8017 as this bug report originally was about something different (speed of .text in 1.4.4 not the perf drop from 1.4.4 to 1.5b1)

Changed January 20, 2011 04:18PM UTC by rwaldron comment:11

@jitter,

The perf test will show that this is the appropriate ticket for the patch - they are actually all related.

The perf test will append a string to 500 divs per iteration with 4 different versions of jQuery.

http://jsperf.com/dommanip-is-a-sloth

Also, I should note that jsperf is not sending the correct values to browserscope.

Changed January 20, 2011 04:28PM UTC by rwaldron comment:12

More results shots...

Chrome 8 on Win7: http://goo.gl/tVaZn

Changed January 20, 2011 04:32PM UTC by danheberden comment:13

IE6 - which of course, is running the slow bug fix for events...

http://gyazo.com/b780d57adb15b970d5ff66d09a73f2f9.png

Changed January 20, 2011 08:12PM UTC by rwaldron comment:14

Patch is being refactored - please hold.

Changed January 20, 2011 08:42PM UTC by danheberden comment:15

Changed January 21, 2011 04:21PM UTC by john comment:16

milestone: 1.next1.5
resolution: → fixed
status: assignedclosed

Landed this pull request, which should resolve this issue: https://github.com/jquery/jquery/pull/200