fix bug 139243. Remove reference count based rooting of JS objects of wrapped natives and replace it with GC-time marking. This is a pretty good DOM performance win. r=dbradley sr=jst.

js/src/xpconnect/src/xpcjsruntime.cpp

@@ -234,6 +234,13 @@ JSBool XPCJSRuntime::GCCallback(JSContext *cx, JSGCStatus status)
             case JSGC_MARK_END:
             {
                 NS_ASSERTION(!self->mDoingFinalization, "bad state");
+    
+                // mThreadRunningGC indicates that GC is running
+                { // scoped lock
+                    XPCAutoLock lock(self->GetMapLock());
+                    NS_ASSERTION(!self->mThreadRunningGC, "bad state");
+                    self->mThreadRunningGC = PR_GetCurrentThread();
+                }
 
                 // Skip this part if XPConnect is shutting down. We get into
                 // bad locking problems with the thread iteration otherwise.
@@ -471,6 +478,16 @@ JSBool XPCJSRuntime::GCCallback(JSContext *cx, JSGCStatus status)
                 self->mDyingWrappedNativeProtoMap->
                     Enumerate(DyingProtoKiller, nsnull);
 
+
+                // mThreadRunningGC indicates that GC is running.
+                // Clear it and notify waiters.
+                { // scoped lock
+                    XPCAutoLock lock(self->GetMapLock());
+                    NS_ASSERTION(self->mThreadRunningGC == PR_GetCurrentThread(), "bad state");
+                    self->mThreadRunningGC = nsnull;
+                    xpc_NotifyAll(self->GetMapLock());
+                }
+
                 break;
             }
             case JSGC_END:
@@ -738,6 +755,7 @@ XPCJSRuntime::XPCJSRuntime(nsXPConnect* aXPConnect,
    mDyingWrappedNativeProtoMap(XPCWrappedNativeProtoMap::newMap(XPC_DYING_NATIVE_PROTO_MAP_SIZE)),
    mDetachedWrappedNativeProtoMap(XPCWrappedNativeProtoMap::newMap(XPC_DETACHED_NATIVE_PROTO_MAP_SIZE)),
    mMapLock(XPCAutoLock::NewLock("XPCJSRuntime::mMapLock")),
+   mThreadRunningGC(nsnull),
    mWrappedJSToReleaseArray(),
    mNativesToReleaseArray(),
    mMainThreadOnlyGC(JS_FALSE),

js/src/xpconnect/src/xpcprivate.h

@@ -229,10 +229,30 @@ extern const char XPC_XPCONNECT_CONTRACTID[];
 
 typedef PRMonitor XPCLock;
 
+static inline void xpc_Wait(XPCLock* lock) 
+    {
+        NS_ASSERTION(lock, "xpc_Wait called with null lock!");
+#ifdef DEBUG
+        PRStatus result = 
+#endif    
+        PR_Wait(lock, PR_INTERVAL_NO_TIMEOUT);
+        NS_ASSERTION(PR_SUCCESS == result, "bad result from PR_Wait!");
+    }
+
+static inline void xpc_NotifyAll(XPCLock* lock) 
+    {
+        NS_ASSERTION(lock, "xpc_NotifyAll called with null lock!");
+#ifdef DEBUG
+        PRStatus result = 
+#endif    
+        PR_NotifyAll(lock);
+        NS_ASSERTION(PR_SUCCESS == result, "bad result from PR_NotifyAll!");
+    }
+
 // This is a cloned subset of nsAutoMonitor. We want the use of a monitor -
 // mostly because we need reenterability - but we also want to support passing
 // a null monitor in without things blowing up. This is used for wrappers that
-// are gaurenteeded to be used only on one thread. We avoid lock overhead by
+// are guaranteed to be used only on one thread. We avoid lock overhead by
 // using a null monitor. By changing this class we can avoid having multiplte
 // code paths or (conditional) manual calls to PR_{Enter,Exit}Monitor.
 //
@@ -547,6 +567,8 @@ public:
 
     void SystemIsBeingShutDown(XPCCallContext* ccx);
 
+    PRThread* GetThreadRunningGC() const {return mThreadRunningGC;}
+
     ~XPCJSRuntime();
 
 #ifdef XPC_CHECK_WRAPPERS_AT_SHUTDOWN
@@ -594,6 +616,7 @@ private:
     XPCWrappedNativeProtoMap* mDyingWrappedNativeProtoMap;
     XPCWrappedNativeProtoMap* mDetachedWrappedNativeProtoMap;
     XPCLock* mMapLock;
+    PRThread* mThreadRunningGC;
     nsVoidArray mWrappedJSToReleaseArray;
     nsVoidArray mNativesToReleaseArray;
     JSBool mMainThreadOnlyGC;
@@ -1863,6 +1886,8 @@ public:
                         nsIClassInfo* classInfo,
                         XPCNativeScriptableCreateInfo* sciProto);
 
+    JSBool HasExternalReference() const {return mRefCnt > 1;}
+
     // Make ctor and dtor protected (rather than private) to placate nsCOMPtr.
 protected:
     XPCWrappedNative(); // not implemented

js/src/xpconnect/src/xpcwrappednative.cpp

@@ -205,6 +205,9 @@ XPCWrappedNative::GetNewOrUsed(XPCCallContext& ccx,
 {
     nsresult rv;
 
+    NS_ASSERTION(!Scope->GetRuntime()->GetThreadRunningGC(), 
+                 "XPCWrappedNative::GetNewOrUsed called during GC");
+
     nsCOMPtr<nsISupports> identity(do_QueryInterface(Object));
     if(!identity)
     {
@@ -745,17 +748,18 @@ NS_INTERFACE_MAP_BEGIN(XPCWrappedNative)
   NS_INTERFACE_MAP_ENTRY_AMBIGUOUS(nsISupports, nsIXPConnectWrappedNative)
 NS_INTERFACE_MAP_END_THREADSAFE
 
+NS_IMPL_THREADSAFE_ADDREF(XPCWrappedNative)
+NS_IMPL_THREADSAFE_RELEASE(XPCWrappedNative)
 
 /*
  *  Wrapped Native lifetime management is messy!
  *
  *  - At creation we push the refcount to 2 (only one of which is owned by
  *    the native caller that caused the wrapper creation).
- *  - Whenever the refcount goes from 1 -> 2 we root mFlatJSObject.
- *  - Whenever the refcount goes from 2 -> 1 we unroot mFlatJSObject
- *    (in this state *no* native callers own a reference to the wrapper).
+ *  - During the JS GC Mark phase we mark any wrapper with a refcount > 1.
  *  - The *only* thing that can make the wrapper get destroyed is the
- *    finalization of mFlatJSObject.
+ *    finalization of mFlatJSObject. And *that* should only happen if the only
+ *    reference is the single extra (internal) reference we hold.
  *
  *  - The wrapper has a pointer to the nsISupports 'view' of the wrapped native
  *    object i.e... mIdentity. This is held until the wrapper's refcount goes
@@ -799,42 +803,6 @@ NS_INTERFACE_MAP_END_THREADSAFE
  *    finalization within a given gc cycle.
  */
 
-nsrefcnt
-XPCWrappedNative::AddRef(void)
-{
-    nsrefcnt cnt = (nsrefcnt) PR_AtomicIncrement((PRInt32*)&mRefCnt);
-    NS_LOG_ADDREF(this, cnt, "XPCWrappedNative", sizeof(*this));
-    if(2 == cnt && IsValid())
-    {
-        XPCJSRuntime* rt = GetRuntime();
-        if(rt)
-            JS_AddNamedRootRT(rt->GetJSRuntime(), &mFlatJSObject,
-                              "XPCWrappedNative::mFlatJSObject");
-    }
-    return cnt;
-}
-
-nsrefcnt
-XPCWrappedNative::Release(void)
-{
-    NS_PRECONDITION(0 != mRefCnt, "dup release");
-    nsrefcnt cnt = (nsrefcnt) PR_AtomicDecrement((PRInt32*)&mRefCnt);
-    NS_LOG_RELEASE(this, cnt, "XPCWrappedNative");
-    if(0 == cnt)
-    {
-        NS_ASSERTION(!mFlatJSObject,"Caller is releasing internally owned reference!");
-        NS_DELETEXPCOM(this);
-        return 0;
-    }
-    if(1 == cnt && IsValid())
-    {
-        XPCJSRuntime* rt = GetRuntime();
-        if(rt)
-            JS_RemoveRootRT(rt->GetJSRuntime(), &mFlatJSObject);
-    }
-    return cnt;
-}
-
 void
 XPCWrappedNative::FlatJSObjectFinalized(JSContext *cx, JSObject *obj)
 {

js/src/xpconnect/src/xpcwrappednativescope.cpp

@@ -213,6 +213,19 @@ XPCWrappedNativeScope::~XPCWrappedNativeScope()
 }
 
 
+JS_STATIC_DLL_CALLBACK(JSDHashOperator)
+WrappedNativeJSGCThingMarker(JSDHashTable *table, JSDHashEntryHdr *hdr,
+                             uint32 number, void *arg)
+{
+    XPCWrappedNative* wrapper = ((Native2WrappedNativeMap::Entry*)hdr)->value;
+    if(wrapper->HasExternalReference())
+    {
+        JS_MarkGCThing((JSContext*)arg, wrapper->GetFlatJSObject(), 
+                       "XPCWrappedNative::mFlatJSObject", nsnull);
+    }
+    return JS_DHASH_NEXT;
+}
+
 // static
 void
 XPCWrappedNativeScope::FinishedMarkPhaseOfGC(JSContext* cx, XPCJSRuntime* rt)
@@ -220,12 +233,21 @@ XPCWrappedNativeScope::FinishedMarkPhaseOfGC(JSContext* cx, XPCJSRuntime* rt)
     // Hold the lock until return...
     XPCAutoLock lock(rt->GetMapLock());
 
+    XPCWrappedNativeScope* cur;
+    
+    // Do JS_MarkGCThing for all wrapperednatives with external references.
+    for(cur = gScopes; cur; cur = cur->mNext)
+    {
+        cur->mWrappedNativeMap->Enumerate(WrappedNativeJSGCThingMarker, cx);
+    }
+
     // Since the JSGC_END call happens outside of a lock,
     // it is possible for us to get called here twice before the FinshedGC
     // call happens. So, we allow for gDyingScopes not being null.
 
-    XPCWrappedNativeScope* cur = gScopes;
     XPCWrappedNativeScope* prev = nsnull;
+    cur = gScopes;
+
     while(cur)
     {
         XPCWrappedNativeScope* next = cur->mNext;