2011-01-22 Geoffrey Garen <ggaren@apple.com>
authorggaren@apple.com <ggaren@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 23 Jan 2011 05:03:16 +0000 (05:03 +0000)
committerggaren@apple.com <ggaren@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 23 Jan 2011 05:03:16 +0000 (05:03 +0000)
        Reviewed by Dan Bernstein.

        ASSERT running run-webkit-tests --threaded.
        https://bugs.webkit.org/show_bug.cgi?id=52971

        SunSpider and v8 report no change.

        * runtime/ConservativeSet.cpp:
        (JSC::ConservativeSet::grow):
        (JSC::ConservativeSet::add):
        * runtime/ConservativeSet.h: Tweaked the inline capacity to 128, and
        the growth policy to 2X, to make SunSpider and v8 happy.
        (JSC::ConservativeSet::ConservativeSet):
        (JSC::ConservativeSet::~ConservativeSet):
        (JSC::ConservativeSet::mark): Use OSAllocator directly, instead of malloc.
        Malloc is forbidden during a multi-threaded mark phase because it can
        cause deadlock.
2011-01-22  Geoffrey Garen  <ggaren@apple.com>

        Reviewed by Dan Bernstein.

        Beefed up --threaded mode to catch even more kinds of errors.
        https://bugs.webkit.org/show_bug.cgi?id=52971

        * DumpRenderTree/pthreads/JavaScriptThreadingPthreads.cpp: Use a shared
        context group to force JSC to mark multiple threads. (This used to be
        the default, but it changed in SnowLeopard.)
        (runJavaScriptThread): Do more locking and unlocking, and more allocation,
        to give threading mistakes more chances to show themselves.
        (startJavaScriptThreads):
        (stopJavaScriptThreads):

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

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/ConservativeSet.cpp
Source/JavaScriptCore/runtime/ConservativeSet.h
Tools/ChangeLog
Tools/DumpRenderTree/pthreads/JavaScriptThreadingPthreads.cpp

index 26e2ed64b63a10b55b9df0bfaf549185c8216910..94a0b83e84b4a12ed02a6e55d1a0d02bcc8bb113 100644 (file)
@@ -1,3 +1,23 @@
+2011-01-22  Geoffrey Garen  <ggaren@apple.com>
+
+        Reviewed by Dan Bernstein.
+
+        ASSERT running run-webkit-tests --threaded.
+        https://bugs.webkit.org/show_bug.cgi?id=52971
+        
+        SunSpider and v8 report no change.
+
+        * runtime/ConservativeSet.cpp:
+        (JSC::ConservativeSet::grow):
+        (JSC::ConservativeSet::add):
+        * runtime/ConservativeSet.h: Tweaked the inline capacity to 128, and
+        the growth policy to 2X, to make SunSpider and v8 happy.
+        (JSC::ConservativeSet::ConservativeSet):
+        (JSC::ConservativeSet::~ConservativeSet):
+        (JSC::ConservativeSet::mark): Use OSAllocator directly, instead of malloc.
+        Malloc is forbidden during a multi-threaded mark phase because it can
+        cause deadlock.
+
 2011-01-22  Geoffrey Garen  <ggaren@apple.com>
 
         Reviewed by Geoffrey Garen.
index b2765026e823894dc8ffa09068b38bfa1a5874e3..930fe4aa2195bd8869bb57146bd47b67c7548c5a 100644 (file)
@@ -33,6 +33,17 @@ inline bool isPointerAligned(void* p)
     return !((intptr_t)(p) & (sizeof(char*) - 1));
 }
 
+void ConservativeSet::grow()
+{
+    size_t newCapacity = m_capacity == inlineCapacity ? nonInlineCapacity : m_capacity * 2;
+    JSCell** newSet = static_cast<JSCell**>(OSAllocator::reserveAndCommit(newCapacity * sizeof(JSCell*)));
+    memcpy(newSet, m_set, m_size * sizeof(JSCell*));
+    if (m_set != m_inlineSet)
+        OSAllocator::decommitAndRelease(m_set, m_capacity * sizeof(JSCell*));
+    m_capacity = newCapacity;
+    m_set = newSet;
+}
+
 void ConservativeSet::add(void* begin, void* end)
 {
     ASSERT(begin <= end);
@@ -43,7 +54,11 @@ void ConservativeSet::add(void* begin, void* end)
     for (char** it = static_cast<char**>(begin); it != static_cast<char**>(end); ++it) {
         if (!m_heap->contains(*it))
             continue;
-        m_vector.append(reinterpret_cast<JSCell*>(*it));
+
+        if (m_size == m_capacity)
+            grow();
+
+        m_set[m_size++] = reinterpret_cast<JSCell*>(*it);
     }
 }
 
index 9c7095027a6d371f725ce0004c8503ca12212e6d..276be89a25c4fad87e31ac1bc0f560721c65b9cf 100644 (file)
@@ -37,24 +37,42 @@ class JSCell;
 class ConservativeSet {
 public:
     ConservativeSet(Heap*);
+    ~ConservativeSet();
 
     void add(void* begin, void* end);
     void mark(MarkStack&);
 
 private:
+    static const size_t inlineCapacity = 128;
+    static const size_t nonInlineCapacity = 8192 / sizeof(JSCell*);
+    
+    void grow();
+
     Heap* m_heap;
-    Vector<JSCell*, 64> m_vector;
+    JSCell** m_set;
+    size_t m_size;
+    size_t m_capacity;
+    JSCell* m_inlineSet[inlineCapacity];
 };
 
 inline ConservativeSet::ConservativeSet(Heap* heap)
     : m_heap(heap)
+    , m_set(m_inlineSet)
+    , m_size(0)
+    , m_capacity(inlineCapacity)
+{
+}
+
+inline ConservativeSet::~ConservativeSet()
 {
+    if (m_set != m_inlineSet)
+        OSAllocator::decommitAndRelease(m_set, m_capacity * sizeof(JSCell*));
 }
 
 inline void ConservativeSet::mark(MarkStack& markStack)
 {
-    for (size_t i = 0; i < m_vector.size(); ++i)
-        markStack.append(m_vector[i]);
+    for (size_t i = 0; i < m_size; ++i)
+        markStack.append(m_set[i]);
 }
 
 } // namespace JSC
index f19bb75801c3c46d9e646a6fe94e5e1b427af191..63762646c1d1a10cb461d7a783726d441eef983d 100644 (file)
@@ -1,3 +1,18 @@
+2011-01-22  Geoffrey Garen  <ggaren@apple.com>
+
+        Reviewed by Dan Bernstein.
+
+        Beefed up --threaded mode to catch even more kinds of errors.
+        https://bugs.webkit.org/show_bug.cgi?id=52971
+
+        * DumpRenderTree/pthreads/JavaScriptThreadingPthreads.cpp: Use a shared
+        context group to force JSC to mark multiple threads. (This used to be
+        the default, but it changed in SnowLeopard.)
+        (runJavaScriptThread): Do more locking and unlocking, and more allocation,
+        to give threading mistakes more chances to show themselves.
+        (startJavaScriptThreads):
+        (stopJavaScriptThreads):
+
 2011-01-22  Robert Hogan  <robert@webkit.org>
 
         Reviewed by Andreas Kling.
index 5a48b27fe3a07c5e69e89807a1e1d5e1fb19d040..1266c020e9764e944d8c0889f4c42519a0aff83d 100644 (file)
@@ -37,6 +37,8 @@
 #include <wtf/Assertions.h>
 #include <wtf/HashSet.h>
 
+static JSContextGroupRef javaScriptThreadsGroup;
+
 static pthread_mutex_t javaScriptThreadsMutex = PTHREAD_MUTEX_INITIALIZER;
 static bool javaScriptThreadsShouldTerminate;
 
@@ -51,58 +53,68 @@ static ThreadSet* javaScriptThreads()
     return &staticJavaScriptThreads;
 }
 
-// Loops forever, running a script and randomly respawning, until 
-// javaScriptThreadsShouldTerminate becomes true.
+// This function exercises JSC in a loop until javaScriptThreadsShouldTerminate
+// becomes true or it probabilistically decides to spawn a replacement thread and exit.
 void* runJavaScriptThread(void* arg)
 {
-    const char* const script =
+    static const char* const script =
         "var array = [];"
-        "for (var i = 0; i < 10; i++) {"
+        "for (var i = 0; i < 1024; i++) {"
         "    array.push(String(i));"
         "}";
 
-    while (1) {
-        JSGlobalContextRef ctx = JSGlobalContextCreate(0);
-        JSStringRef scriptRef = JSStringCreateWithUTF8CString(script);
+    pthread_mutex_lock(&javaScriptThreadsMutex);
+    JSGlobalContextRef ctx = JSGlobalContextCreateInGroup(javaScriptThreadsGroup, 0);
+    pthread_mutex_unlock(&javaScriptThreadsMutex);
+
+    pthread_mutex_lock(&javaScriptThreadsMutex);
+    JSStringRef scriptRef = JSStringCreateWithUTF8CString(script);
+    pthread_mutex_unlock(&javaScriptThreadsMutex);
 
+    while (1) {
+        pthread_mutex_lock(&javaScriptThreadsMutex);
         JSValueRef exception = 0;
         JSEvaluateScript(ctx, scriptRef, 0, 0, 1, &exception);
         ASSERT(!exception);
-
-        JSGarbageCollect(ctx);
-        JSGlobalContextRelease(ctx);
-        JSStringRelease(scriptRef);
-        
-        JSGarbageCollect(0);
+        pthread_mutex_unlock(&javaScriptThreadsMutex);
 
         pthread_mutex_lock(&javaScriptThreadsMutex);
+        size_t valuesCount = 1024;
+        JSValueRef values[valuesCount];
+        for (size_t i = 0; i < valuesCount; ++i)
+            values[i] = JSObjectMake(ctx, 0, 0);
+        pthread_mutex_unlock(&javaScriptThreadsMutex);
 
         // Check for cancellation.
-        if (javaScriptThreadsShouldTerminate) {
-            javaScriptThreads()->remove(pthread_self());
-            pthread_mutex_unlock(&javaScriptThreadsMutex);
-            return 0;
-        }
+        if (javaScriptThreadsShouldTerminate)
+            goto done;
 
         // Respawn probabilistically.
         if (random() % 5 == 0) {
+            pthread_mutex_lock(&javaScriptThreadsMutex);
             pthread_t pthread;
             pthread_create(&pthread, 0, &runJavaScriptThread, 0);
             pthread_detach(pthread);
-
-            javaScriptThreads()->remove(pthread_self());
             javaScriptThreads()->add(pthread);
-
             pthread_mutex_unlock(&javaScriptThreadsMutex);
-            return 0;
+            goto done;
         }
-
-        pthread_mutex_unlock(&javaScriptThreadsMutex);
     }
+
+done:
+    pthread_mutex_lock(&javaScriptThreadsMutex);
+    JSStringRelease(scriptRef);
+    JSGarbageCollect(ctx);
+    JSGlobalContextRelease(ctx);
+    javaScriptThreads()->remove(pthread_self());
+    pthread_mutex_unlock(&javaScriptThreadsMutex);
+    return 0;
 }
 
 void startJavaScriptThreads()
 {
+    javaScriptThreadsGroup = JSContextGroupCreate();
+
     pthread_mutex_lock(&javaScriptThreadsMutex);
 
     for (int i = 0; i < javaScriptThreadsCount; i++) {
@@ -121,8 +133,6 @@ void stopJavaScriptThreads()
 
     javaScriptThreadsShouldTerminate = true;
 
-    ASSERT(javaScriptThreads()->size() == javaScriptThreadsCount);
-
     pthread_mutex_unlock(&javaScriptThreadsMutex);
 
     while (true) {