2008-05-01 Maciej Stachowiak <mjs@apple.com>
authormjs@apple.com <mjs@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 2 May 2008 01:02:59 +0000 (01:02 +0000)
committermjs@apple.com <mjs@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 2 May 2008 01:02:59 +0000 (01:02 +0000)
        Reviewed by Oliver (a while ago)

        - just a wee bit more bindings speedup

        Store the per-document Node --> JS wrapper cache in the document
        instead of an external hashtable.

        * bindings/js/kjs_binding.cpp:
        (WebCore::ScriptInterpreter::getDOMNodeForDocument):
        (WebCore::ScriptInterpreter::forgetDOMNodeForDocument):
        (WebCore::ScriptInterpreter::putDOMNodeForDocument):
        (WebCore::ScriptInterpreter::forgetAllDOMNodesForDocument):
        (WebCore::ScriptInterpreter::markDOMNodesForDocument):
        * dom/Document.h:
        (WebCore::Document::wrapperCache):

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@32788 268f45cc-cd09-0410-ab3c-d52691b4dbfc

WebCore/ChangeLog
WebCore/bindings/js/kjs_binding.cpp
WebCore/dom/Document.h

index acc678ef770060555c5009c4007055050af0511b..0d5cd3ae3fe72e9fa7a101512d7e041404e88df7 100644 (file)
@@ -1,3 +1,21 @@
+2008-05-01  Maciej Stachowiak  <mjs@apple.com>
+
+        Reviewed by Oliver (a while ago)
+
+        - just a wee bit more bindings speedup
+        
+        Store the per-document Node --> JS wrapper cache in the document
+        instead of an external hashtable.
+
+        * bindings/js/kjs_binding.cpp:
+        (WebCore::ScriptInterpreter::getDOMNodeForDocument):
+        (WebCore::ScriptInterpreter::forgetDOMNodeForDocument):
+        (WebCore::ScriptInterpreter::putDOMNodeForDocument):
+        (WebCore::ScriptInterpreter::forgetAllDOMNodesForDocument):
+        (WebCore::ScriptInterpreter::markDOMNodesForDocument):
+        * dom/Document.h:
+        (WebCore::Document::wrapperCache):
+
 2008-05-01  Anders Carlsson  <andersca@apple.com>
 
         Reviewed by Tim.
index 61a1f29cd363dc9692c37d2621932d6229d61b00..07b0c4ea6b43f331888c700d2ae15341bdf01315 100644 (file)
@@ -59,8 +59,7 @@ namespace WebCore {
 using namespace HTMLNames;
 
 typedef HashMap<void*, DOMObject*> DOMObjectMap;
-typedef HashMap<WebCore::Node*, JSNode*> NodeMap;
-typedef HashMap<Document*, NodeMap*> NodePerDocMap;
+typedef Document::JSWrapperCache JSWrapperCache;
 
 // For debugging, keep a set of wrappers currently registered, and check that
 // all are unregistered before they are destroyed. This has helped us fix at
@@ -68,7 +67,7 @@ typedef HashMap<Document*, NodeMap*> NodePerDocMap;
 
 static void addWrapper(DOMObject* wrapper);
 static void removeWrapper(DOMObject* wrapper);
-static void removeWrappers(const NodeMap& wrappers);
+static void removeWrappers(const JSWrapperCache& wrappers);
 
 #ifdef NDEBUG
 
@@ -80,7 +79,7 @@ static inline void removeWrapper(DOMObject*)
 {
 }
 
-static inline void removeWrappers(const NodeMap&)
+static inline void removeWrappers(const JSWrapperCache&)
 {
 }
 
@@ -106,9 +105,9 @@ static void removeWrapper(DOMObject* wrapper)
     wrapperSet().remove(wrapper);
 }
 
-static void removeWrappers(const NodeMap& wrappers)
+static void removeWrappers(const JSWrapperCache& wrappers)
 {
-    for (NodeMap::const_iterator it = wrappers.begin(); it != wrappers.end(); ++it)
+    for (JSWrapperCache::const_iterator it = wrappers.begin(); it != wrappers.end(); ++it)
         removeWrapper(it->second);
 }
 
@@ -126,18 +125,6 @@ static DOMObjectMap& domObjects()
     return staticDOMObjects;
 }
 
-static NodePerDocMap& domNodesPerDocument()
-{
-    // domNodesPerDocument() callers must synchronize using the JSLock because 
-    // domNodesPerDocument() is called from a mark function, which can run
-    // on a secondary thread.
-    ASSERT(JSLock::lockCount());
-
-    // Don't use malloc here. Calling malloc from a mark function can deadlock.
-    static NodePerDocMap staticDOMNodesPerDocument;
-    return staticDOMNodesPerDocument;
-}
-
 DOMObject* ScriptInterpreter::getDOMObject(void* objectHandle) 
 {
     return domObjects().get(objectHandle);
@@ -158,10 +145,7 @@ JSNode* ScriptInterpreter::getDOMNodeForDocument(Document* document, WebCore::No
 {
     if (!document)
         return static_cast<JSNode*>(domObjects().get(node));
-    NodeMap* documentDict = domNodesPerDocument().get(document);
-    if (documentDict)
-        return documentDict->get(node);
-    return NULL;
+    return document->wrapperCache().get(node);
 }
 
 void ScriptInterpreter::forgetDOMNodeForDocument(Document* document, WebCore::Node* node)
@@ -170,9 +154,7 @@ void ScriptInterpreter::forgetDOMNodeForDocument(Document* document, WebCore::No
         removeWrapper(domObjects().take(node));
         return;
     }
-    NodeMap* documentDict = domNodesPerDocument().get(document);
-    if (documentDict)
-        removeWrapper(documentDict->take(node));
+    removeWrapper(document->wrapperCache().take(node));
 }
 
 void ScriptInterpreter::putDOMNodeForDocument(Document* document, WebCore::Node* node, JSNode* wrapper)
@@ -182,43 +164,31 @@ void ScriptInterpreter::putDOMNodeForDocument(Document* document, WebCore::Node*
         domObjects().set(node, wrapper);
         return;
     }
-    NodeMap* documentDict = domNodesPerDocument().get(document);
-    if (!documentDict) {
-        documentDict = new NodeMap;
-        domNodesPerDocument().set(document, documentDict);
-    }
-    documentDict->set(node, wrapper);
+    document->wrapperCache().set(node, wrapper);
 }
 
 void ScriptInterpreter::forgetAllDOMNodesForDocument(Document* document)
 {
     ASSERT(document);
-    NodeMap* map = domNodesPerDocument().take(document);
-    if (!map)
-        return;
-    removeWrappers(*map);
-    delete map;
+    removeWrappers(document->wrapperCache());
 }
 
 void ScriptInterpreter::markDOMNodesForDocument(Document* doc)
 {
-    NodePerDocMap::iterator dictIt = domNodesPerDocument().find(doc);
-    if (dictIt != domNodesPerDocument().end()) {
-        NodeMap* nodeDict = dictIt->second;
-        NodeMap::iterator nodeEnd = nodeDict->end();
-        for (NodeMap::iterator nodeIt = nodeDict->begin(); nodeIt != nodeEnd; ++nodeIt) {
-            JSNode* jsNode = nodeIt->second;
-            WebCore::Node* node = jsNode->impl();
-            
-            // don't mark wrappers for nodes that are no longer in the
-            // document - they should not be saved if the node is not
-            // otherwise reachable from JS.
-            // However, image elements that aren't in the document are also
-            // marked, if they are not done loading yet.
-            if (!jsNode->marked() && (node->inDocument() || (node->hasTagName(imgTag) &&
-                                                             !static_cast<HTMLImageElement*>(node)->haveFiredLoadEvent())))
-                jsNode->mark();
-        }
+    JSWrapperCache& nodeDict = doc->wrapperCache();
+    JSWrapperCache::iterator nodeEnd = nodeDict.end();
+    for (JSWrapperCache::iterator nodeIt = nodeDict.begin(); nodeIt != nodeEnd; ++nodeIt) {
+        JSNode* jsNode = nodeIt->second;
+        WebCore::Node* node = jsNode->impl();
+        
+        // don't mark wrappers for nodes that are no longer in the
+        // document - they should not be saved if the node is not
+        // otherwise reachable from JS.
+        // However, image elements that aren't in the document are also
+        // marked, if they are not done loading yet.
+        if (!jsNode->marked() && (node->inDocument() || (node->hasTagName(imgTag) &&
+                                                         !static_cast<HTMLImageElement*>(node)->haveFiredLoadEvent())))
+            jsNode->mark();
     }
 }
 
index d9a7198398bede19f7320db2e1e49d6693a6073d..eb4549b0e2cc80f727511094dbe609878a03ce33 100644 (file)
@@ -85,6 +85,7 @@ namespace WebCore {
     class HTMLInputElement;
     class HTMLMapElement;
     class IntPoint;
+    class JSNode;
     class MouseEventWithHitTestResults;
     class NodeFilter;
     class NodeIterator;
@@ -991,6 +992,12 @@ private:
 
     unsigned m_numNodeListCaches;
 
+public:
+    typedef HashMap<WebCore::Node*, JSNode*> JSWrapperCache;
+    JSWrapperCache& wrapperCache() { return m_wrapperCache; }
+private:
+    JSWrapperCache m_wrapperCache;
+
 #if ENABLE(DATABASE)
     RefPtr<DatabaseThread> m_databaseThread;
     bool m_hasOpenDatabases;    // This never changes back to false, even as the database thread is closed.