Bug Tracker

Modify

Ticket #8013 (closed bug: fixed)

Opened 2 years ago

Last modified 15 months ago

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
Blocking: Blocked by:

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?

Change History

comment:1 Changed 2 years ago by rwaldron

  • Component changed from unfiled to manipulation

comment:2 Changed 2 years ago by rwaldron

  • Priority changed from undecided to blocker
  • Status changed from new to open

Confirmed, 1.5 is mega slow.

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

comment:3 Changed 2 years ago by danheberden

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

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

comment:4 Changed 2 years ago by addyosmani

  • Cc rwaldron, danheberden added

+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?.

comment:5 Changed 2 years ago by jitter

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

comment:6 Changed 2 years ago by rwaldron

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

comment:7 Changed 2 years ago by rwaldron

Duplicate of #7992.

comment:8 Changed 2 years ago by brad@…

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

comment:9 Changed 2 years ago by rwaldron

  • Status changed from closed to reopened
  • Resolution duplicate deleted

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.

Last edited 2 years ago by rwaldron (previous) (diff)

comment:10 Changed 2 years ago by jitter

  • Owner set to rwaldron
  • Status changed from reopened to assigned

 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)

comment:11 Changed 2 years ago by rwaldron

@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.

comment:12 Changed 2 years ago by rwaldron

More results shots...

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

comment:13 Changed 2 years ago by danheberden

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

 http://gyazo.com/b780d57adb15b970d5ff66d09a73f2f9.png

comment:14 Changed 2 years ago by rwaldron

Patch is being refactored - please hold.

comment:15 Changed 2 years ago by danheberden

comment:16 Changed 2 years ago by john

  • Status changed from assigned to closed
  • Resolution set to fixed
  • Milestone changed from 1.next to 1.5

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

Please follow the  bug reporting guidlines and use  jsFiddle when providing test cases and demonstrations instead of pasting the code in the ticket.

View

Add a comment

Modify Ticket

Action
as closed
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.