- Revision
- 134408
- Author
- kl...@webkit.org
- Date
- 2012-11-13 06:21:28 -0800 (Tue, 13 Nov 2012)
Log Message
Exploit ElementAttributeData sharing in Node.cloneNode.
<http://webkit.org/b/101374>
Reviewed by Anders Carlsson.
Instead of blindly creating a new ElementAttributeData for Node.cloneNode, let's be clever!
If the source data is immutable, simply ref it from the new node at virtually no cost.
If the source data is mutable, convert it to immutable data so it can be shared between both nodes.
Since the typical use-case for Node.cloneNode is to create-once/clone-many, this saves both time
and memory in the long run.
~8% speed-up on PerformanceTests/DOM/CloneNodes on my MBP.
* dom/Element.cpp:
(WebCore::Element::cloneAttributesFromElement):
Move attribute data cloning logic from ElementAttributeData to Element.
* dom/ElementAttributeData.cpp:
(WebCore::ImmutableElementAttributeData::~ImmutableElementAttributeData):
(WebCore::ImmutableElementAttributeData::ImmutableElementAttributeData):
(WebCore::ElementAttributeData::ElementAttributeData):
(WebCore::MutableElementAttributeData::MutableElementAttributeData):
(WebCore::ElementAttributeData::makeMutableCopy):
(WebCore::ElementAttributeData::makeImmutableCopy):
* dom/ElementAttributeData.h:
(ElementAttributeData):
(ImmutableElementAttributeData):
(MutableElementAttributeData):
Add facilities for converting a mutable ElementAttributeData to an immutable one.
Share some code in the common base class constructor.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (134407 => 134408)
--- trunk/Source/WebCore/ChangeLog 2012-11-13 14:09:23 UTC (rev 134407)
+++ trunk/Source/WebCore/ChangeLog 2012-11-13 14:21:28 UTC (rev 134408)
@@ -1,3 +1,39 @@
+2012-11-13 Andreas Kling <kl...@webkit.org>
+
+ Exploit ElementAttributeData sharing in Node.cloneNode.
+ <http://webkit.org/b/101374>
+
+ Reviewed by Anders Carlsson.
+
+ Instead of blindly creating a new ElementAttributeData for Node.cloneNode, let's be clever!
+ If the source data is immutable, simply ref it from the new node at virtually no cost.
+
+ If the source data is mutable, convert it to immutable data so it can be shared between both nodes.
+ Since the typical use-case for Node.cloneNode is to create-once/clone-many, this saves both time
+ and memory in the long run.
+
+ ~8% speed-up on PerformanceTests/DOM/CloneNodes on my MBP.
+
+ * dom/Element.cpp:
+ (WebCore::Element::cloneAttributesFromElement):
+
+ Move attribute data cloning logic from ElementAttributeData to Element.
+
+ * dom/ElementAttributeData.cpp:
+ (WebCore::ImmutableElementAttributeData::~ImmutableElementAttributeData):
+ (WebCore::ImmutableElementAttributeData::ImmutableElementAttributeData):
+ (WebCore::ElementAttributeData::ElementAttributeData):
+ (WebCore::MutableElementAttributeData::MutableElementAttributeData):
+ (WebCore::ElementAttributeData::makeMutableCopy):
+ (WebCore::ElementAttributeData::makeImmutableCopy):
+ * dom/ElementAttributeData.h:
+ (ElementAttributeData):
+ (ImmutableElementAttributeData):
+ (MutableElementAttributeData):
+
+ Add facilities for converting a mutable ElementAttributeData to an immutable one.
+ Share some code in the common base class constructor.
+
2012-11-13 Kentaro Hara <hara...@chromium.org>
Unreviewed. Build fix of V8 bindings.
Modified: trunk/Source/WebCore/dom/Element.cpp (134407 => 134408)
--- trunk/Source/WebCore/dom/Element.cpp 2012-11-13 14:09:23 UTC (rev 134407)
+++ trunk/Source/WebCore/dom/Element.cpp 2012-11-13 14:21:28 UTC (rev 134408)
@@ -4,7 +4,7 @@
* (C) 2001 Peter Kelly (p...@post.com)
* (C) 2001 Dirk Mueller (muel...@kde.org)
* (C) 2007 David Smith (catfish....@gmail.com)
- * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010 Apple Inc. All rights reserved.
+ * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2012 Apple Inc. All rights reserved.
* (C) 2007 Eric Seidel (e...@webkit.org)
*
* This library is free software; you can redistribute it and/or
@@ -2509,12 +2509,45 @@
if (hasSyntheticAttrChildNodes())
detachAllAttrNodesFromElement();
- if (const ElementAttributeData* attributeData = other.updatedAttributeData())
- mutableAttributeData()->cloneDataFrom(*attributeData, other, *this);
- else if (m_attributeData) {
- m_attributeData->clearAttributes();
+ setIsStyleAttributeValid(other.isStyleAttributeValid());
+
+ other.updateInvalidAttributes();
+ if (!other.m_attributeData) {
m_attributeData.clear();
+ return;
}
+
+ const AtomicString& oldID = getIdAttribute();
+ const AtomicString& newID = other.getIdAttribute();
+
+ if (!oldID.isNull() || !newID.isNull())
+ updateId(oldID, newID);
+
+ const AtomicString& oldName = getNameAttribute();
+ const AtomicString& newName = other.getNameAttribute();
+
+ if (!oldName.isNull() || !newName.isNull())
+ updateName(oldName, newName);
+
+ // If 'other' has a mutable ElementAttributeData, convert it to an immutable one so we can share it between both elements.
+ // We can only do this if there is no CSSOM wrapper for other's inline style (the isMutable() check.)
+ if (other.m_attributeData->isMutable() && (!other.m_attributeData->inlineStyle() || !other.m_attributeData->inlineStyle()->isMutable()))
+ const_cast<Element&>(other).m_attributeData = other.m_attributeData->makeImmutableCopy();
+
+ if (!other.m_attributeData->isMutable())
+ m_attributeData = other.m_attributeData;
+ else
+ m_attributeData = other.m_attributeData->makeMutableCopy();
+
+ for (unsigned i = 0; i < m_attributeData->length(); ++i) {
+ const Attribute* attribute = const_cast<const ElementAttributeData*>(m_attributeData.get())->attributeItem(i);
+ // This optimization isn't very nicely factored, but a huge win for cloning elements with inline style.
+ if (isStyledElement() && attribute->name() == HTMLNames::styleAttr) {
+ static_cast<StyledElement*>(this)->styleAttributeChanged(attribute->value(), StyledElement::DoNotReparseStyleAttribute);
+ continue;
+ }
+ attributeChanged(attribute->name(), attribute->value());
+ }
}
void Element::cloneDataFromElement(const Element& other)
Modified: trunk/Source/WebCore/dom/ElementAttributeData.cpp (134407 => 134408)
--- trunk/Source/WebCore/dom/ElementAttributeData.cpp 2012-11-13 14:09:23 UTC (rev 134407)
+++ trunk/Source/WebCore/dom/ElementAttributeData.cpp 2012-11-13 14:21:28 UTC (rev 134408)
@@ -58,35 +58,67 @@
new (&reinterpret_cast<Attribute*>(&m_attributeArray)[i]) Attribute(attributes[i]);
}
-MutableElementAttributeData::MutableElementAttributeData(const ImmutableElementAttributeData& other)
+ImmutableElementAttributeData::~ImmutableElementAttributeData()
{
- const ElementAttributeData& baseOther = static_cast<const ElementAttributeData&>(other);
+ for (unsigned i = 0; i < m_arraySize; ++i)
+ (reinterpret_cast<Attribute*>(&m_attributeArray)[i]).~Attribute();
+}
- m_inlineStyleDecl = baseOther.m_inlineStyleDecl;
- m_presentationAttributeStyle = baseOther.m_presentationAttributeStyle;
- m_classNames = baseOther.m_classNames;
- m_idForStyleResolution = baseOther.m_idForStyleResolution;
+ImmutableElementAttributeData::ImmutableElementAttributeData(const MutableElementAttributeData& other)
+ : ElementAttributeData(other, false)
+{
+ if (other.m_inlineStyleDecl) {
+ ASSERT(!other.m_inlineStyleDecl->isMutable());
+ m_inlineStyleDecl = other.m_inlineStyleDecl->immutableCopyIfNeeded();
+ }
- // An ImmutableElementAttributeData should never have a mutable inline StylePropertySet attached.
- ASSERT(!baseOther.m_inlineStyleDecl || !baseOther.m_inlineStyleDecl->isMutable());
+ for (unsigned i = 0; i < m_arraySize; ++i)
+ new (&reinterpret_cast<Attribute*>(&m_attributeArray)[i]) Attribute(*other.attributeItem(i));
+}
- m_attributeVector.reserveCapacity(baseOther.m_arraySize);
- for (unsigned i = 0; i < baseOther.m_arraySize; ++i)
- m_attributeVector.uncheckedAppend(other.immutableAttributeArray()[i]);
+ElementAttributeData::ElementAttributeData(const ElementAttributeData& other, bool isMutable)
+ : m_isMutable(isMutable)
+ , m_arraySize(isMutable ? 0 : other.length())
+ , m_presentationAttributeStyle(other.m_presentationAttributeStyle)
+ , m_classNames(other.m_classNames)
+ , m_idForStyleResolution(other.m_idForStyleResolution)
+{
+ // NOTE: The inline style is copied by the subclass copy constructor since we don't know what to do with it here.
}
-ImmutableElementAttributeData::~ImmutableElementAttributeData()
+MutableElementAttributeData::MutableElementAttributeData(const MutableElementAttributeData& other)
+ : ElementAttributeData(other, true)
+ , m_attributeVector(other.m_attributeVector)
{
- for (unsigned i = 0; i < m_arraySize; ++i)
- (reinterpret_cast<Attribute*>(&m_attributeArray)[i]).~Attribute();
+ m_inlineStyleDecl = other.m_inlineStyleDecl ? other.m_inlineStyleDecl->copy() : 0;
}
+MutableElementAttributeData::MutableElementAttributeData(const ImmutableElementAttributeData& other)
+ : ElementAttributeData(other, true)
+{
+ // An ImmutableElementAttributeData should never have a mutable inline StylePropertySet attached.
+ ASSERT(!other.m_inlineStyleDecl || !other.m_inlineStyleDecl->isMutable());
+ m_inlineStyleDecl = other.m_inlineStyleDecl;
+
+ m_attributeVector.reserveCapacity(other.length());
+ for (unsigned i = 0; i < other.length(); ++i)
+ m_attributeVector.uncheckedAppend(other.immutableAttributeArray()[i]);
+}
+
PassRefPtr<ElementAttributeData> ElementAttributeData::makeMutableCopy() const
{
- ASSERT(!isMutable());
+ if (isMutable())
+ return adoptRef(new MutableElementAttributeData(static_cast<const MutableElementAttributeData&>(*this)));
return adoptRef(new MutableElementAttributeData(static_cast<const ImmutableElementAttributeData&>(*this)));
}
+PassRefPtr<ElementAttributeData> ElementAttributeData::makeImmutableCopy() const
+{
+ ASSERT(isMutable());
+ void* slot = WTF::fastMalloc(sizeForImmutableElementAttributeDataWithAttributeCount(mutableAttributeVector().size()));
+ return adoptRef(new (slot) ImmutableElementAttributeData(static_cast<const MutableElementAttributeData&>(*this)));
+}
+
StylePropertySet* ElementAttributeData::ensureInlineStyle(StyledElement* element)
{
ASSERT(isMutable());
@@ -196,53 +228,4 @@
return notFound;
}
-void ElementAttributeData::cloneDataFrom(const ElementAttributeData& sourceData, const Element& sourceElement, Element& targetElement)
-{
- // FIXME: Cloned elements could start out with immutable attribute data.
- ASSERT(isMutable());
-
- const AtomicString& oldID = targetElement.getIdAttribute();
- const AtomicString& newID = sourceElement.getIdAttribute();
-
- if (!oldID.isNull() || !newID.isNull())
- targetElement.updateId(oldID, newID);
-
- const AtomicString& oldName = targetElement.getNameAttribute();
- const AtomicString& newName = sourceElement.getNameAttribute();
-
- if (!oldName.isNull() || !newName.isNull())
- targetElement.updateName(oldName, newName);
-
- clearAttributes();
-
- if (sourceData.isMutable())
- mutableAttributeVector() = sourceData.mutableAttributeVector();
- else {
- mutableAttributeVector().reserveInitialCapacity(sourceData.m_arraySize);
- for (unsigned i = 0; i < sourceData.m_arraySize; ++i)
- mutableAttributeVector().uncheckedAppend(sourceData.immutableAttributeArray()[i]);
- }
-
- for (unsigned i = 0; i < length(); ++i) {
- const Attribute& attribute = mutableAttributeVector().at(i);
- if (targetElement.isStyledElement() && attribute.name() == HTMLNames::styleAttr) {
- static_cast<StyledElement&>(targetElement).styleAttributeChanged(attribute.value(), StyledElement::DoNotReparseStyleAttribute);
- continue;
- }
- targetElement.attributeChanged(attribute.name(), attribute.value());
- }
-
- if (targetElement.isStyledElement() && sourceData.m_inlineStyleDecl) {
- m_inlineStyleDecl = sourceData.m_inlineStyleDecl->immutableCopyIfNeeded();
- targetElement.setIsStyleAttributeValid(sourceElement.isStyleAttributeValid());
- }
}
-
-void ElementAttributeData::clearAttributes()
-{
- ASSERT(isMutable());
- clearClass();
- mutableAttributeVector().clear();
-}
-
-}
Modified: trunk/Source/WebCore/dom/ElementAttributeData.h (134407 => 134408)
--- trunk/Source/WebCore/dom/ElementAttributeData.h 2012-11-13 14:09:23 UTC (rev 134407)
+++ trunk/Source/WebCore/dom/ElementAttributeData.h 2012-11-13 14:21:28 UTC (rev 134408)
@@ -3,7 +3,7 @@
* (C) 1999 Antti Koivisto (koivi...@kde.org)
* (C) 2001 Peter Kelly (p...@post.com)
* (C) 2001 Dirk Mueller (muel...@kde.org)
- * Copyright (C) 2003, 2004, 2005, 2006, 2008, 2010 Apple Inc. All rights reserved.
+ * Copyright (C) 2003, 2004, 2005, 2006, 2008, 2010, 2012 Apple Inc. All rights reserved.
* Copyright (C) 2012 Nokia Corporation and/or its subsidiary(-ies).
*
* This library is free software; you can redistribute it and/or
@@ -101,6 +101,8 @@
, m_arraySize(arraySize)
{ }
+ ElementAttributeData(const ElementAttributeData&, bool isMutable);
+
unsigned m_isMutable : 1;
unsigned m_arraySize : 31;
@@ -118,10 +120,9 @@
Attribute* getAttributeItem(const AtomicString& name, bool shouldIgnoreAttributeCase);
const Attribute* getAttributeItem(const AtomicString& name, bool shouldIgnoreAttributeCase) const;
size_t getAttributeItemIndexSlowCase(const AtomicString& name, bool shouldIgnoreAttributeCase) const;
- void cloneDataFrom(const ElementAttributeData& sourceData, const Element& sourceElement, Element& targetElement);
- void clearAttributes();
PassRefPtr<ElementAttributeData> makeMutableCopy() const;
+ PassRefPtr<ElementAttributeData> makeImmutableCopy() const;
Vector<Attribute, 4>& mutableAttributeVector();
const Vector<Attribute, 4>& mutableAttributeVector() const;
@@ -130,6 +131,7 @@
class ImmutableElementAttributeData : public ElementAttributeData {
public:
ImmutableElementAttributeData(const Vector<Attribute>&);
+ ImmutableElementAttributeData(const MutableElementAttributeData&);
~ImmutableElementAttributeData();
void* m_attributeArray;
@@ -139,6 +141,7 @@
public:
MutableElementAttributeData() { }
MutableElementAttributeData(const ImmutableElementAttributeData&);
+ MutableElementAttributeData(const MutableElementAttributeData&);
Vector<Attribute, 4> m_attributeVector;
};
