Changeset 18786 in webkit for trunk/JavaScriptCore/bindings/runtime_root.cpp

Timestamp:

Jan 11, 2007, 4:46:36 PM (18 years ago)

Author:

ggaren

Message:

Reviewed by Anders Carlsson.

Even more cleanup in preparation for fixing <rdar://problem/4608404>
WebScriptObject's _executionContext lack of ownership policy causes
crashes (e.g., in Dashcode)

Layout tests pass.

Renames:

ReferencesSet | ProtectCounts => ProtectCountSet (because it's a typename for a set of GC protect counts)
ReferencesByRootMap => RootObjectMap (because RootObjectToProtectCountSetMap would have been confusing)
pv => protectedValues
rootObjectForImp => getRootObject (overloaded for JSObject* and Interpreter*)
rootObjectForInterpreter => getRootObject (ditto)
findReferenceSet => getProtectCountSet
imp => jsObject

(KJS::Bindings::getRootObjectMap): Changed to take advantage of built-in
facility for initializing static variables.

(KJS::Bindings::getProtectCountSet):
(KJS::Bindings::destroyProtectCountSet): Added. Helps encapsulate the fact
that getting a ProtectCountSet entails adding a RootObject to a hash table,
and destroying one entails the reverse.

(KJS::Bindings::getRootObject): Removed spurious NULL check.

(KJS::Bindings::findReferenceSet): Renamed. Changed to use getRootObject()
instead of iterating on its own.

(KJS::Bindings::addNativeReference): Changed to use an early return instead
of indenting the whole function.
(KJS::Bindings::removeNativeReference): Ditto.

File:

• trunk/JavaScriptCore/bindings/runtime_root.cpp (modified) (4 diffs)

trunk/JavaScriptCore/bindings/runtime_root.cpp

@@ -45 +45 @@
 // are represented by a jlong.
 
-typedef HashMap<const RootObject*, ReferencesSet*> ReferencesByRootMap; 
-
-static ReferencesByRootMap* getReferencesByRootMap()
-{
-    static ReferencesByRootMap* referencesByRootMap = 0;
-    
-    if (!referencesByRootMap)
-        referencesByRootMap = new ReferencesByRootMap;
-    
-    return referencesByRootMap;
-}
-
-static ReferencesSet* getReferencesSet(const RootObject* rootObject)
-{
-    ReferencesByRootMap* refsByRoot = getReferencesByRootMap();
-    ReferencesSet* referencesSet = 0;
-    
-    referencesSet = refsByRoot->get(rootObject);
-    if (!referencesSet) {
-        referencesSet  = new ReferencesSet;
-        refsByRoot->add(rootObject, referencesSet);
-    }
-    return referencesSet;
-}
-
-// Scan all the dictionary for all the roots to see if any have a 
-// reference to the imp, and if so, return it's reference count
-// dictionary.
-// FIXME:  This is a potential performance bottleneck with many applets.  We could fix be adding a
-// imp to root dictionary.
-ReferencesSet* findReferenceSet(JSObject* imp)
-{
-    ReferencesByRootMap* refsByRoot = getReferencesByRootMap ();
-    if (refsByRoot) {
-        ReferencesByRootMap::const_iterator end = refsByRoot->end();
-        for (ReferencesByRootMap::const_iterator it = refsByRoot->begin(); it != end; ++it) {
-            ReferencesSet* set = it->second;
-            
-            if (set->contains(imp))
-                return set;
-        }
+typedef HashMap<const RootObject*, ProtectCountSet*> RootObjectMap;
+
+static RootObjectMap* getRootObjectMap()
+{
+    static RootObjectMap staticRootObjectMap;
+    return &staticRootObjectMap;
+}
+
+static ProtectCountSet* getProtectCountSet(const RootObject* rootObject)
+{
+    RootObjectMap* rootObjectMap = getRootObjectMap();
+    ProtectCountSet* protectCountSet = rootObjectMap->get(rootObject);
+
+    if (!protectCountSet) {
+        protectCountSet = new ProtectCountSet;
+        rootObjectMap->add(rootObject, protectCountSet);
+    }
+    return protectCountSet;
+}
+
+static void destroyProtectCountSet(const RootObject* rootObject, ProtectCountSet* protectCountSet)
+{
+    RootObjectMap* rootObjectMap = getRootObjectMap();
+    rootObjectMap->remove(rootObject);
+    delete protectCountSet;
+}
+
+// FIXME:  These two functions are a potential performance problem.  We could 
+// fix them by adding a JSObject to RootObject dictionary.
+
+ProtectCountSet* getProtectCountSet(JSObject* jsObject)
+{
+    const RootObject* rootObject = getRootObject(jsObject);
+    return getProtectCountSet(rootObject);
+}
+
+const RootObject* getRootObject(JSObject* jsObject)
+{
+    RootObjectMap* rooObjectMap = getRootObjectMap();
+    
+    RootObjectMap::const_iterator end = rooObjectMap->end();
+    for (RootObjectMap::const_iterator it = rooObjectMap->begin(); it != end; ++it) {
+        ProtectCountSet* set = it->second;
+        if (set->contains(jsObject))
+            return it->first;
     }
     
@@ -91 +95 @@
 }
 
-// FIXME:  This is a potential performance bottleneck with many applets.  We could fix be adding a
-// imp to root dictionary.
-const RootObject* rootObjectForImp (JSObject* imp)
-{
-    ReferencesByRootMap* refsByRoot = getReferencesByRootMap ();
-    const RootObject* rootObject = 0;
-    
-    if (refsByRoot) {
-        ReferencesByRootMap::const_iterator end = refsByRoot->end();
-        for (ReferencesByRootMap::const_iterator it = refsByRoot->begin(); it != end; ++it) {
-            ReferencesSet* set = it->second;
-            if (set->contains(imp)) {
-                rootObject = it->first;
-                break;
-            }
-        }
-    }
-    
-    return rootObject;
-}
-
-const RootObject* rootObjectForInterpreter(Interpreter* interpreter)
-{
-    ReferencesByRootMap* refsByRoot = getReferencesByRootMap ();
-    
-    if (refsByRoot) {
-        ReferencesByRootMap::const_iterator end = refsByRoot->end();
-        for (ReferencesByRootMap::const_iterator it = refsByRoot->begin(); it != end; ++it) {
+const RootObject* getRootObject(Interpreter* interpreter)
+{
+    RootObjectMap* rooObjectMap = getRootObjectMap();
+    
+    if (rooObjectMap) {
+        RootObjectMap::const_iterator end = rooObjectMap->end();
+        for (RootObjectMap::const_iterator it = rooObjectMap->begin(); it != end; ++it) {
             const RootObject* aRootObject = it->first;
             
@@ -129 +112 @@
 }
 
-void addNativeReference(const RootObject* rootObject, JSObject* imp)
-{
-    if (rootObject) {
-        ReferencesSet* referenceMap = getReferencesSet(rootObject);
-        
-        unsigned numReferences = referenceMap->count(imp);
-        if (numReferences == 0) {
-            JSLock lock;
-            gcProtect(imp);
-        }
-        referenceMap->add(imp);
-    }
-}
-
-void removeNativeReference(JSObject* imp)
-{
-    if (!imp)
+void addNativeReference(const RootObject* rootObject, JSObject* jsObject)
+{
+    if (!rootObject)
         return;
-
-    ReferencesSet *referencesSet = findReferenceSet(imp);
-    if (referencesSet) {
-        unsigned numReferences = referencesSet->count(imp);
-        
-        if (numReferences == 1) {
-            JSLock lock;
-            gcUnprotect(imp);
-        }
-        referencesSet->remove(imp);
-    }
+    
+    ProtectCountSet* protectCountSet = getProtectCountSet(rootObject);
+    if (!protectCountSet->contains(jsObject)) {
+        JSLock lock;
+        gcProtect(jsObject);
+    }
+    protectCountSet->add(jsObject);
+}
+
+void removeNativeReference(JSObject* jsObject)
+{
+    if (!jsObject)
+        return;
+
+    ProtectCountSet* protectCountSet = getProtectCountSet(jsObject);
+    if (protectCountSet->count(jsObject) == 1) {
+        JSLock lock;
+        gcUnprotect(jsObject);
+    }
+    protectCountSet->remove(jsObject);
 }
 
@@ -287 +265 @@
 void RootObject::destroy()
 {
-    ReferencesSet* referencesSet = getReferencesSet(this);
-    
-    if (referencesSet) {
-        ReferencesSet::iterator end = referencesSet->end();
-        for (ReferencesSet::iterator it = referencesSet->begin(); it != end; ++it) {
+    ProtectCountSet* protectCountSet = getProtectCountSet(this);
+    
+    if (protectCountSet) {
+        ProtectCountSet::iterator end = protectCountSet->end();
+        for (ProtectCountSet::iterator it = protectCountSet->begin(); it != end; ++it) {
             JSLock lock;
             gcUnprotect(it->first);            
         }
-        referencesSet->clear();
-        ReferencesByRootMap* refsByRoot = getReferencesByRootMap();
-        refsByRoot->remove(this);
-        delete referencesSet;
+
+        destroyProtectCountSet(this, protectCountSet);
     }