Closed
Bug 507393
Opened 15 years ago
Closed 15 years ago
Manipulating tbody dom elements is very slow when the tbody is visible.
Categories
(Core :: Layout, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: thecheatah, Unassigned)
References
Details
(Keywords: perf)
Attachments
(1 file)
|
(deleted),
text/html
|
Details |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.1) Gecko/20090715 Firefox/3.5.1
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.1) Gecko/20090715 Firefox/3.5.1
In firefox 3.5, when manipulating the DOM tree, the speed at which it executes the DOM operations depends on the visibility of the node elements (tbody in my case). Simple operation such as:
function replaceNodeWithChildren(domNode)
{
if (domNode.parentNode == null)
return;
while(domNode.childNodes.length != 0)
{
domNode.parentNode.insertBefore(domNode.firstChild, domNode);
}
domNode.parentNode.removeChild(domNode);
}
Where domNode.childNodes.length has at most 5 node. The method is called about 200 times and takes OVER 30 seconds to execute. Triggering the "code taking too long" message.
If I make the domNode.parentNode.style['display'] = "none" before doing any dom manipulation, it processes the DOM in a split second.
This problem started after 3.5. There is no slow down in 3.0.
Reproducible: Always
Steps to Reproduce:
1. Save the following as an html file:
<html>
<head>
<script>
function replaceNodeWithChildren(domNode)
{
if (domNode.parentNode == null)
return;
while(domNode.childNodes.length != 0)
{
domNode.parentNode.insertBefore(domNode.firstChild, domNode);
}
domNode.parentNode.removeChild(domNode);
}
function run(hidden)
{
var source = document.getElementById('source');
var destination = document.getElementById('destination');
while(destination.childNodes.length != 0)
destination.removeChild(destination.firstChild);
if (hidden)
destination.style['display'] = "none";
for(var i = 0; i < 200; i++)
{
source.innerHTML = "<tbody><tr><td><b>Line 1:</b> Hello</td><td>World</td></tr><tr><td>Goodbye</td><td>World</td></tr></tbody>";
var child = source.firstChild;
destination.appendChild(child);
replaceNodeWithChildren(child);
}
source.innerHTML = "";
if (hidden)
destination.style['display'] = "block";
}
</script>
</head>
<body>
<input type="button" value="Run Visible" onclick="run(false);"/>
<input type="button" value="Run Hidden" onclick="run(true);"/>
<table>
<tbody id="destination">
</tbody>
</table>
<table id="source">
</div>
</body>
</html>
2. Open it in firefox 3.5
3. Click "Run Visible". This should take 5+ seconds to run.
4. Reload the page.
5. Click "Run Hidden". This should happen instantly.
6. Without reloading the page click "Run Hidden" again. This will once again take over 5+ seconds
Actual Results:
At step 3: Takes 5+ seconds to execute
Expected Results:
The speed at which dom manipulation is done should not depend on the type of element and
I have:
<table>
<tbody id="destination">
</tbody>
</table>
I am taking:
<tbody>
<tr><td><b>Line 1:</b> Hello</td><td>World</td></tr>
<tr><td>Goodbye</td><td>World</td></tr>
</tbody>
and appending it to the "destination". Then I replace the inserted tbody and replace it with its children.
This only happens for tbody I think.
Odd. This looks similar to my:
http://m8y.org/tmp/table.xhtml
Where both in-DOM (which is equiv to visible) and off-DOM (should be similar to invisible) have identical perf times on my 3.5 (well. off-DOM is slightly faster, but not by a large margin).
Can you attach your example as a testcase? Or link to one?
|
Reporter | |
Comment 2•15 years ago
|
||
Posted the same thing online.
http://squam.rutgers.edu/bart/firefoxbug.html
|
|
Reporter | |
Comment 3•15 years ago
|
||
@bugs@m8y.org
I think the problem is different. You are manipulating large amount of dom objects. I am only manipulating 200 or so.
Only reason number would matter, surely, is if the issue is on-viewport vs off...
Number should improve timing.
But. I will make a modified test where all rows are on-viewport.
And try your testcase I guess. I wonder if bz would have an opinion on this. He seems to like performance issues. :)
Looking at your example - seems like you are measuring rendering time - not node creation time. Needs some timing numbers to verify that.
For example:
http://m8y.org/tmp/primes.xhtml
Try with, say, one million cells.
On my machine I get:
Finished creation in 2.152 seconds.
Sieve completed in 0.006 seconds.
That 0.006 seconds includes the DOM manipulation of className HOWEVER the rendering is clearly much longer than that.
Oh. Just to be clear. IMO INVALID since you're simply measuring rendering time of drawing the 200 tbodies. But still need to add timing checks. Or I guess bz could do it properly :)
mm. n/m. read testcase more carefully. Seems reasonable complaint - plus you're right. Something changed... :(
Playing around in:
http://m8y.org/tmp/testcase135.xhtml
Yours is in:
http://m8y.org/tmp/testcase134.html
So far making it non-quirky and cleaning up the tags did not help.
using cloneNode instead simply made the non-rendered one even faster :) didn't impact rendered one at all.
And of course doing a fast remove simply sped up both.
|
|
Reporter | |
Comment 9•15 years ago
|
||
I am not too concerned with the exact times.
In our project, just updating 200 nodes using the replaceNodeWithChildren function causes the browser to freeze long enough for the "the script is taking too long to run. Do you want to stop, continue, etc." message to come up. We are using more css and have a much more complex DOM layout.
In firefox 3.0, this took no more then a half a second.
|
||
Comment 10•15 years ago
|
||
Well. Sure. I was just cleaning it up in case the quirkiness was the issue (it wasn't). And if your speeds are fast enough, I guess it doesn't matter that your approach (yes, even in production) slows it down by an order of magnitude. Isn't like the user notices 50ms vs 5ms.
In any case. There really is a bug so, CCing bz and hoping he adds his wisdom.
|
|
Reporter | |
Comment 11•15 years ago
|
||
OOh, ok. Thank you very much.
|
|
||
Comment 12•15 years ago
|
||
This is a regression from bug 162063. In particular, the page does the following over and over again:
1) Set the innerHTML of a <table> to the string
"<tbody><tr><td><b>Line 1:</b> Hello</td><td>World</td></tr>
<tr><td>Goodbye</td><td>World</td></tr></tbody>"
2) Remove the resulting <tbody> from its parent.
3) Append this <tbody> to a <tbody>
4) For each row in the inner <tbody>, insert it before the tbody (as a direct
child of the <tbody>)
5) Remove the inner <tbody>.
The fix for bug 162063 makes step 5 also remove the table, table cell, and table row that got produced when a tbody child was added to a tbody. We used to not remove those. The problem is that the removal process is pretty slow, and in particular O(N) in the number of kids the tbody has. Since it happens for every single iteration of the loop, you get overall O(N^2) behavior.
Bug 494546 should help here.
|
|
||
Comment 13•15 years ago
|
||
Yeah, with that patch time for the visible version drops by a factor of 30 or so.
|
|
||
Comment 14•15 years ago
|
||
Fixed by checkin for bug 494546.
You need to log in
before you can comment on or make changes to this bug.
Description
•