Changeset 34977 in webkit for trunk/JavaScriptCore

Timestamp:

Jul 3, 2008, 12:44:43 AM (17 years ago)

Author:

[email protected]

Message:

Reviewed by Geoff.

Don't create unnecessary JSGlobalData instances.

• kjs/JSGlobalData.h:
• kjs/JSGlobalData.cpp: (KJS::JSGlobalData::threadInstanceExists): (KJS::JSGlobalData::sharedInstanceExists): (KJS::JSGlobalData::threadInstance): (KJS::JSGlobalData::sharedInstance): (KJS::JSGlobalData::threadInstanceInternal): (KJS::JSGlobalData::sharedInstanceInternal): Added methods to query instance existence.

• kjs/InitializeThreading.cpp: (KJS::initializeThreadingOnce): Initialize thread instance static in a new way.

• API/JSBase.cpp: (JSGarbageCollect):
• kjs/collector.cpp: (KJS::Heap::collect): Check for instance existence before accessing it.

Location:

trunk/JavaScriptCore

Files:

• API/JSBase.cpp (modified) (1 diff)
• ChangeLog (modified) (1 diff)
• kjs/InitializeThreading.cpp (modified) (1 diff)
• kjs/JSGlobalData.cpp (modified) (2 diffs)
• kjs/JSGlobalData.h (modified) (3 diffs)
• kjs/collector.cpp (modified) (1 diff)

trunk/JavaScriptCore/API/JSBase.cpp

@@ -89 +89 @@
     // and it may actually be garbage for some clients (most likely, because of JSGarbageCollect being called after releasing the context).
 
-    // FIXME: It would be good to avoid creating a JSGlobalData instance if it didn't exist for this thread yet.
-    Heap* heap = JSGlobalData::threadInstance().heap;
-    if (!heap->isBusy())
-        heap->collect();
+    if (JSGlobalData::threadInstanceExists()) {
+        Heap* heap = JSGlobalData::threadInstance().heap;
+        if (!heap->isBusy())
+            heap->collect();
+    }
 
     JSLock lock(true);
 
-    // FIXME: Similarly, we shouldn't create a shared instance here.
-    heap = JSGlobalData::sharedInstance().heap;
-    if (!heap->isBusy())
-        heap->collect();
+    if (JSGlobalData::sharedInstanceExists()) {
+        Heap* heap = JSGlobalData::sharedInstance().heap;
+        if (!heap->isBusy())
+            heap->collect();
+    }
 
     // FIXME: Perhaps we should trigger a second mark and sweep

trunk/JavaScriptCore/ChangeLog

@@ +1 @@
+2008-07-02  Alexey Proskuryakov  <[email protected]>
+
+        Reviewed by Geoff.
+
+        Don't create unnecessary JSGlobalData instances.
+
+        * kjs/JSGlobalData.h:
+        * kjs/JSGlobalData.cpp:
+        (KJS::JSGlobalData::threadInstanceExists):
+        (KJS::JSGlobalData::sharedInstanceExists):
+        (KJS::JSGlobalData::threadInstance):
+        (KJS::JSGlobalData::sharedInstance):
+        (KJS::JSGlobalData::threadInstanceInternal):
+        (KJS::JSGlobalData::sharedInstanceInternal):
+        Added methods to query instance existence.
+
+        * kjs/InitializeThreading.cpp:
+        (KJS::initializeThreadingOnce):
+        Initialize thread instance static in a new way.
+
+        * API/JSBase.cpp:
+        (JSGarbageCollect):
+        * kjs/collector.cpp:
+        (KJS::Heap::collect):
+        Check for instance existence before accessing it.
+
 2008-07-02  Geoffrey Garen  <[email protected]>
 

trunk/JavaScriptCore/kjs/InitializeThreading.cpp

@@ -49 +49 @@
 #if USE(MULTIPLE_THREADS)
     s_dtoaP5Mutex = new Mutex;
-    JSGlobalData::threadInstance();
+    JSGlobalData::threadInstanceExists();
     UString::null();
     initDateMath();

trunk/JavaScriptCore/kjs/JSGlobalData.cpp

@@ -32 +32 @@
 #include "collector.h"
 #include "CommonIdentifiers.h"
+#include "JSLock.h"
 #include "lexer.h"
 #include "list.h"
@@ -120 +121 @@
 }
 
+bool JSGlobalData::threadInstanceExists()
+{
+    return threadInstanceInternal();
+}
+
+bool JSGlobalData::sharedInstanceExists()
+{
+    return sharedInstanceInternal();
+}
+
 JSGlobalData& JSGlobalData::threadInstance()
 {
-#if USE(MULTIPLE_THREADS)
-    static ThreadSpecific<JSGlobalData> sharedInstance;
-    return *sharedInstance;
-#else
-    static JSGlobalData sharedInstance;
-    return sharedInstance;
-#endif
+    JSGlobalData*& instance = threadInstanceInternal();
+    if (!instance)
+        instance = new JSGlobalData;
+    return *instance;
 }
 
 JSGlobalData& JSGlobalData::sharedInstance()
 {
+    JSGlobalData*& instance = sharedInstanceInternal();
+    if (!instance)
+        instance = new JSGlobalData(true);
+    return *instance;
+}
+
+JSGlobalData*& JSGlobalData::threadInstanceInternal()
+{
 #if USE(MULTIPLE_THREADS)
-    MutexLocker locker(*atomicallyInitializedStaticMutex);
+    static ThreadSpecific<DataInstance> threadInstance;
+    return *threadInstance;
+#else
+    static JSGlobalData threadInstance;
+    return &threadInstance;
 #endif
+}
+
+JSGlobalData*& JSGlobalData::sharedInstanceInternal()
+{
+    ASSERT(JSLock::currentThreadIsHoldingLock());
     static JSGlobalData* sharedInstance;
-    if (!sharedInstance)
-        sharedInstance = new JSGlobalData(true);
-    return *sharedInstance;
+    return sharedInstance;
 }
 

trunk/JavaScriptCore/kjs/JSGlobalData.h

@@ -34 +34 @@
 #include <wtf/HashSet.h>
 #include <wtf/Noncopyable.h>
-
-namespace WTF {
-    template<typename> class ThreadSpecific;
-}
+#include <wtf/OwnPtr.h>
 
 namespace KJS {
@@ -56 +53 @@
     // JavaScriptCore clients, which all share a single JSGlobalData, and thus cannot run concurrently.
     struct JSGlobalData : Noncopyable {
+        static bool threadInstanceExists();
+        static bool sharedInstanceExists();
         static JSGlobalData& threadInstance();
         static JSGlobalData& sharedInstance();
@@ -85 +84 @@
 
     private:
-        friend class WTF::ThreadSpecific<JSGlobalData>;
-
         JSGlobalData(bool isShared = false);
         ~JSGlobalData();
+
+        static JSGlobalData*& threadInstanceInternal();
+        static JSGlobalData*& sharedInstanceInternal();
+
+        struct DataInstance {
+            DataInstance() : m_data(0) {}
+            ~DataInstance() { delete m_data; }
+            operator JSGlobalData*&() { return m_data; }
+
+            JSGlobalData* m_data;
+        };
     };
 

trunk/JavaScriptCore/kjs/collector.cpp

@@ -926 +926 @@
 {
 #ifndef NDEBUG
-    if (JSGlobalData::sharedInstance().heap == this) {
+    if (JSGlobalData::sharedInstanceExists() && JSGlobalData::sharedInstance().heap == this) {
         ASSERT(JSLock::lockCount() > 0);
         ASSERT(JSLock::currentThreadIsHoldingLock());