Changeset 121412 in webkit

Timestamp:

Jun 27, 2012 11:29:27 PM (12 years ago)

Author:

haraken@chromium.org

Message:

Performance: Optimize Dromaeo/dom-query.html by caching NodeRareData on Document
​https://bugs.webkit.org/show_bug.cgi?id=90059

Reviewed by Ryosuke Niwa.

This patch improves performance of document.getElementsBy*().
e.g. the patch makes Dromaeo/dom-query.html 5.4% faster.

Dromaeo/dom-query.html without the patch (Chromium/Linux):
784714 runs/s, 765947 runs/s, 803109 runs/s, 804450 runs/s

Dromaeo/dom-query.html with the patch (Chromium/Linux):
839245 runs/s, 829867 runs/s, 811032 runs/s, 847486 runs/s

Based on the assumption that document.getElementsByClassName(),
document.getElementsByTagName() and document.getElementsByName()
would be used frequently in the real world, this patch implements
a fast path for Document methods that require to access NodeRareData.
Specifically, this patch caches a pointer to NodeRareData on Document,
by which Document can access NodeRareData without looking up a HashMap.

The only performance concern is the overhead of the isDocumentNode() check
that this patch added to Node::ensureRareData. However, I could not
observe any performance regression caused by the overhead.

No tests. No change in behavior.

• dom/Document.cpp:

(WebCore::Document::Document):
(WebCore::Document::setCachedRareData): I didn't inline this method,
since the inlining slightly regressed performance for some reason.
(WebCore):

• dom/Document.h:

(WebCore):
(WebCore::Document::cachedRareData):
(Document):
(~Document): Moved 'm_document = 0' to the tail of the destructor,
since isDocumentNode() has to return true in clearRareData() that is called
in ~Document().

• dom/Node.cpp:

(WebCore::Node::ensureRareData):
(~Node): Moved the assertion into clearRareData().

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• dom/Document.cpp (modified) (4 diffs)
• dom/Document.h (modified) (3 diffs)
• dom/Node.cpp (modified) (4 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2012-06-27  Kentaro Hara  <haraken@chromium.org>
+
+        Performance: Optimize Dromaeo/dom-query.html by caching NodeRareData on Document
+        https://bugs.webkit.org/show_bug.cgi?id=90059
+
+        Reviewed by Ryosuke Niwa.
+
+        This patch improves performance of document.getElementsBy*().
+        e.g. the patch makes Dromaeo/dom-query.html 5.4% faster.
+
+        Dromaeo/dom-query.html without the patch (Chromium/Linux):
+        784714 runs/s, 765947 runs/s, 803109 runs/s, 804450 runs/s
+
+        Dromaeo/dom-query.html with the patch (Chromium/Linux):
+        839245 runs/s, 829867 runs/s, 811032 runs/s, 847486 runs/s
+
+        Based on the assumption that document.getElementsByClassName(),
+        document.getElementsByTagName() and document.getElementsByName()
+        would be used frequently in the real world, this patch implements
+        a fast path for Document methods that require to access NodeRareData.
+        Specifically, this patch caches a pointer to NodeRareData on Document,
+        by which Document can access NodeRareData without looking up a HashMap.
+
+        The only performance concern is the overhead of the isDocumentNode() check
+        that this patch added to Node::ensureRareData. However, I could not
+        observe any performance regression caused by the overhead.
+
+        No tests. No change in behavior.
+
+        * dom/Document.cpp:
+        (WebCore::Document::Document):
+        (WebCore::Document::setCachedRareData): I didn't inline this method,
+        since the inlining slightly regressed performance for some reason.
+        (WebCore):
+        * dom/Document.h:
+        (WebCore):
+        (WebCore::Document::cachedRareData):
+        (Document):
+        (~Document): Moved 'm_document = 0' to the tail of the destructor,
+        since isDocumentNode() has to return true in clearRareData() that is called
+        in ~Document().
+        * dom/Node.cpp:
+        (WebCore::Node::ensureRareData):
+        (~Node): Moved the assertion into clearRareData().
+
 2012-06-27  Mary Wu  <mary.wu@torchmobile.com.cn>
 

trunk/Source/WebCore/dom/Document.cpp

@@ -471 +471 @@
     , m_sawElementsInKnownNamespaces(false)
     , m_isSrcdocDocument(false)
+    , m_documentRareData(0)
     , m_eventQueue(DocumentEventQueue::create(this))
     , m_weakReference(DocumentWeakReference::create(this))
@@ -607 +608 @@
     ASSERT(!m_parser || m_parser->refCount() == 1);
     detachParser();
-    m_document = 0;
 
     m_renderArena.clear();
@@ -650 +650 @@
     if (hasRareData())
         clearRareData();
+
+    m_document = 0;
 
     InspectorCounters::decrementCounter(InspectorCounters::DocumentCounter);
@@ -2008 +2010 @@
     marginBottom = style->marginBottom().isAuto() ? marginBottom : intValueForLength(style->marginBottom(), width, view);
     marginLeft = style->marginLeft().isAuto() ? marginLeft : intValueForLength(style->marginLeft(), width, view);
+}
+
+void Document::setDocumentRareData(NodeRareData* rareData)
+{
+    m_documentRareData = rareData;
 }
 

trunk/Source/WebCore/dom/Document.h

@@ -112 +112 @@
 class NodeFilter;
 class NodeIterator;
+class NodeRareData;
 class Page;
 class PlatformMouseEvent;
@@ -433 +434 @@
     bool isSrcdocDocument() const { return m_isSrcdocDocument; }
 
+    NodeRareData* documentRareData() const { return m_documentRareData; };
+    void setDocumentRareData(NodeRareData*);
+
     StyleResolver* styleResolverIfExists() const { return m_styleResolver.get(); }
 
@@ -1436 +1440 @@
     bool m_isSrcdocDocument;
 
+    NodeRareData* m_documentRareData;
+
     RefPtr<DocumentEventQueue> m_eventQueue;
 

trunk/Source/WebCore/dom/Node.cpp

@@ -400 +400 @@
 #endif
 
-    ASSERT(hasRareData() == NodeRareData::rareDataMap().contains(this));
     if (hasRareData())
         clearRareData();
@@ -460 +459 @@
 {
     ASSERT(hasRareData());
-    return NodeRareData::rareDataFromMap(this);
+    NodeRareData* data = isDocumentNode() ? static_cast<const Document*>(this)->documentRareData() : NodeRareData::rareDataFromMap(this);
+    ASSERT(data);
+    return data;
 }
 
@@ -467 +468 @@
     if (hasRareData())
         return rareData();
-    
-    ASSERT(!NodeRareData::rareDataMap().contains(this));
+
     NodeRareData* data = createRareData().leakPtr();
-    NodeRareData::rareDataMap().set(this, data);
+    if (isDocumentNode()) {
+        // Fast path for a Document. A Document knows a pointer to NodeRareData.
+        ASSERT(!static_cast<Document*>(this)->documentRareData());
+        static_cast<Document*>(this)->setDocumentRareData(data);
+    } else {
+        ASSERT(!NodeRareData::rareDataMap().contains(this));
+        NodeRareData::rareDataMap().set(this, data);
+    }
     setFlag(HasRareDataFlag);
     return data;
@@ -490 +497 @@
 #endif
 
-    NodeRareData::NodeRareDataMap& dataMap = NodeRareData::rareDataMap();
-    NodeRareData::NodeRareDataMap::iterator it = dataMap.find(this);
-    ASSERT(it != dataMap.end());
-    delete it->second;
-    dataMap.remove(it);
+    if (isDocumentNode()) {
+        Document* document = static_cast<Document*>(this);
+        NodeRareData* data = document->documentRareData();
+        ASSERT(data);
+        delete data;
+        document->setDocumentRareData(0);
+    } else {
+        NodeRareData::NodeRareDataMap& dataMap = NodeRareData::rareDataMap();
+        NodeRareData::NodeRareDataMap::iterator it = dataMap.find(this);
+        ASSERT(it != dataMap.end());
+        delete it->second;
+        dataMap.remove(it);
+    }
     clearFlag(HasRareDataFlag);
 }