Changeset 132764 in webkit

Timestamp:

Oct 29, 2012, 12:38:01 AM (13 years ago)

Author:

eric@webkit.org

Message:

Make rendering tables with <colgroups> twice as fast by avoiding walking the DOM for colgroups 4 times for each cell
​https://bugs.webkit.org/show_bug.cgi?id=100630

Reviewed by Ojan Vafai.

This is not a complete fix. Our rendering of this large tables still takes 7.8 seconds
on my retina MBP (down from 14.3s before this change).
It's very expensive to walk the DOM each time we call RenderTable::colElement
so this caches the RenderTableCol* in a vector for easier walking.
We invalidate the cache any time a RenderTableCol is added or removed from the
rendering sub-tree to avoid holding a bad pointer.

• rendering/RenderTable.cpp:

(WebCore::RenderTable::RenderTable):
(WebCore::RenderTable::invalidateCachedColumns):
(WebCore):
(WebCore::RenderTable::addColumn):
(WebCore::RenderTable::removeColumn):
(WebCore::RenderTable::updateColumnCache):
(WebCore::RenderTable::slowColElement):

• rendering/RenderTable.h:

(RenderTable):

• rendering/RenderTableCol.cpp:

(WebCore::RenderTableCol::insertedIntoTree):
(WebCore):
(WebCore::RenderTableCol::willBeRemovedFromTree):

• rendering/RenderTableCol.h:

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• rendering/RenderTable.cpp (modified) (4 diffs)
• rendering/RenderTable.h (modified) (5 diffs)
• rendering/RenderTableCol.cpp (modified) (1 diff)
• rendering/RenderTableCol.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-10-29  Eric Seidel  <eric@webkit.org>
+
+        Make rendering tables with <colgroups> twice as fast by avoiding walking the DOM for colgroups 4 times for each cell
+        https://bugs.webkit.org/show_bug.cgi?id=100630
+
+        Reviewed by Ojan Vafai.
+
+        This is not a complete fix.  Our rendering of this large tables still takes 7.8 seconds
+        on my retina MBP (down from 14.3s before this change).
+        It's very expensive to walk the DOM each time we call RenderTable::colElement
+        so this caches the RenderTableCol* in a vector for easier walking.
+        We invalidate the cache any time a RenderTableCol is added or removed from the
+        rendering sub-tree to avoid holding a bad pointer.
+
+        * rendering/RenderTable.cpp:
+        (WebCore::RenderTable::RenderTable):
+        (WebCore::RenderTable::invalidateCachedColumns):
+        (WebCore):
+        (WebCore::RenderTable::addColumn):
+        (WebCore::RenderTable::removeColumn):
+        (WebCore::RenderTable::updateColumnCache):
+        (WebCore::RenderTable::slowColElement):
+        * rendering/RenderTable.h:
+        (RenderTable):
+        * rendering/RenderTableCol.cpp:
+        (WebCore::RenderTableCol::insertedIntoTree):
+        (WebCore):
+        (WebCore::RenderTableCol::willBeRemovedFromTree):
+        * rendering/RenderTableCol.h:
+
 2012-10-28  Kent Tamura  <tkent@chromium.org>
 

trunk/Source/WebCore/rendering/RenderTable.cpp

@@ -60 +60 @@
     , m_needsSectionRecalc(false)
     , m_columnLogicalWidthChanged(false)
+    , m_columnRenderersValid(false)
     , m_hSpacing(0)
     , m_vSpacing(0)
@@ -214 +215 @@
 }
 
+void RenderTable::invalidateCachedColumns()
+{
+    m_columnRenderersValid = false;
+    m_columnRenderers.resize(0);
+}
+
+void RenderTable::addColumn(const RenderTableCol*)
+{
+    invalidateCachedColumns();
+}
+
+void RenderTable::removeColumn(const RenderTableCol*)
+{
+    invalidateCachedColumns();
+    // We don't really need to recompute our sections, but we need to update our
+    // column count and whether we have a column. Currently, we only have one
+    // size-fit-all flag but we may have to consider splitting it.
+    setNeedsSectionRecalc();
+}
+
 void RenderTable::updateLogicalWidth()
 {
@@ -760 +781 @@
 }
 
-RenderTableCol* RenderTable::slowColElement(unsigned col, bool* startEdge, bool* endEdge) const
+void RenderTable::updateColumnCache() const
 {
     ASSERT(m_hasColElements);
-
-    unsigned columnCount = 0;
+    ASSERT(m_columnRenderers.isEmpty());
+    ASSERT(!m_columnRenderersValid);
+
     for (RenderTableCol* columnRenderer = firstColumn(); columnRenderer; columnRenderer = columnRenderer->nextColumn()) {
         if (columnRenderer->isTableColumnGroupWithColumnChildren())
             continue;
-
+        m_columnRenderers.append(columnRenderer);
+    }
+    m_columnRenderersValid = true;
+}
+
+RenderTableCol* RenderTable::slowColElement(unsigned col, bool* startEdge, bool* endEdge) const
+{
+    ASSERT(m_hasColElements);
+
+    if (!m_columnRenderersValid)
+        updateColumnCache();
+
+    unsigned columnCount = 0;
+    for (unsigned i = 0; i < m_columnRenderers.size(); i++) {
+        RenderTableCol* columnRenderer = m_columnRenderers[i];
         unsigned span = columnRenderer->span();
         unsigned startCol = columnCount;
@@ -782 +818 @@
         }
     }
-
     return 0;
 }

trunk/Source/WebCore/rendering/RenderTable.h

@@ -46 +46 @@
     virtual ~RenderTable();
 
-    int getColumnPos(unsigned col) const { return m_columnPos[col]; }
-
     // Per CSS 3 writing-mode: "The first and second values of the 'border-spacing' property represent spacing between columns
     // and rows respectively, not necessarily the horizontal and vertical spacing respectively".
@@ -259 +257 @@
     void addCaption(const RenderTableCaption*);
     void removeCaption(const RenderTableCaption*);
+    void addColumn(const RenderTableCol*);
+    void removeColumn(const RenderTableCol*);
 
 protected:
@@ -284 +284 @@
     RenderTableCol* slowColElement(unsigned col, bool* startEdge, bool* endEdge) const;
 
+    void updateColumnCache() const;
+    void invalidateCachedColumns();
+
     virtual RenderBlock* firstLineBlock() const;
     virtual void updateFirstLetter();
@@ -306 +309 @@
     mutable Vector<ColumnStruct> m_columns;
     mutable Vector<RenderTableCaption*> m_captions;
+    mutable Vector<RenderTableCol*> m_columnRenderers;
 
     mutable RenderTableSection* m_head;
@@ -316 +320 @@
     const CollapsedBorderValue* m_currentBorder;
     bool m_collapsedBordersValid : 1;
-    
+
     mutable bool m_hasColElements : 1;
     mutable bool m_needsSectionRecalc : 1;
+
     bool m_columnLogicalWidthChanged : 1;
+    mutable bool m_columnRenderersValid: 1;
 
     short m_hSpacing;

trunk/Source/WebCore/rendering/RenderTableCol.cpp

@@ -71 +71 @@
 }
 
+void RenderTableCol::insertedIntoTree()
+{
+    RenderBox::insertedIntoTree();
+    table()->addColumn(this);
+}
+
 void RenderTableCol::willBeRemovedFromTree()
 {
     RenderBox::willBeRemovedFromTree();
-
-    // We don't really need to recompute our sections, but we need to update our
-    // column count and whether we have a column. Currently, we only have one
-    // size-fit-all flag but we may have to consider splitting it.
-    table()->setNeedsSectionRecalc();
+    table()->removeColumn(this);
 }
 

trunk/Source/WebCore/rendering/RenderTableCol.h

@@ -82 +82 @@
     virtual void updateFromElement();
 
+    virtual void insertedIntoTree() OVERRIDE;
     virtual void willBeRemovedFromTree() OVERRIDE;