- Revision
- 164707
- Author
- rn...@webkit.org
- Date
- 2014-02-26 00:26:05 -0800 (Wed, 26 Feb 2014)
Log Message
Avoid unnecessary HTML Collection invalidations for id and name attribute changes
https://bugs.webkit.org/show_bug.cgi?id=129361
Reviewed by Benjamin Poulain.
Before this patch, setting id and name attributes resulted in traversing all the ancestors to invalidate
HTML collections on those nodes whenever we had more than one HTMLCollection alive.
Avoid the traversal when HTMLCollections don't have any valid id and name map caches by making each
HTMLCollection explicitly call collectionCachedIdNameMap and collectionWillClearIdNameMap when it caches
or clears the id and name map.
Inspired by https://chromium.googlesource.com/chromium/blink/+/5b06b91b79098f7d42e480f85be32198315d2440
* dom/Document.cpp:
(WebCore::Document::registerCollection): Takes a boolean to indicate whether collection has a valid cache
for the id and name map.
(WebCore::Document::unregisterCollection): Ditto.
(WebCore::Document::collectionCachedIdNameMap): Added.
(WebCore::Document::collectionWillClearIdNameMap): Added.
* dom/Document.h:
* dom/NodeRareData.h:
(WebCore::NodeListsNodeData::adoptDocument): Call invalidateCache on HTML collections after, not before,
calling unregisterCollection and registerCollection since collections' owner nodes have already been
moved to the new document here and invalidateCache uses owner node's document to call
collectionWillClearIdNameMap. So calling invalidateCache before calling unregister/registerCollection
would result in collectionWillClearIdNameMap getting called on a wrong document.
* html/HTMLCollection.cpp:
(WebCore::HTMLCollection::HTMLCollection):
(WebCore::HTMLCollection::~HTMLCollection):
(WebCore::HTMLCollection::invalidateCache):
(WebCore::HTMLCollection::invalidateIdNameCacheMaps): Added the code to uncount itself from the number
of live node lists and HTML collections that need to be invalidated upon id and name attribute changes.
(WebCore::HTMLCollection::updateNameCache):
* html/HTMLCollection.h:
(WebCore::HTMLCollection::invalidateCache):
(WebCore::HTMLCollection::hasIdNameCache): Renamed from hasNameCache.
(WebCore::HTMLCollection::setHasIdNameCache): Renamed from setHasIdNameCache.
* html/HTMLFormControlsCollection.cpp:
(WebCore::HTMLFormControlsCollection::updateNameCache):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (164706 => 164707)
--- trunk/Source/WebCore/ChangeLog 2014-02-26 08:23:22 UTC (rev 164706)
+++ trunk/Source/WebCore/ChangeLog 2014-02-26 08:26:05 UTC (rev 164707)
@@ -1,3 +1,50 @@
+2014-02-26 Ryosuke Niwa <rn...@webkit.org>
+
+ Avoid unnecessary HTML Collection invalidations for id and name attribute changes
+ https://bugs.webkit.org/show_bug.cgi?id=129361
+
+ Reviewed by Benjamin Poulain.
+
+ Before this patch, setting id and name attributes resulted in traversing all the ancestors to invalidate
+ HTML collections on those nodes whenever we had more than one HTMLCollection alive.
+
+ Avoid the traversal when HTMLCollections don't have any valid id and name map caches by making each
+ HTMLCollection explicitly call collectionCachedIdNameMap and collectionWillClearIdNameMap when it caches
+ or clears the id and name map.
+
+ Inspired by https://chromium.googlesource.com/chromium/blink/+/5b06b91b79098f7d42e480f85be32198315d2440
+
+ * dom/Document.cpp:
+ (WebCore::Document::registerCollection): Takes a boolean to indicate whether collection has a valid cache
+ for the id and name map.
+ (WebCore::Document::unregisterCollection): Ditto.
+ (WebCore::Document::collectionCachedIdNameMap): Added.
+ (WebCore::Document::collectionWillClearIdNameMap): Added.
+ * dom/Document.h:
+
+ * dom/NodeRareData.h:
+ (WebCore::NodeListsNodeData::adoptDocument): Call invalidateCache on HTML collections after, not before,
+ calling unregisterCollection and registerCollection since collections' owner nodes have already been
+ moved to the new document here and invalidateCache uses owner node's document to call
+ collectionWillClearIdNameMap. So calling invalidateCache before calling unregister/registerCollection
+ would result in collectionWillClearIdNameMap getting called on a wrong document.
+
+ * html/HTMLCollection.cpp:
+ (WebCore::HTMLCollection::HTMLCollection):
+ (WebCore::HTMLCollection::~HTMLCollection):
+ (WebCore::HTMLCollection::invalidateCache):
+ (WebCore::HTMLCollection::invalidateIdNameCacheMaps): Added the code to uncount itself from the number
+ of live node lists and HTML collections that need to be invalidated upon id and name attribute changes.
+ (WebCore::HTMLCollection::updateNameCache):
+
+ * html/HTMLCollection.h:
+ (WebCore::HTMLCollection::invalidateCache):
+ (WebCore::HTMLCollection::hasIdNameCache): Renamed from hasNameCache.
+ (WebCore::HTMLCollection::setHasIdNameCache): Renamed from setHasIdNameCache.
+
+ * html/HTMLFormControlsCollection.cpp:
+ (WebCore::HTMLFormControlsCollection::updateNameCache):
+
2014-02-25 Frédéric Wang <fred.w...@free.fr>
Add support for minsize/maxsize attributes.
Modified: trunk/Source/WebCore/dom/Document.cpp (164706 => 164707)
--- trunk/Source/WebCore/dom/Document.cpp 2014-02-26 08:23:22 UTC (rev 164706)
+++ trunk/Source/WebCore/dom/Document.cpp 2014-02-26 08:26:05 UTC (rev 164707)
@@ -3450,17 +3450,20 @@
}
}
-void Document::registerCollection(HTMLCollection& collection)
+void Document::registerCollection(HTMLCollection& collection, bool hasIdNameMap)
{
- m_nodeListAndCollectionCounts[InvalidateOnIdNameAttrChange]++;
+ if (hasIdNameMap)
+ collectionCachedIdNameMap(collection);
m_nodeListAndCollectionCounts[collection.invalidationType()]++;
if (collection.isRootedAtDocument())
m_collectionsInvalidatedAtDocument.add(&collection);
}
-void Document::unregisterCollection(HTMLCollection& collection)
+void Document::unregisterCollection(HTMLCollection& collection, bool hasIdNameMap)
{
- m_nodeListAndCollectionCounts[InvalidateOnIdNameAttrChange]--;
+ if (hasIdNameMap)
+ collectionWillClearIdNameMap(collection);
+ ASSERT(m_nodeListAndCollectionCounts[collection.invalidationType()]);
m_nodeListAndCollectionCounts[collection.invalidationType()]--;
if (collection.isRootedAtDocument()) {
ASSERT(m_collectionsInvalidatedAtDocument.contains(&collection));
@@ -3468,6 +3471,19 @@
}
}
+void Document::collectionCachedIdNameMap(const HTMLCollection& collection)
+{
+ ASSERT_UNUSED(collection, collection.hasIdNameCache());
+ m_nodeListAndCollectionCounts[InvalidateOnIdNameAttrChange]++;
+}
+
+void Document::collectionWillClearIdNameMap(const HTMLCollection& collection)
+{
+ ASSERT_UNUSED(collection, collection.hasIdNameCache());
+ ASSERT(m_nodeListAndCollectionCounts[InvalidateOnIdNameAttrChange]);
+ m_nodeListAndCollectionCounts[InvalidateOnIdNameAttrChange]--;
+}
+
void Document::attachNodeIterator(NodeIterator* ni)
{
m_nodeIterators.add(ni);
Modified: trunk/Source/WebCore/dom/Document.h (164706 => 164707)
--- trunk/Source/WebCore/dom/Document.h 2014-02-26 08:23:22 UTC (rev 164706)
+++ trunk/Source/WebCore/dom/Document.h 2014-02-26 08:26:05 UTC (rev 164707)
@@ -758,8 +758,10 @@
void registerNodeList(LiveNodeList&);
void unregisterNodeList(LiveNodeList&);
- void registerCollection(HTMLCollection&);
- void unregisterCollection(HTMLCollection&);
+ void registerCollection(HTMLCollection&, bool hasIdNameCache);
+ void unregisterCollection(HTMLCollection&, bool hasIdNameCache);
+ void collectionCachedIdNameMap(const HTMLCollection&);
+ void collectionWillClearIdNameMap(const HTMLCollection&);
bool shouldInvalidateNodeListAndCollectionCaches(const QualifiedName* attrName = nullptr) const;
void invalidateNodeListAndCollectionCaches(const QualifiedName* attrName);
Modified: trunk/Source/WebCore/dom/NodeRareData.h (164706 => 164707)
--- trunk/Source/WebCore/dom/NodeRareData.h 2014-02-26 08:23:22 UTC (rev 164706)
+++ trunk/Source/WebCore/dom/NodeRareData.h 2014-02-26 08:26:05 UTC (rev 164707)
@@ -225,33 +225,38 @@
void adoptDocument(Document* oldDocument, Document* newDocument)
{
- invalidateCaches();
+ if (oldDocument == newDocument) {
+ invalidateCaches();
+ return;
+ }
- if (oldDocument != newDocument) {
- for (auto it = m_atomicNameCaches.begin(), end = m_atomicNameCaches.end(); it != end; ++it) {
- LiveNodeList& list = *it->value;
- oldDocument->unregisterNodeList(list);
- newDocument->registerNodeList(list);
- }
+ for (auto it : m_atomicNameCaches) {
+ LiveNodeList& list = *it.value;
+ oldDocument->unregisterNodeList(list);
+ newDocument->registerNodeList(list);
+ list.invalidateCache();
+ }
- for (auto it = m_nameCaches.begin(), end = m_nameCaches.end(); it != end; ++it) {
- LiveNodeList& list = *it->value;
- oldDocument->unregisterNodeList(list);
- newDocument->registerNodeList(list);
- }
+ for (auto it : m_nameCaches) {
+ LiveNodeList& list = *it.value;
+ oldDocument->unregisterNodeList(list);
+ newDocument->registerNodeList(list);
+ list.invalidateCache();
+ }
- for (auto it = m_tagNodeListCacheNS.begin(), end = m_tagNodeListCacheNS.end(); it != end; ++it) {
- LiveNodeList& list = *it->value;
- ASSERT(!list.isRootedAtDocument());
- oldDocument->unregisterNodeList(list);
- newDocument->registerNodeList(list);
- }
+ for (auto it : m_tagNodeListCacheNS) {
+ LiveNodeList& list = *it.value;
+ ASSERT(!list.isRootedAtDocument());
+ oldDocument->unregisterNodeList(list);
+ newDocument->registerNodeList(list);
+ list.invalidateCache();
+ }
- for (auto it = m_cachedCollections.begin(), end = m_cachedCollections.end(); it != end; ++it) {
- HTMLCollection& collection = *it->value;
- oldDocument->unregisterCollection(collection);
- newDocument->registerCollection(collection);
- }
+ for (auto it : m_cachedCollections) {
+ HTMLCollection& collection = *it.value;
+ oldDocument->unregisterCollection(collection, collection.hasIdNameCache());
+ newDocument->registerCollection(collection, collection.hasIdNameCache());
+ collection.invalidateCache();
}
}
Modified: trunk/Source/WebCore/html/HTMLCollection.cpp (164706 => 164707)
--- trunk/Source/WebCore/html/HTMLCollection.cpp 2014-02-26 08:23:22 UTC (rev 164706)
+++ trunk/Source/WebCore/html/HTMLCollection.cpp 2014-02-26 08:26:05 UTC (rev 164707)
@@ -145,7 +145,7 @@
ASSERT(m_invalidationType == static_cast<unsigned>(invalidationTypeExcludingIdAndNameAttributes(type)));
ASSERT(m_collectionType == static_cast<unsigned>(type));
- document().registerCollection(*this);
+ document().registerCollection(*this, hasIdNameCache());
}
PassRefPtr<HTMLCollection> HTMLCollection::create(ContainerNode& base, CollectionType type)
@@ -155,7 +155,7 @@
HTMLCollection::~HTMLCollection()
{
- document().unregisterCollection(*this);
+ document().unregisterCollection(*this, hasIdNameCache());
// HTMLNameCollection removes cache by itself.
if (type() != WindowNamedItems && type() != DocumentNamedItems)
ownerNode().nodeLists()->removeCachedCollection(this);
@@ -367,14 +367,16 @@
void HTMLCollection::invalidateCache() const
{
m_indexCache.invalidate();
- m_isNameCacheValid = false;
m_isItemRefElementsCacheValid = false;
- m_idCache.clear();
- m_nameCache.clear();
+ if (hasIdNameCache())
+ invalidateIdNameCacheMaps();
}
void HTMLCollection::invalidateIdNameCacheMaps() const
{
+ ASSERT(hasIdNameCache());
+ document().collectionWillClearIdNameMap(*this);
+ m_isNameCacheValid = false;
m_idCache.clear();
m_nameCache.clear();
}
@@ -429,7 +431,7 @@
void HTMLCollection::updateNameCache() const
{
- if (hasNameCache())
+ if (hasIdNameCache())
return;
ContainerNode& root = rootNode();
@@ -446,7 +448,7 @@
appendNameCache(nameAttrVal, element);
}
- setHasNameCache();
+ setHasIdNameCache();
}
bool HTMLCollection::hasNamedItem(const AtomicString& name) const
Modified: trunk/Source/WebCore/html/HTMLCollection.h (164706 => 164707)
--- trunk/Source/WebCore/html/HTMLCollection.h 2014-02-26 08:23:22 UTC (rev 164706)
+++ trunk/Source/WebCore/html/HTMLCollection.h 2014-02-26 08:26:05 UTC (rev 164707)
@@ -61,11 +61,10 @@
{
if (!attrName || shouldInvalidateTypeOnAttributeChange(invalidationType(), *attrName))
invalidateCache();
- else if (*attrName == HTMLNames::idAttr || *attrName == HTMLNames::nameAttr)
+ else if (hasIdNameCache() && (*attrName == HTMLNames::idAttr || *attrName == HTMLNames::nameAttr))
invalidateIdNameCacheMaps();
}
virtual void invalidateCache() const;
- void invalidateIdNameCacheMaps() const;
// For CollectionIndexCache
Element* collectionFirst() const;
@@ -74,10 +73,13 @@
Element* collectionTraverseBackward(Element&, unsigned count) const;
bool collectionCanTraverseBackward() const { return !m_usesCustomForwardOnlyTraversal; }
+ bool hasIdNameCache() const { return m_isNameCacheValid; }
+
protected:
enum ElementTraversalType { NormalTraversal, CustomForwardOnlyTraversal };
HTMLCollection(ContainerNode& base, CollectionType, ElementTraversalType = NormalTraversal);
+ void invalidateIdNameCacheMaps() const;
virtual void updateNameCache() const;
typedef HashMap<AtomicStringImpl*, OwnPtr<Vector<Element*>>> NodeCacheMap;
@@ -95,8 +97,11 @@
NodeListRootType rootType() const { return static_cast<NodeListRootType>(m_rootType); }
- bool hasNameCache() const { return m_isNameCacheValid; }
- void setHasNameCache() const { m_isNameCacheValid = true; }
+ void setHasIdNameCache() const
+ {
+ m_isNameCacheValid = true;
+ document().collectionCachedIdNameMap(*this);
+ }
private:
static void append(NodeCacheMap&, const AtomicString&, Element*);
Modified: trunk/Source/WebCore/html/HTMLFormControlsCollection.cpp (164706 => 164707)
--- trunk/Source/WebCore/html/HTMLFormControlsCollection.cpp 2014-02-26 08:23:22 UTC (rev 164706)
+++ trunk/Source/WebCore/html/HTMLFormControlsCollection.cpp 2014-02-26 08:26:05 UTC (rev 164707)
@@ -137,7 +137,7 @@
void HTMLFormControlsCollection::updateNameCache() const
{
- if (hasNameCache())
+ if (hasIdNameCache())
return;
HashSet<AtomicStringImpl*> foundInputElements;
@@ -174,7 +174,7 @@
}
}
- setHasNameCache();
+ setHasIdNameCache();
}
void HTMLFormControlsCollection::invalidateCache() const
