- Revision
- 132764
- Author
- [email protected]
- Date
- 2012-10-29 00:38:01 -0700 (Mon, 29 Oct 2012)
Log 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:
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (132763 => 132764)
--- trunk/Source/WebCore/ChangeLog 2012-10-29 07:29:43 UTC (rev 132763)
+++ trunk/Source/WebCore/ChangeLog 2012-10-29 07:38:01 UTC (rev 132764)
@@ -1,3 +1,33 @@
+2012-10-29 Eric Seidel <[email protected]>
+
+ 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 <[email protected]>
Rename Localizer.{cpp,h} to PlatformLocale.{cpp,h}
Modified: trunk/Source/WebCore/rendering/RenderTable.cpp (132763 => 132764)
--- trunk/Source/WebCore/rendering/RenderTable.cpp 2012-10-29 07:29:43 UTC (rev 132763)
+++ trunk/Source/WebCore/rendering/RenderTable.cpp 2012-10-29 07:38:01 UTC (rev 132764)
@@ -59,6 +59,7 @@
, m_hasColElements(false)
, m_needsSectionRecalc(false)
, m_columnLogicalWidthChanged(false)
+ , m_columnRenderersValid(false)
, m_hSpacing(0)
, m_vSpacing(0)
, m_borderStart(0)
@@ -213,6 +214,26 @@
m_captions.remove(index);
}
+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()
{
recalcSectionsIfNeeded();
@@ -759,15 +780,30 @@
return 0;
}
-RenderTableCol* RenderTable::slowColElement(unsigned col, bool* startEdge, bool* endEdge) const
+void RenderTable::updateColumnCache() const
{
ASSERT(m_hasColElements);
+ ASSERT(m_columnRenderers.isEmpty());
+ ASSERT(!m_columnRenderersValid);
- unsigned columnCount = 0;
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;
ASSERT(span >= 1);
@@ -781,7 +817,6 @@
return columnRenderer;
}
}
-
return 0;
}
Modified: trunk/Source/WebCore/rendering/RenderTable.h (132763 => 132764)
--- trunk/Source/WebCore/rendering/RenderTable.h 2012-10-29 07:29:43 UTC (rev 132763)
+++ trunk/Source/WebCore/rendering/RenderTable.h 2012-10-29 07:38:01 UTC (rev 132764)
@@ -45,8 +45,6 @@
explicit RenderTable(Node*);
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".
int hBorderSpacing() const { return m_hSpacing; }
@@ -258,6 +256,8 @@
void addCaption(const RenderTableCaption*);
void removeCaption(const RenderTableCaption*);
+ void addColumn(const RenderTableCol*);
+ void removeColumn(const RenderTableCol*);
protected:
virtual void styleDidChange(StyleDifference, const RenderStyle* oldStyle);
@@ -283,6 +283,9 @@
RenderTableCol* slowColElement(unsigned col, bool* startEdge, bool* endEdge) const;
+ void updateColumnCache() const;
+ void invalidateCachedColumns();
+
virtual RenderBlock* firstLineBlock() const;
virtual void updateFirstLetter();
@@ -305,6 +308,7 @@
mutable Vector<int> m_columnPos;
mutable Vector<ColumnStruct> m_columns;
mutable Vector<RenderTableCaption*> m_captions;
+ mutable Vector<RenderTableCol*> m_columnRenderers;
mutable RenderTableSection* m_head;
mutable RenderTableSection* m_foot;
@@ -315,10 +319,12 @@
CollapsedBorderValues m_collapsedBorders;
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;
short m_vSpacing;
Modified: trunk/Source/WebCore/rendering/RenderTableCol.cpp (132763 => 132764)
--- trunk/Source/WebCore/rendering/RenderTableCol.cpp 2012-10-29 07:29:43 UTC (rev 132763)
+++ trunk/Source/WebCore/rendering/RenderTableCol.cpp 2012-10-29 07:38:01 UTC (rev 132764)
@@ -70,14 +70,16 @@
setNeedsLayoutAndPrefWidthsRecalc();
}
+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);
}
bool RenderTableCol::isChildAllowed(RenderObject* child, RenderStyle* style) const
Modified: trunk/Source/WebCore/rendering/RenderTableCol.h (132763 => 132764)
--- trunk/Source/WebCore/rendering/RenderTableCol.h 2012-10-29 07:29:43 UTC (rev 132763)
+++ trunk/Source/WebCore/rendering/RenderTableCol.h 2012-10-29 07:38:01 UTC (rev 132764)
@@ -81,6 +81,7 @@
virtual bool isRenderTableCol() const OVERRIDE { return true; }
virtual void updateFromElement();
+ virtual void insertedIntoTree() OVERRIDE;
virtual void willBeRemovedFromTree() OVERRIDE;
virtual bool isChildAllowed(RenderObject*, RenderStyle*) const;
