Title: [117353] trunk/Source/WebCore
- Revision
- 117353
- Author
- o...@chromium.org
- Date
- 2012-05-16 15:25:10 -0700 (Wed, 16 May 2012)
Log Message
Fix perf regression from r116487
https://bugs.webkit.org/show_bug.cgi?id=86680
Reviewed by Ryosuke Niwa.
http://trac.webkit.org/changeset/116487 caused a 6% regression on
Dromaeo's dom-attr test. The issue is that we invalidated NodeList
caches whenever an id/checked/type attribute changed.
First, we don't need to invalidate on checked/type since that only
affects the values return by NodeList items, not the list of items.
Second, we only need to invalidate NodeList caches when an id attribute
changes on a FormControlElement.
Incidentally, we also don't need to invalidate caches for changes
to attributes that don't have an ownerElement.
No new tests. This is strictly a performance improvement.
* dom/Attr.cpp:
(WebCore::Attr::setValue):
(WebCore::Attr::childrenChanged):
* dom/Element.cpp:
(WebCore::Element::attributeChanged):
* dom/Node.cpp:
(WebCore::Node::invalidateNodeListsCacheAfterAttributeChanged):
* dom/Node.h:
(Node):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (117352 => 117353)
--- trunk/Source/WebCore/ChangeLog 2012-05-16 22:16:13 UTC (rev 117352)
+++ trunk/Source/WebCore/ChangeLog 2012-05-16 22:25:10 UTC (rev 117353)
@@ -1,3 +1,34 @@
+2012-05-16 Ojan Vafai <o...@chromium.org>
+
+ Fix perf regression from r116487
+ https://bugs.webkit.org/show_bug.cgi?id=86680
+
+ Reviewed by Ryosuke Niwa.
+
+ http://trac.webkit.org/changeset/116487 caused a 6% regression on
+ Dromaeo's dom-attr test. The issue is that we invalidated NodeList
+ caches whenever an id/checked/type attribute changed.
+
+ First, we don't need to invalidate on checked/type since that only
+ affects the values return by NodeList items, not the list of items.
+ Second, we only need to invalidate NodeList caches when an id attribute
+ changes on a FormControlElement.
+
+ Incidentally, we also don't need to invalidate caches for changes
+ to attributes that don't have an ownerElement.
+
+ No new tests. This is strictly a performance improvement.
+
+ * dom/Attr.cpp:
+ (WebCore::Attr::setValue):
+ (WebCore::Attr::childrenChanged):
+ * dom/Element.cpp:
+ (WebCore::Element::attributeChanged):
+ * dom/Node.cpp:
+ (WebCore::Node::invalidateNodeListsCacheAfterAttributeChanged):
+ * dom/Node.h:
+ (Node):
+
2012-04-22 Robert Hogan <rob...@webkit.org>
CSS 2.1 failure: inline-table-001 fails
Modified: trunk/Source/WebCore/dom/Attr.cpp (117352 => 117353)
--- trunk/Source/WebCore/dom/Attr.cpp 2012-05-16 22:16:13 UTC (rev 117352)
+++ trunk/Source/WebCore/dom/Attr.cpp 2012-05-16 22:25:10 UTC (rev 117353)
@@ -119,7 +119,7 @@
createTextChild();
m_ignoreChildrenChanged--;
- invalidateNodeListsCacheAfterAttributeChanged(m_name);
+ invalidateNodeListsCacheAfterAttributeChanged(m_name, m_element);
}
void Attr::setValue(const AtomicString& value, ExceptionCode&)
@@ -162,7 +162,7 @@
if (m_ignoreChildrenChanged > 0)
return;
- invalidateNodeListsCacheAfterAttributeChanged(qualifiedName());
+ invalidateNodeListsCacheAfterAttributeChanged(qualifiedName(), m_element);
// FIXME: We should include entity references in the value
Modified: trunk/Source/WebCore/dom/Element.cpp (117352 => 117353)
--- trunk/Source/WebCore/dom/Element.cpp 2012-05-16 22:16:13 UTC (rev 117352)
+++ trunk/Source/WebCore/dom/Element.cpp 2012-05-16 22:25:10 UTC (rev 117353)
@@ -716,7 +716,7 @@
setNeedsStyleRecalc();
}
- invalidateNodeListsCacheAfterAttributeChanged(attribute.name());
+ invalidateNodeListsCacheAfterAttributeChanged(attribute.name(), this);
if (!AXObjectCache::accessibilityEnabled())
return;
Modified: trunk/Source/WebCore/dom/Node.cpp (117352 => 117353)
--- trunk/Source/WebCore/dom/Node.cpp 2012-05-16 22:16:13 UTC (rev 117352)
+++ trunk/Source/WebCore/dom/Node.cpp 2012-05-16 22:25:10 UTC (rev 117353)
@@ -976,7 +976,7 @@
removeNodeListCacheIfPossible(this, data);
}
-void Node::invalidateNodeListsCacheAfterAttributeChanged(const QualifiedName& attrName)
+void Node::invalidateNodeListsCacheAfterAttributeChanged(const QualifiedName& attrName, Element* attributeOwnerElement)
{
if (hasRareData() && isAttributeNode()) {
NodeRareData* data = ""
@@ -984,18 +984,25 @@
data->clearChildNodeListCache();
}
- // This list should be sync'ed with NodeListsNodeData.
+ // Modifications to attributes that are not associated with an Element can't invalidate NodeList caches.
+ if (!attributeOwnerElement)
+ return;
+
+ // FIXME: Move the list of attributes each NodeList type cares about to be a static on the
+ // appropriate NodeList class. Then use those lists here and in invalidateCachesThatDependOnAttributes
+ // to only invalidate the cache types that depend on the attribute that changed.
+ // FIXME: Keep track of when we have no caches of a given type so that we can avoid the for-loop
+ // below even if a related attribute changed (e.g. if we have no RadioNodeLists, we don't need
+ // to invalidate any caches when id attributes change.)
if (attrName != classAttr
#if ENABLE(MICRODATA)
&& attrName != itemscopeAttr
&& attrName != itempropAttr
&& attrName != itemtypeAttr
#endif
- && attrName != idAttr
- && attrName != typeAttr
- && attrName != checkedAttr
&& attrName != nameAttr
- && attrName != forAttr)
+ && attrName != forAttr
+ && (attrName != idAttr || !attributeOwnerElement->isFormControlElement()))
return;
if (!treeScope()->hasNodeListCaches())
Modified: trunk/Source/WebCore/dom/Node.h (117352 => 117353)
--- trunk/Source/WebCore/dom/Node.h 2012-05-16 22:16:13 UTC (rev 117352)
+++ trunk/Source/WebCore/dom/Node.h 2012-05-16 22:25:10 UTC (rev 117353)
@@ -557,7 +557,7 @@
void registerDynamicSubtreeNodeList(DynamicSubtreeNodeList*);
void unregisterDynamicSubtreeNodeList(DynamicSubtreeNodeList*);
- void invalidateNodeListsCacheAfterAttributeChanged(const QualifiedName&);
+ void invalidateNodeListsCacheAfterAttributeChanged(const QualifiedName&, Element* attributeOwnerElement);
void invalidateNodeListsCacheAfterChildrenChanged();
void removeCachedClassNodeList(ClassNodeList*, const String&);
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes
