Changeset 49280 in webkit for trunk/JavaScriptCore/runtime/Collector.cpp

Timestamp:

Oct 7, 2009, 6:46:07 PM (16 years ago)

Author:

[email protected]

Message:

JavaScriptCore: Fixed <rdar://problem/5751979> Database code takes JSLock on secondary
thread, permanently slowing down JavaScript

Patch by Geoffrey Garen <​[email protected]> on 2009-10-07
Reviewed by Oliver Hunt.

Removed the optional lock from Heap::protect, Heap::unprotect, and friends,
since WebCore no longer uses it.

• JavaScriptCore.exp:
• runtime/Collector.cpp:

(JSC::Heap::protect):
(JSC::Heap::unprotect):
(JSC::Heap::markProtectedObjects):
(JSC::Heap::protectedGlobalObjectCount):
(JSC::Heap::protectedObjectCount):
(JSC::Heap::protectedObjectTypeCounts):

• runtime/Collector.h:

WebCore: Fixed <rdar://problem/5751979> Database code takes JSLock on secondary
thread, permanently slowing down JavaScript

Patch by Geoffrey Garen <​[email protected]> on 2009-10-07
Reviewed by Oliver Hunt.

Changed callback objects to use a standard helper object. The helper
object ASSERTs that it is deleted on the main thread, so no lock is
required when unprotecting its members. It also centralizes some previously
duplicated code.

Callback objects that might be deleted on a secondary thread use
callOnMainThread to delete their helper objects on the main thread.

• WebCore.xcodeproj/project.pbxproj:
• bindings/js/JSCallbackData.cpp: Copied from bindings/js/JSCustomSQLTransactionCallback.cpp.

(WebCore::JSCallbackData::deleteData):
(WebCore::JSCallbackData::invokeCallback):

• bindings/js/JSCallbackData.h: Copied from bindings/js/JSCustomSQLTransactionCallback.cpp.

(WebCore::JSCallbackData::JSCallbackData):
(WebCore::JSCallbackData::~JSCallbackData):
(WebCore::JSCallbackData::callback):

• bindings/js/JSCustomPositionCallback.cpp:

(WebCore::JSCustomPositionCallback::JSCustomPositionCallback):
(WebCore::JSCustomPositionCallback::handleEvent):

• bindings/js/JSCustomPositionCallback.h:
• bindings/js/JSCustomPositionErrorCallback.cpp:

(WebCore::JSCustomPositionErrorCallback::JSCustomPositionErrorCallback):
(WebCore::JSCustomPositionErrorCallback::handleEvent):

• bindings/js/JSCustomPositionErrorCallback.h:
• bindings/js/JSCustomSQLStatementCallback.cpp:

(WebCore::JSCustomSQLStatementCallback::JSCustomSQLStatementCallback):
(WebCore::JSCustomSQLStatementCallback::~JSCustomSQLStatementCallback):
(WebCore::JSCustomSQLStatementCallback::handleEvent):

• bindings/js/JSCustomSQLStatementCallback.h:
• bindings/js/JSCustomSQLStatementErrorCallback.cpp:

(WebCore::JSCustomSQLStatementErrorCallback::JSCustomSQLStatementErrorCallback):
(WebCore::JSCustomSQLStatementErrorCallback::~JSCustomSQLStatementErrorCallback):
(WebCore::JSCustomSQLStatementErrorCallback::handleEvent):

• bindings/js/JSCustomSQLStatementErrorCallback.h:
• bindings/js/JSCustomSQLTransactionCallback.cpp:

(WebCore::JSCustomSQLTransactionCallback::JSCustomSQLTransactionCallback):
(WebCore::JSCustomSQLTransactionCallback::~JSCustomSQLTransactionCallback):
(WebCore::JSCustomSQLTransactionCallback::handleEvent):

• bindings/js/JSCustomSQLTransactionCallback.h:
• bindings/js/JSCustomSQLTransactionErrorCallback.cpp:

(WebCore::JSCustomSQLTransactionErrorCallback::JSCustomSQLTransactionErrorCallback):
(WebCore::JSCustomSQLTransactionErrorCallback::~JSCustomSQLTransactionErrorCallback):
(WebCore::JSCustomSQLTransactionErrorCallback::handleEvent):

• bindings/js/JSCustomSQLTransactionErrorCallback.h:
• bindings/js/JSCustomVoidCallback.cpp:

(WebCore::JSCustomVoidCallback::JSCustomVoidCallback):
(WebCore::JSCustomVoidCallback::~JSCustomVoidCallback):
(WebCore::JSCustomVoidCallback::handleEvent):

• bindings/js/JSCustomVoidCallback.h:
• storage/Database.cpp:

(WebCore::Database::Database):

File:

• trunk/JavaScriptCore/runtime/Collector.cpp (modified) (8 diffs)

trunk/JavaScriptCore/runtime/Collector.cpp

@@ -981 +981 @@
 }
 
-void Heap::setGCProtectNeedsLocking()
-{
-    // Most clients do not need to call this, with the notable exception of WebCore.
-    // Clients that use shared heap have JSLock protection, while others are supposed
-    // to do explicit locking. WebCore violates this contract in Database code,
-    // which calls gcUnprotect from a secondary thread.
-    if (!m_protectedValuesMutex)
-        m_protectedValuesMutex.set(new Mutex);
-}
-
 void Heap::protect(JSValue k)
 {
@@ -999 +989 @@
         return;
 
-    if (m_protectedValuesMutex)
-        m_protectedValuesMutex->lock();
-
     m_protectedValues.add(k.asCell());
-
-    if (m_protectedValuesMutex)
-        m_protectedValuesMutex->unlock();
 }
 
@@ -1016 +1000 @@
         return;
 
-    if (m_protectedValuesMutex)
-        m_protectedValuesMutex->lock();
-
     m_protectedValues.remove(k.asCell());
-
-    if (m_protectedValuesMutex)
-        m_protectedValuesMutex->unlock();
 }
 
 void Heap::markProtectedObjects(MarkStack& markStack)
 {
-    if (m_protectedValuesMutex)
-        m_protectedValuesMutex->lock();
-
     ProtectCountSet::iterator end = m_protectedValues.end();
     for (ProtectCountSet::iterator it = m_protectedValues.begin(); it != end; ++it) {
@@ -1035 +1010 @@
         markStack.drain();
     }
-
-    if (m_protectedValuesMutex)
-        m_protectedValuesMutex->unlock();
 }
 
@@ -1229 +1201 @@
 size_t Heap::protectedGlobalObjectCount()
 {
-    if (m_protectedValuesMutex)
-        m_protectedValuesMutex->lock();
-
     size_t count = 0;
     if (JSGlobalObject* head = m_globalData->head) {
@@ -1242 +1211 @@
     }
 
-    if (m_protectedValuesMutex)
-        m_protectedValuesMutex->unlock();
-
     return count;
 }
@@ -1250 +1216 @@
 size_t Heap::protectedObjectCount()
 {
-    if (m_protectedValuesMutex)
-        m_protectedValuesMutex->lock();
-
-    size_t result = m_protectedValues.size();
-
-    if (m_protectedValuesMutex)
-        m_protectedValuesMutex->unlock();
-
-    return result;
+    return m_protectedValues.size();
 }
 
@@ -1280 +1238 @@
     HashCountedSet<const char*>* counts = new HashCountedSet<const char*>;
 
-    if (m_protectedValuesMutex)
-        m_protectedValuesMutex->lock();
-
     ProtectCountSet::iterator end = m_protectedValues.end();
     for (ProtectCountSet::iterator it = m_protectedValues.begin(); it != end; ++it)
         counts->add(typeName(it->first));
 
-    if (m_protectedValuesMutex)
-        m_protectedValuesMutex->unlock();
-
     return counts;
 }