Bug 43687 - Web Inspector: speed-up Element.prototype.removeChildren
Summary: Web Inspector: speed-up Element.prototype.removeChildren
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nikita Vasilyev
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-08-08 04:53 PDT by Nikita Vasilyev
Modified: 2010-08-08 10:45 PDT (History)
9 users (show)

See Also:


Attachments
patch (1.02 KB, patch)
2010-08-08 04:57 PDT, Nikita Vasilyev
pfeldman: review-
Details | Formatted Diff | Diff
this.textContent = "" (1006 bytes, patch)
2010-08-08 06:59 PDT, Nikita Vasilyev
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nikita Vasilyev 2010-08-08 04:53:04 PDT
The current Element.prototype.removeChildren is using `element.innerHTML = ""`. It's the slowest method in my benchmarks:

http://jsperf.com/remove-all-child-nodes/2 (20x faster in WebKit, 35x in Chrome)
http://jsperf.com/remove-all-child-nodes-5-childs/3 (1,6 faster in WebKit, 2x in Chrome)
http://jsperf.com/remove-all-child-nodes-zero-childs (1,6 faster in WebKit, 2x in Chrome)
Comment 1 Nikita Vasilyev 2010-08-08 04:57:39 PDT
Created attachment 63840 [details]
patch
Comment 2 Pavel Feldman 2010-08-08 05:11:36 PDT
Comment on attachment 63840 [details]
patch

In fact I've done the opposite optimization earlier:
https://bugs.webkit.org/show_bug.cgi?id=31160

Timeline was lagging visually on scrolling and clearing + shark was showing this to be the slow operation. I'd suggest that you try those. Wrt benchmarks, I think they lack listeners - adding those would change the WebKit picture a lot. I know that the bindings have improved, but still, single call to native will always be ways faster.
Comment 3 Nikita Vasilyev 2010-08-08 05:17:58 PDT
(In reply to comment #2)
> (From update of attachment 63840 [details])
> In fact I've done the opposite optimization earlier:
> https://bugs.webkit.org/show_bug.cgi?id=31160
> 
> Timeline was lagging visually on scrolling and clearing + shark was showing this to be the slow operation. I'd suggest that you try those. Wrt benchmarks, I think they lack listeners - adding those would change the WebKit picture a lot. I know that the bindings have improved, but still, single call to native will always be ways faster.

Oh, I didn't think about reflow. At this case `element.textContent = ""` would be faster.

By the way, what are Wrt benchmarks?
Comment 4 Nikita Vasilyev 2010-08-08 06:59:23 PDT
Created attachment 63842 [details]
this.textContent = ""

http://elv1s.ru/files/js/removeChildren.html
Now innerHTML and textContent works the same. removeFirstChild is a little slower.

When inspecting the DOM, there are many calls of removeChildren while an element already doesn't have children. So I added `if (this.firstChild)` check.
Comment 5 Pavel Feldman 2010-08-08 09:29:15 PDT
Comment on attachment 63842 [details]
this.textContent = ""

Thanks for doing this!
Comment 6 Eric Seidel (no email) 2010-08-08 10:45:31 PDT
Comment on attachment 63842 [details]
this.textContent = ""

Clearing flags on attachment: 63842

Committed r64952: <http://trac.webkit.org/changeset/64952>
Comment 7 Eric Seidel (no email) 2010-08-08 10:45:37 PDT
All reviewed patches have been landed.  Closing bug.