Title: [97691] trunk/Source/WebCore
- Revision
- 97691
- Author
- [email protected]
- Date
- 2011-10-17 18:19:17 -0700 (Mon, 17 Oct 2011)
Log Message
Remove colSpan / rowSpan caching from RenderTableCell
https://bugs.webkit.org/show_bug.cgi?id=69569
Reviewed by Darin Adler.
Memory optimization, no change in behavior expected.
This change removes m_colSpan & m_rowSpan from RenderTableCell (inspired by
kling's memory shaving effort).
This makes us save 8 bytes per RenderTableCell on my machine (x86-64). No slowdown
on PageCycler Alexa-US.
This change refactored the way we handle updates from the DOM side to simplify
the code using the following: colspan / rowspan updates always go through
parseMappedAttribute where we already check for the renderer type. Thus removed the
generic updateFromElement and replaced it with colSpanOrRowSpanChanged. This removes
a virtual dispatch.
As there is no way to know if an attribute has changed in the parseMappedAttribute code,
we now unconditionally call colSpanOrRowSpanChanged. Looking at Chromium's page data,
colSpan and rowSpan are never changed outside the HTML markup thus such a change should
have a limited impact.
* html/HTMLTableCellElement.cpp:
(WebCore::HTMLTableCellElement::parseMappedAttribute): Updated after updateFromElement
removal.
* html/HTMLTableCellElement.h:
(WebCore::toHTMLTableCellElement): Added the usual conversion functions.
* rendering/RenderTableCell.cpp:
(WebCore::RenderTableCell::RenderTableCell): Added a boolean to know if we have
the right type of associated DOM node to avoid the cost of checking that every
time.
(WebCore::RenderTableCell::colSpan):
(WebCore::RenderTableCell::rowSpan):
Forwarded the calls to our object if we have the right type (normal case).
(WebCore::RenderTableCell::colSpanOrRowSpanChanged): Handles the
updateFromElement calls but in a more streamlined way.
* rendering/RenderTableCell.h:
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (97690 => 97691)
--- trunk/Source/WebCore/ChangeLog 2011-10-18 01:15:14 UTC (rev 97690)
+++ trunk/Source/WebCore/ChangeLog 2011-10-18 01:19:17 UTC (rev 97691)
@@ -1,3 +1,50 @@
+2011-10-17 Julien Chaffraix <[email protected]>
+
+ Remove colSpan / rowSpan caching from RenderTableCell
+ https://bugs.webkit.org/show_bug.cgi?id=69569
+
+ Reviewed by Darin Adler.
+
+ Memory optimization, no change in behavior expected.
+
+ This change removes m_colSpan & m_rowSpan from RenderTableCell (inspired by
+ kling's memory shaving effort).
+
+ This makes us save 8 bytes per RenderTableCell on my machine (x86-64). No slowdown
+ on PageCycler Alexa-US.
+
+ This change refactored the way we handle updates from the DOM side to simplify
+ the code using the following: colspan / rowspan updates always go through
+ parseMappedAttribute where we already check for the renderer type. Thus removed the
+ generic updateFromElement and replaced it with colSpanOrRowSpanChanged. This removes
+ a virtual dispatch.
+
+ As there is no way to know if an attribute has changed in the parseMappedAttribute code,
+ we now unconditionally call colSpanOrRowSpanChanged. Looking at Chromium's page data,
+ colSpan and rowSpan are never changed outside the HTML markup thus such a change should
+ have a limited impact.
+
+ * html/HTMLTableCellElement.cpp:
+ (WebCore::HTMLTableCellElement::parseMappedAttribute): Updated after updateFromElement
+ removal.
+
+ * html/HTMLTableCellElement.h:
+ (WebCore::toHTMLTableCellElement): Added the usual conversion functions.
+
+ * rendering/RenderTableCell.cpp:
+ (WebCore::RenderTableCell::RenderTableCell): Added a boolean to know if we have
+ the right type of associated DOM node to avoid the cost of checking that every
+ time.
+
+ (WebCore::RenderTableCell::colSpan):
+ (WebCore::RenderTableCell::rowSpan):
+ Forwarded the calls to our object if we have the right type (normal case).
+
+ (WebCore::RenderTableCell::colSpanOrRowSpanChanged): Handles the
+ updateFromElement calls but in a more streamlined way.
+
+ * rendering/RenderTableCell.h:
+
2011-10-17 James Robinson <[email protected]>
[chromium] Fix shutdown race when posting main thread task to CCThreadProxy and enable tests
Modified: trunk/Source/WebCore/html/HTMLTableCellElement.cpp (97690 => 97691)
--- trunk/Source/WebCore/html/HTMLTableCellElement.cpp 2011-10-18 01:15:14 UTC (rev 97690)
+++ trunk/Source/WebCore/html/HTMLTableCellElement.cpp 2011-10-18 01:19:17 UTC (rev 97691)
@@ -95,10 +95,10 @@
{
if (attr->name() == rowspanAttr) {
if (renderer() && renderer()->isTableCell())
- toRenderTableCell(renderer())->updateFromElement();
+ toRenderTableCell(renderer())->colSpanOrRowSpanChanged();
} else if (attr->name() == colspanAttr) {
if (renderer() && renderer()->isTableCell())
- toRenderTableCell(renderer())->updateFromElement();
+ toRenderTableCell(renderer())->colSpanOrRowSpanChanged();
} else if (attr->name() == nowrapAttr) {
if (!attr->isNull())
addCSSProperty(attr, CSSPropertyWhiteSpace, CSSValueWebkitNowrap);
@@ -187,4 +187,20 @@
return static_cast<HTMLTableCellElement*>(cellAboveRenderer->node());
}
+#ifndef NDEBUG
+
+HTMLTableCellElement* toHTMLTableCellElement(Node* node)
+{
+ ASSERT(!node || node->hasTagName(HTMLNames::tdTag) || node->hasTagName(HTMLNames::thTag));
+ return static_cast<HTMLTableCellElement*>(node);
+}
+
+const HTMLTableCellElement* toHTMLTableCellElement(const Node* node)
+{
+ ASSERT(!node || node->hasTagName(HTMLNames::tdTag) || node->hasTagName(HTMLNames::thTag));
+ return static_cast<const HTMLTableCellElement*>(node);
+}
+
+#endif
+
} // namespace WebCore
Modified: trunk/Source/WebCore/html/HTMLTableCellElement.h (97690 => 97691)
--- trunk/Source/WebCore/html/HTMLTableCellElement.h 2011-10-18 01:15:14 UTC (rev 97690)
+++ trunk/Source/WebCore/html/HTMLTableCellElement.h 2011-10-18 01:19:17 UTC (rev 97691)
@@ -65,6 +65,30 @@
virtual void addSubresourceAttributeURLs(ListHashSet<KURL>&) const;
};
+HTMLTableCellElement* toHTMLTableCellElement(Node* node);
+const HTMLTableCellElement* toHTMLTableCellElement(const Node* node);
+// This will catch anyone doing an unnecessary cast.
+void toHTMLTableCellElement(const HTMLTableCellElement*);
+
+#ifdef NDEBUG
+
+// The debug versions of these, with assertions, are not inlined.
+
+inline HTMLTableCellElement* toHTMLTableCellElement(Node* node)
+{
+ return static_cast<HTMLTableCellElement*>(node);
+}
+
+inline const HTMLTableCellElement* toHTMLTableCellElement(const Node* node)
+{
+ return static_cast<const HTMLTableCellElement*>(node);
+}
+
+// This will catch anyone doing an unnecessary cast.
+void toHTMLTableCellElement(const HTMLTableCellElement*);
+
+#endif
+
} // namespace
#endif
Modified: trunk/Source/WebCore/rendering/RenderTableCell.cpp (97690 => 97691)
--- trunk/Source/WebCore/rendering/RenderTableCell.cpp 2011-10-18 01:15:14 UTC (rev 97690)
+++ trunk/Source/WebCore/rendering/RenderTableCell.cpp 2011-10-18 01:19:17 UTC (rev 97691)
@@ -45,13 +45,11 @@
: RenderBlock(node)
, m_row(-1)
, m_column(-1)
- , m_rowSpan(1)
- , m_columnSpan(1)
- , m_cellWidthChanged(false)
, m_intrinsicPaddingBefore(0)
, m_intrinsicPaddingAfter(0)
+ , m_cellWidthChanged(false)
+ , m_hasAssociatedTableCellElement(node && (node->hasTagName(tdTag) || node->hasTagName(thTag)))
{
- updateFromElement();
}
void RenderTableCell::willBeDestroyed()
@@ -64,24 +62,33 @@
recalcSection->setNeedsCellRecalc();
}
-void RenderTableCell::updateFromElement()
+int RenderTableCell::colSpan() const
{
- Node* n = node();
- if (n && (n->hasTagName(tdTag) || n->hasTagName(thTag))) {
- HTMLTableCellElement* tc = static_cast<HTMLTableCellElement*>(n);
- int oldRSpan = m_rowSpan;
- int oldCSpan = m_columnSpan;
+ if (UNLIKELY(!m_hasAssociatedTableCellElement))
+ return 1;
- m_columnSpan = tc->colSpan();
- m_rowSpan = tc->rowSpan();
- if ((oldRSpan != m_rowSpan || oldCSpan != m_columnSpan) && style() && parent()) {
- setNeedsLayoutAndPrefWidthsRecalc();
- if (section())
- section()->setNeedsCellRecalc();
- }
- }
+ return toHTMLTableCellElement(node())->colSpan();
}
+int RenderTableCell::rowSpan() const
+{
+ if (UNLIKELY(!m_hasAssociatedTableCellElement))
+ return 1;
+
+ return toHTMLTableCellElement(node())->rowSpan();
+}
+
+void RenderTableCell::colSpanOrRowSpanChanged()
+{
+ ASSERT(m_hasAssociatedTableCellElement);
+ ASSERT(node());
+ ASSERT(node()->hasTagName(tdTag) || node()->hasTagName(thTag));
+
+ setNeedsLayoutAndPrefWidthsRecalc();
+ if (parent() && section())
+ section()->setNeedsCellRecalc();
+}
+
Length RenderTableCell::styleOrColLogicalWidth() const
{
Length w = style()->logicalWidth();
Modified: trunk/Source/WebCore/rendering/RenderTableCell.h (97690 => 97691)
--- trunk/Source/WebCore/rendering/RenderTableCell.h 2011-10-18 01:15:14 UTC (rev 97690)
+++ trunk/Source/WebCore/rendering/RenderTableCell.h 2011-10-18 01:19:17 UTC (rev 97691)
@@ -37,11 +37,11 @@
int cellIndex() const { return 0; }
void setCellIndex(int) { }
- int colSpan() const { return m_columnSpan; }
- void setColSpan(int c) { m_columnSpan = c; }
+ int colSpan() const;
+ int rowSpan() const;
- int rowSpan() const { return m_rowSpan; }
- void setRowSpan(int r) { m_rowSpan = r; }
+ // Called from HTMLTableCellElement.
+ void colSpanOrRowSpanChanged();
int col() const { return m_column; }
void setCol(int col) { m_column = col; }
@@ -89,8 +89,6 @@
void collectBorderValues(RenderTable::CollapsedBorderValues&) const;
static void sortBorderValues(RenderTable::CollapsedBorderValues&);
- virtual void updateFromElement();
-
virtual void layout();
virtual void paint(PaintInfo&, const LayoutPoint&);
@@ -149,11 +147,12 @@
int m_row;
int m_column;
- int m_rowSpan;
- int m_columnSpan : 31;
- bool m_cellWidthChanged : 1;
int m_intrinsicPaddingBefore;
int m_intrinsicPaddingAfter;
+
+ // FIXME: It would be nice to pack these 2 bits into some of the previous fields.
+ bool m_cellWidthChanged : 1;
+ bool m_hasAssociatedTableCellElement : 1;
};
inline RenderTableCell* toRenderTableCell(RenderObject* object)
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes
