Title: [130548] trunk/Source/WebCore
- Revision
- 130548
- Author
- e...@webkit.org
- Date
- 2012-10-05 13:47:16 -0700 (Fri, 05 Oct 2012)
Log Message
Make tables which don't use col/row span much faster to layout
https://bugs.webkit.org/show_bug.cgi?id=98221
Reviewed by Julien Chaffraix.
My sense is that most tables on webpages do not use colspan/rowspan
so I stole another bit from RenderTableCell::m_column to avoid
having to re-parse the colSpan/rowSpan attributes for every cell
when doing table layout.
This made these symbols disappear from biggrid.html/redraw.html (dglazkov's spreadsheets benchmarks)
as well as moved our robohornet/resizecol.html number from an average of 3221ms to 2608ms (~20%!).
I removed m_hasHTMLTableCellElement (from http://trac.webkit.org/changeset/97691)
since it was attempting to do the same sort of optimization.
* rendering/RenderTableCell.cpp:
(WebCore::RenderTableCell::RenderTableCell):
(WebCore::RenderTableCell::parseColSpanFromDOM):
(WebCore::RenderTableCell::parseRowSpanFromDOM):
(WebCore::RenderTableCell::layout):
* rendering/RenderTableCell.h:
(WebCore::RenderTableCell::colSpan):
(WebCore::RenderTableCell::rowSpan):
(RenderTableCell):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (130547 => 130548)
--- trunk/Source/WebCore/ChangeLog 2012-10-05 20:45:25 UTC (rev 130547)
+++ trunk/Source/WebCore/ChangeLog 2012-10-05 20:47:16 UTC (rev 130548)
@@ -1,3 +1,30 @@
+2012-10-05 Eric Seidel <e...@webkit.org>
+
+ Make tables which don't use col/row span much faster to layout
+ https://bugs.webkit.org/show_bug.cgi?id=98221
+
+ Reviewed by Julien Chaffraix.
+
+ My sense is that most tables on webpages do not use colspan/rowspan
+ so I stole another bit from RenderTableCell::m_column to avoid
+ having to re-parse the colSpan/rowSpan attributes for every cell
+ when doing table layout.
+ This made these symbols disappear from biggrid.html/redraw.html (dglazkov's spreadsheets benchmarks)
+ as well as moved our robohornet/resizecol.html number from an average of 3221ms to 2608ms (~20%!).
+
+ I removed m_hasHTMLTableCellElement (from http://trac.webkit.org/changeset/97691)
+ since it was attempting to do the same sort of optimization.
+
+ * rendering/RenderTableCell.cpp:
+ (WebCore::RenderTableCell::RenderTableCell):
+ (WebCore::RenderTableCell::parseColSpanFromDOM):
+ (WebCore::RenderTableCell::parseRowSpanFromDOM):
+ (WebCore::RenderTableCell::layout):
+ * rendering/RenderTableCell.h:
+ (WebCore::RenderTableCell::colSpan):
+ (WebCore::RenderTableCell::rowSpan):
+ (RenderTableCell):
+
2012-10-05 Oli Lan <oli...@chromium.org>
Allow EventHandler to handle longpress gestures, including longpress selection on Android.
Modified: trunk/Source/WebCore/rendering/RenderTableCell.cpp (130547 => 130548)
--- trunk/Source/WebCore/rendering/RenderTableCell.cpp 2012-10-05 20:45:25 UTC (rev 130547)
+++ trunk/Source/WebCore/rendering/RenderTableCell.cpp 2012-10-05 20:47:16 UTC (rev 130548)
@@ -47,14 +47,24 @@
using namespace HTMLNames;
+struct SameSizeAsRenderTableCell : public RenderBlock {
+ unsigned bitfields;
+ int paddings[2];
+};
+
+COMPILE_ASSERT(sizeof(RenderTableCell) == sizeof(SameSizeAsRenderTableCell), RenderTableCell_should_stay_small);
+
+
RenderTableCell::RenderTableCell(Node* node)
: RenderBlock(node)
, m_column(unsetColumnIndex)
, m_cellWidthChanged(false)
- , m_hasHTMLTableCellElement(node && (node->hasTagName(tdTag) || node->hasTagName(thTag)))
, m_intrinsicPaddingBefore(0)
, m_intrinsicPaddingAfter(0)
{
+ // We only update the flags when notified of DOM changes in colSpanOrRowSpanChanged()
+ // so we need to set their initial values here in case something asks for colSpan()/rowSpan() before then.
+ updateColAndRowSpanFlags();
}
void RenderTableCell::willBeRemovedFromTree()
@@ -65,46 +75,40 @@
section()->removeCachedCollapsedBorders(this);
}
+unsigned RenderTableCell::parseColSpanFromDOM() const
+{
+ ASSERT(node());
+ if (node()->hasTagName(tdTag) || node()->hasTagName(thTag))
+ return toHTMLTableCellElement(node())->colSpan();
#if ENABLE(MATHML)
-inline bool isMathMLElement(Node* node)
-{
- return node && node->isElementNode() && toElement(node)->isMathMLElement();
+ if (node()->hasTagName(MathMLNames::mtdTag))
+ return toMathMLElement(node())->colSpan();
+#endif
+ return 1;
}
-#endif
-unsigned RenderTableCell::colSpan() const
+unsigned RenderTableCell::parseRowSpanFromDOM() const
{
- if (UNLIKELY(!m_hasHTMLTableCellElement)) {
+ ASSERT(node());
+ if (node()->hasTagName(tdTag) || node()->hasTagName(thTag))
+ return toHTMLTableCellElement(node())->rowSpan();
#if ENABLE(MATHML)
- if (isMathMLElement(node()))
- return toMathMLElement(node())->colSpan();
+ if (node()->hasTagName(MathMLNames::mtdTag))
+ return toMathMLElement(node())->rowSpan();
#endif
- return 1;
- }
-
- return toHTMLTableCellElement(node())->colSpan();
+ return 1;
}
-unsigned RenderTableCell::rowSpan() const
+void RenderTableCell::updateColAndRowSpanFlags()
{
- if (UNLIKELY(!m_hasHTMLTableCellElement)) {
-#if ENABLE(MATHML)
- if (isMathMLElement(node()))
- return toMathMLElement(node())->rowSpan();
-#endif
- return 1;
- }
-
- return toHTMLTableCellElement(node())->rowSpan();
+ // The vast majority of table cells do not have a colspan or rowspan,
+ // so we keep a bool to know if we need to bother reading from the DOM.
+ m_hasColSpan = node() && parseColSpanFromDOM() != 1;
+ m_hasRowSpan = node() && parseRowSpanFromDOM() != 1;
}
void RenderTableCell::colSpanOrRowSpanChanged()
{
-#if ENABLE(MATHML)
- ASSERT(m_hasHTMLTableCellElement || isMathMLElement(node()));
-#else
- ASSERT(m_hasHTMLTableCellElement);
-#endif
ASSERT(node());
#if ENABLE(MATHML)
ASSERT(node()->hasTagName(tdTag) || node()->hasTagName(thTag) || node()->hasTagName(MathMLNames::mtdTag));
@@ -112,6 +116,10 @@
ASSERT(node()->hasTagName(tdTag) || node()->hasTagName(thTag));
#endif
+ updateColAndRowSpanFlags();
+
+ // FIXME: I suspect that we could return early here if !m_hasColSpan && !m_hasRowSpan.
+
setNeedsLayoutAndPrefWidthsRecalc();
if (parent() && section())
section()->setNeedsCellRecalc();
Modified: trunk/Source/WebCore/rendering/RenderTableCell.h (130547 => 130548)
--- trunk/Source/WebCore/rendering/RenderTableCell.h 2012-10-05 20:45:25 UTC (rev 130547)
+++ trunk/Source/WebCore/rendering/RenderTableCell.h 2012-10-05 20:47:16 UTC (rev 130548)
@@ -30,8 +30,8 @@
namespace WebCore {
-static const unsigned unsetColumnIndex = 0x3FFFFFFF;
-static const unsigned maxColumnIndex = 0x3FFFFFFE; // 1,073,741,823
+static const unsigned unsetColumnIndex = 0x1FFFFFFF;
+static const unsigned maxColumnIndex = 0x1FFFFFFE; // 536,870,910
enum IncludeBorderColorOrNot { DoNotIncludeBorderColor, IncludeBorderColor };
@@ -39,8 +39,18 @@
public:
explicit RenderTableCell(Node*);
- unsigned colSpan() const;
- unsigned rowSpan() const;
+ unsigned colSpan() const
+ {
+ if (!m_hasColSpan)
+ return 1;
+ return parseColSpanFromDOM();
+ }
+ unsigned rowSpan() const
+ {
+ if (!m_hasRowSpan)
+ return 1;
+ return parseRowSpanFromDOM();
+ }
// Called from HTMLTableCellElement.
void colSpanOrRowSpanChanged();
@@ -227,9 +237,16 @@
CollapsedBorderValue computeCollapsedBeforeBorder(IncludeBorderColorOrNot = IncludeBorderColor) const;
CollapsedBorderValue computeCollapsedAfterBorder(IncludeBorderColorOrNot = IncludeBorderColor) const;
- unsigned m_column : 30;
- bool m_cellWidthChanged : 1;
- bool m_hasHTMLTableCellElement : 1;
+ void updateColAndRowSpanFlags();
+
+ unsigned parseRowSpanFromDOM() const;
+ unsigned parseColSpanFromDOM() const;
+
+ // Note MSVC will only pack members if they have identical types, hence we use unsigned instead of bool here.
+ unsigned m_column : 29;
+ unsigned m_cellWidthChanged : 1;
+ unsigned m_hasColSpan: 1;
+ unsigned m_hasRowSpan: 1;
int m_intrinsicPaddingBefore;
int m_intrinsicPaddingAfter;
};
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-changes
