Ignore:
Timestamp:
Jul 2, 2008, 12:00:53 AM (17 years ago)
Author:
[email protected]
Message:

Reviewed by Darin.

Disable JSLock for per-thread contexts.

No change on SunSpider.

  • kjs/JSGlobalData.h:
  • kjs/JSGlobalData.cpp: (KJS::JSGlobalData::JSGlobalData): (KJS::JSGlobalData::sharedInstance): Added isSharedInstance as a better way to tell whether the instance is shared (legacy).
  • kjs/JSLock.cpp: (KJS::createJSLockCount): (KJS::JSLock::lockCount): (KJS::setLockCount): (KJS::JSLock::JSLock): (KJS::JSLock::lock): (KJS::JSLock::unlock): (KJS::JSLock::currentThreadIsHoldingLock): (KJS::JSLock::DropAllLocks::DropAllLocks): (KJS::JSLock::DropAllLocks::~DropAllLocks):
  • kjs/JSLock.h: (KJS::JSLock::JSLock): (KJS::JSLock::~JSLock): Made JSLock and JSLock::DropAllLocks constructors take a parameter to decide whether to actually lock a mutex, or only to increment recursion count. We cannot turn it into no-op if we want to keep existing assertions working. Made recursion count per-thread, now that locks may not lock.
  • API/JSBase.cpp: (JSEvaluateScript): Take JSLock after casting JSContextRef to ExecState* (which doesn't need locking in any case), so that a decision whether to actually lock can be made. (JSCheckScriptSyntax): Ditto. (JSGarbageCollect): Only lock while collecting the shared heap, not the per-thread one.
  • API/JSObjectRef.cpp: (JSClassCreate): Don't lock, as there is no reason to. (JSClassRetain): Ditto. (JSClassRelease): Ditto. (JSPropertyNameArrayRetain): Ditto. (JSPropertyNameArrayRelease): Only lock while deleting the array, as that may touch identifier table. (JSPropertyNameAccumulatorAddName): Adding a string also involves an identifier table lookup, and possibly modification.
  • API/JSStringRef.cpp: (JSStringCreateWithCharacters): (JSStringCreateWithUTF8CString): (JSStringRetain): (JSStringRelease): (JSStringGetUTF8CString): (JSStringIsEqual):
  • API/JSStringRefCF.cpp: (JSStringCreateWithCFString): JSStringRef operations other than releasing do not need locking.
  • VM/Machine.cpp: Don't include unused JSLock.h.
  • kjs/CollectorHeapIntrospector.cpp: (KJS::CollectorHeapIntrospector::statistics): Don't take the lock for real, as heap introspection pauses the process anyway. It seems that the existing code could cause deadlocks.
  • kjs/Shell.cpp: (functionGC): (main): (jscmain): The test tool uses a per-thread context, so no real locking is required.
  • kjs/collector.h: (KJS::Heap::setGCProtectNeedsLocking): Optionally protect m_protectedValues access with a per-heap mutex. This is only needed for WebCore Database code, which violates the "no data migration between threads" by using ProtectedPtr on a background thread. (KJS::Heap::isShared): Keep a shared flag here, as well.
  • kjs/protect.h: (KJS::::ProtectedPtr): (KJS::::~ProtectedPtr): (KJS::::operator): (KJS::operator==): (KJS::operator!=): ProtectedPtr is ony used from WebCore, so it doesn't need to take JSLock. An assertion in Heap::protect/unprotect guards agains possible future unlocked uses of ProtectedPtr in JSC.
  • kjs/collector.cpp: (KJS::Heap::Heap): Initialize m_isShared. (KJS::Heap::~Heap): No need to lock for real during destruction, but must keep assertions in sweep() working. (KJS::destroyRegisteredThread): Registered thread list is only accessed for shared heap, so locking is always needed here. (KJS::Heap::registerThread): Ditto. (KJS::Heap::markStackObjectsConservatively): Use m_isShared instead of comparing to a shared instance for a small speedup. (KJS::Heap::setGCProtectNeedsLocking): Create m_protectedValuesMutex. There is currently no way to undo this - and ideally, Database code will be fixed to lo longer require this quirk. (KJS::Heap::protect): Take m_protectedValuesMutex (if it exists) while accessing m_protectedValues. (KJS::Heap::unprotect): Ditto. (KJS::Heap::markProtectedObjects): Ditto. (KJS::Heap::protectedGlobalObjectCount): Ditto. (KJS::Heap::protectedObjectCount): Ditto. (KJS::Heap::protectedObjectTypeCounts): Ditto.
  • kjs/ustring.cpp:
  • kjs/ustring.h: Don't include JSLock.h, which is no longer used here. As a result, an explicit include had to be added to many files in JavaScriptGlue, WebCore and WebKit.
  • kjs/JSGlobalObject.cpp: (KJS::JSGlobalObject::init):
  • API/JSCallbackConstructor.cpp: (KJS::constructJSCallback):
  • API/JSCallbackFunction.cpp: (KJS::JSCallbackFunction::call):
  • API/JSCallbackObjectFunctions.h: (KJS::::init): (KJS::::getOwnPropertySlot): (KJS::::put): (KJS::::deleteProperty): (KJS::::construct): (KJS::::hasInstance): (KJS::::call): (KJS::::getPropertyNames): (KJS::::toNumber): (KJS::::toString): (KJS::::staticValueGetter): (KJS::::callbackGetter):
  • API/JSContextRef.cpp: (JSGlobalContextCreate): (JSGlobalContextRetain): (JSGlobalContextRelease):
  • API/JSValueRef.cpp: (JSValueIsEqual): (JSValueIsStrictEqual): (JSValueIsInstanceOfConstructor): (JSValueMakeNumber): (JSValueMakeString): (JSValueToNumber): (JSValueToStringCopy): (JSValueToObject): (JSValueProtect): (JSValueUnprotect):
  • JavaScriptCore.exp:
  • kjs/PropertyNameArray.h: (KJS::PropertyNameArray::globalData):
  • kjs/interpreter.cpp: (KJS::Interpreter::checkSyntax): (KJS::Interpreter::evaluate): Pass a parameter to JSLock/JSLock::DropAllLocks to decide whether the lock needs to be taken.
File:
1 edited

Legend:

Unmodified
Added
Removed

  • trunk/JavaScriptCore/kjs/collector.cpp

    r34907 r34947  
    2424#include "ExecState.h"
    2525#include "JSGlobalObject.h"
     26#include "JSLock.h"
    2627#include "JSString.h"
    2728#include "JSValue.h"
    … …  
    9192static void freeHeap(CollectorHeap*);
    9293
    93 Heap::Heap()
     94Heap::Heap(bool isShared)
    9495    : m_markListSet(0)
     96    , m_isShared(isShared)
    9597{
    9698    memset(&primaryHeap, 0, sizeof(CollectorHeap));
    … …  
    100102Heap::~Heap()
    101103{
    102     JSLock lock;
     104    JSLock lock(false);
    103105
    104106    delete m_markListSet;
    … …  
    439441    // Can't use JSLock convenience object here because we don't want to re-register
    440442    // an exiting thread.
    441     JSLock::lock();
     443    // JSLock is only used for code simplicity here, as we only need to protect registeredThreads
     444    // list manipulation, not JS data structures.
     445    JSLock::lock(true);
    442446
    443447    if (registeredThreads == thread) {
    … …  
    456460    }
    457461
    458     JSLock::unlock();
     462    JSLock::unlock(true);
    459463
    460464    delete thread;
    … …  
    468472void Heap::registerThread()
    469473{
     474    // Since registerThread() is only called when using a shared heap, these locks will be real.
    470475    ASSERT(JSLock::lockCount() > 0);
    471476    ASSERT(JSLock::currentThreadIsHoldingLock());
    … …  
    729734#if USE(MULTIPLE_THREADS)
    730735
    731     if (this == JSGlobalData::sharedInstance().heap) {
     736    if (m_isShared) {
    732737
    733738#ifndef NDEBUG
    … …  
    737742        fastMallocForbid();
    738743#endif
     744        // It is safe to access the registeredThreads list, because we earlier asserted that locks are being held,
     745        // and since this is a shared heap, they are real locks.
    739746        for (Thread* thread = registeredThreads; thread != NULL; thread = thread->next) {
    740747            if (!pthread_equal(thread->posixThread, pthread_self()))
    … …  
    748755}
    749756
     757void Heap::setGCProtectNeedsLocking()
     758{
     759    // Most clients do not need to call this, with the notable exception of WebCore.
     760    // Clients that use shared heap have JSLock protection, while others are not supposed
     761    // to migrate JS objects between threads. WebCore violates this contract in Database code,
     762    // which calls gcUnprotect from a secondary thread.
     763    if (!m_protectedValuesMutex)
     764        m_protectedValuesMutex.set(new Mutex);
     765}
     766
    750767void Heap::protect(JSValue* k)
    751768{
    752769    ASSERT(k);
    753     ASSERT(JSLock::lockCount() > 0);
    754     ASSERT(JSLock::currentThreadIsHoldingLock());
     770    ASSERT(JSLock::currentThreadIsHoldingLock() || !m_isShared);
    755771
    756772    if (JSImmediate::isImmediate(k))
    757773        return;
    758774
    759     protectedValues.add(k->asCell());
     775    if (m_protectedValuesMutex)
     776        m_protectedValuesMutex->lock();
     777
     778    m_protectedValues.add(k->asCell());
     779
     780    if (m_protectedValuesMutex)
     781        m_protectedValuesMutex->unlock();
    760782}
    761783
    … …  
    763785{
    764786    ASSERT(k);
    765     ASSERT(JSLock::lockCount() > 0);
    766     ASSERT(JSLock::currentThreadIsHoldingLock());
     787    ASSERT(JSLock::currentThreadIsHoldingLock() || !m_isShared);
    767788
    768789    if (JSImmediate::isImmediate(k))
    769790        return;
    770791
    771     protectedValues.remove(k->asCell());
     792    if (m_protectedValuesMutex)
     793        m_protectedValuesMutex->lock();
     794
     795    m_protectedValues.remove(k->asCell());
     796
     797    if (m_protectedValuesMutex)
     798        m_protectedValuesMutex->unlock();
    772799}
    773800
    … …  
    781808void Heap::markProtectedObjects()
    782809{
    783     ProtectCountSet::iterator end = protectedValues.end();
    784     for (ProtectCountSet::iterator it = protectedValues.begin(); it != end; ++it) {
     810    if (m_protectedValuesMutex)
     811        m_protectedValuesMutex->lock();
     812
     813    ProtectCountSet::iterator end = m_protectedValues.end();
     814    for (ProtectCountSet::iterator it = m_protectedValues.begin(); it != end; ++it) {
    785815        JSCell* val = it->first;
    786816        if (!val->marked())
    787817            val->mark();
    788818    }
     819
     820    if (m_protectedValuesMutex)
     821        m_protectedValuesMutex->unlock();
    789822}
    790823
    … …  
    941974size_t Heap::protectedGlobalObjectCount()
    942975{
     976    if (m_protectedValuesMutex)
     977        m_protectedValuesMutex->lock();
     978
    943979    size_t count = 0;
    944980    if (JSGlobalObject* head = JSGlobalData::threadInstance().head) {
    945981        JSGlobalObject* o = head;
    946982        do {
    947             if (protectedValues.contains(o))
     983            if (m_protectedValues.contains(o))
    948984                ++count;
    949985            o = o->next();
    950986        } while (o != head);
    951987    }
     988
     989    if (m_protectedValuesMutex)
     990        m_protectedValuesMutex->unlock();
     991
    952992    return count;
    953993}
    … …  
    955995size_t Heap::protectedObjectCount()
    956996{
    957     return protectedValues.size();
     997    if (m_protectedValuesMutex)
     998        m_protectedValuesMutex->lock();
     999
     1000    size_t result = m_protectedValues.size();
     1001
     1002    if (m_protectedValuesMutex)
     1003        m_protectedValuesMutex->unlock();
     1004
     1005    return result;
    9581006}
    9591007
    … …  
    9951043    HashCountedSet<const char*>* counts = new HashCountedSet<const char*>;
    9961044
    997     ProtectCountSet::iterator end = protectedValues.end();
    998     for (ProtectCountSet::iterator it = protectedValues.begin(); it != end; ++it)
     1045    if (m_protectedValuesMutex)
     1046        m_protectedValuesMutex->lock();
     1047
     1048    ProtectCountSet::iterator end = m_protectedValues.end();
     1049    for (ProtectCountSet::iterator it = m_protectedValues.begin(); it != end; ++it)
    9991050        counts->add(typeName(it->first));
    10001051
     1052    if (m_protectedValuesMutex)
     1053        m_protectedValuesMutex->unlock();
     1054
    10011055    return counts;
    10021056}
Note: See TracChangeset for help on using the changeset viewer.