As reported at http://crbug.com/94248 : 1. Create a bunch (2000 table rows) of elements 2. Hide them using CSS (display:none) 3. Show them using CSS (display:none) See attached file (showhide.html) for repro. Safari Version 5.0.4 (6533.20.27) (MacOS 10.6) - takes approx 1 sec to toggle WebKit Nightly r93043 (MacOS 10.6) - takes > 30 secs to toggle Chrome 15.0.865.0 dev (MacOS 10.6) - takes > 30 secs to toggle
Created attachment 106297 [details] showhide.html
<rdar://problem/10075663>
Morrita-san, Antti thinks this might have been caused by NodeRenderingContext/NodeRendererFactory refactoring.
Sample showing it spending time under WebCore::NodeRenderingContext::nextRenderer(): Call graph: 2483 Thread_37033 DispatchQueue_1: com.apple.main-thread (serial) + 2483 ??? (in Safari) load address 0x109e36000 + 0xf24 [0x109e36f24] + 2483 SafariMain (in Safari) + 197 [0x7fff955e4725] + 2483 NSApplicationMain (in AppKit) + 867 [0x7fff90e7652a] + 2483 -[NSApplication run] (in AppKit) + 548 [0x7fff90bf842b] + 2483 -[BrowserApplication sendEvent:] (in Safari) + 822 [0x7fff9543247a] + 2483 -[NSApplication sendEvent:] (in AppKit) + 5665 [0x7fff90c61f19] + 2483 -[BrowserWindow sendEvent:] (in Safari) + 378 [0x7fff954897e8] + 2483 -[Window sendEvent:] (in Safari) + 115 [0x7fff956820c5] + 2483 -[NSWindow sendEvent:] (in AppKit) + 6478 [0x7fff90cc9734] + 2483 -[WebHTMLView mouseUp:] (in WebKit) + 223 [0x10a24347f] WebHTMLView.mm:3639 + 2483 WebCore::EventHandler::mouseUp(NSEvent*) (in WebCore) + 206 [0x10acbc39e] EventHandlerMac.mm:527 + 2483 WebCore::EventHandler::handleMouseReleaseEvent(WebCore::PlatformMouseEvent const&) (in WebCore) + 1024 [0x10acb6b50] EventHandler.cpp:1717 + 2483 WebCore::EventHandler::handleMouseReleaseEvent(WebCore::MouseEventWithHitTestResults const&) (in WebCore) + 618 [0x10acb343a] EventHandler.cpp:760 + 2483 WebCore::FrameSelection::notifyRendererOfSelectionChange(WebCore::EUserTriggered) (in WebCore) + 39 [0x10ad156c7] FrameSelection.cpp:1690 + 2483 WebCore::Document::updateStyleIfNeeded() (in WebCore) + 67 [0x10aba5253] Document.cpp:1621 + 2483 WebCore::Document::recalcStyle(WebCore::Node::StyleChange) (in WebCore) + 459 [0x10aba446b] Document.cpp:1564 + 2483 WebCore::Element::recalcStyle(WebCore::Node::StyleChange) (in WebCore) + 2378 [0x10aca705a] Element.cpp:1161 + 2483 WebCore::Element::recalcStyle(WebCore::Node::StyleChange) (in WebCore) + 2378 [0x10aca705a] Element.cpp:1161 + 2483 WebCore::Element::recalcStyle(WebCore::Node::StyleChange) (in WebCore) + 2378 [0x10aca705a] Element.cpp:1161 + 2483 WebCore::Element::recalcStyle(WebCore::Node::StyleChange) (in WebCore) + 2378 [0x10aca705a] Element.cpp:1161 + 2483 WebCore::Element::recalcStyle(WebCore::Node::StyleChange) (in WebCore) + 653 [0x10aca699d] Node.h:761 + 2473 WebCore::Element::attach() (in WebCore) + 101 [0x10aca5d35] Element.cpp:974 + ! 2464 WebCore::Node::attach() (in WebCore) + 79 [0x10b277f7f] Node.cpp:1464 + ! : 2435 WebCore::NodeRendererFactory::createRendererIfNeeded() (in WebCore) + 122 [0x10b281b6a] NodeRenderingContext.cpp:349 + ! : | 1703 WebCore::NodeRenderingContext::nextRenderer() const (in WebCore) + 270 [0x10b28151e] Node.h:437 + ! : | 343 WebCore::NodeRenderingContext::nextRenderer() const (in WebCore) + 287 [0x10b28152f] NodeRenderingContext.cpp:184 + ! : | + 343 WebCore::Node::isContentElement() const (in WebCore) + 0,6,... [0x10aa42530,0x10aa42536,...] Node.h:215 + ! : | 333 WebCore::NodeRenderingContext::nextRenderer() const (in WebCore) + 278,287,... [0x10b281526,0x10b28152f,...] NodeRenderingContext.cpp:184 + ! : | 56 WebCore::NodeRenderingContext::nextRenderer() const (in WebCore) + 251,255 [0x10b28150b,0x10b28150f] Node.h:151 + ! : 27 WebCore::NodeRendererFactory::createRendererIfNeeded() (in WebCore) + 133 [0x10b281b75] NodeRenderingContext.cpp:350
Before NodeRenderingContext this was probably just walking RenderObjects. Is it the code that was added for coalescing adjacent text nodes?
Ouch, We shouldn't need to be doing that isContentElement check. The ShadowContentElement is meaningless in the non-shadow DOM tree!
I am working on this.
(In reply to comment #7) > I am working on this. The patch that regressed this made nextRenderer() call even if the newRenderer is 0. And that's slow. I'll have patch ready tomorrow morning.
Created attachment 106939 [details] Patch
There's probably a perf test I should write for this. But let's fix the bug first.
Comment on attachment 106939 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=106939&action=review > Source/WebCore/ChangeLog:8 > + Covered by existing tests. Maybe the code is covered, but is the performance covered? Can we make a performance test to demonstrate what is fixed here? I think the performance test machinery Ojan create makes that kind of thing easy.
Comment on attachment 106939 [details] Patch will do.
Created attachment 106981 [details] Patch for landing
Comment on attachment 106981 [details] Patch for landing Clearing flags on attachment: 106981 Committed r94938: <http://trac.webkit.org/changeset/94938>
All reviewed patches have been landed. Closing bug.