bug 722348 - eliminate JSRuntime::requestCount. r=luke
authorIgor Bukanov <igor@mir2.org>
Mon, 30 Jan 2012 11:15:13 +0100
changeset 86973 b7e5bbe0003df7d098e14d7b95a84cffc8501404
parent 86972 ea33b9fb8a717946f00ff0c218de6f9c9ed62b1a
child 86974 9f162faf97d261ed019addb9ff186b842e12d71e
push id805
push userakeybl@mozilla.com
push dateWed, 01 Feb 2012 18:17:35 +0000
treeherdermozilla-aurora@6fb3bf232436 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersluke
bugs722348
milestone12.0a1
bug 722348 - eliminate JSRuntime::requestCount. r=luke
ipc/testshell/XPCShellEnvironment.cpp
js/src/jsapi.cpp
js/src/jscntxt.cpp
js/src/jscntxt.h
js/xpconnect/shell/xpcshell.cpp
--- a/ipc/testshell/XPCShellEnvironment.cpp
+++ b/ipc/testshell/XPCShellEnvironment.cpp
@@ -1056,16 +1056,17 @@ XPCShellEnvironment::~XPCShellEnvironmen
         mCxStack = nsnull;
 
         if (mJSPrincipals) {
             JSPRINCIPALS_DROP(mCx, mJSPrincipals);
         }
 
         JSRuntime* rt = gOldContextCallback ? JS_GetRuntime(mCx) : NULL;
 
+        JS_EndRequest(mCx);
         JS_DestroyContext(mCx);
 
         if (gOldContextCallback) {
             NS_ASSERTION(rt, "Should never be null!");
             JS_SetContextCallback(rt, gOldContextCallback);
             gOldContextCallback = NULL;
         }
     }
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -760,17 +760,16 @@ JSRuntime::JSRuntime()
     positiveInfinityValue(UndefinedValue()),
     emptyString(NULL),
     debugMode(false),
     profilingScripts(false),
     hadOutOfMemory(false),
     data(NULL),
 #ifdef JS_THREADSAFE
     gcLock(NULL),
-    requestCount(0),
     gcHelperThread(thisFromCtor()),
 #endif
     debuggerMutations(0),
     securityCallbacks(NULL),
     structuredCloneCallbacks(NULL),
     telemetryCallback(NULL),
     propertyRemovals(0),
     thousandsSeparator(0),
@@ -1008,20 +1007,19 @@ StartRequest(JSContext *cx)
     JS_ASSERT(rt->onOwnerThread());
 
     if (rt->requestDepth) {
         rt->requestDepth++;
     } else {
         AutoLockGC lock(rt);
 
         /* Indicate that a request is running. */
-        rt->requestCount++;
         rt->requestDepth = 1;
 
-        if (rt->requestCount == 1 && rt->activityCallback)
+        if (rt->activityCallback)
             rt->activityCallback(rt->activityCallbackArg, true);
     }
 }
 
 static void
 StopRequest(JSContext *cx)
 {
     JSRuntime *rt = cx->runtime;
@@ -1032,23 +1030,18 @@ StopRequest(JSContext *cx)
     } else {
         rt->conservativeGC.updateForRequestEnd(rt->suspendCount);
 
         /* Lock before clearing to interlock with ClaimScope, in jslock.c. */
         AutoLockGC lock(rt);
 
         rt->requestDepth = 0;
 
-        /* Give the GC a chance to run if this was the last request running. */
-        JS_ASSERT(rt->requestCount > 0);
-        rt->requestCount--;
-        if (rt->requestCount == 0) {
-            if (rt->activityCallback)
-                rt->activityCallback(rt->activityCallbackArg, false);
-        }
+        if (rt->activityCallback)
+            rt->activityCallback(rt->activityCallbackArg, false);
     }
 }
 #endif /* JS_THREADSAFE */
 
 JS_PUBLIC_API(void)
 JS_BeginRequest(JSContext *cx)
 {
 #ifdef JS_THREADSAFE
--- a/js/src/jscntxt.cpp
+++ b/js/src/jscntxt.cpp
@@ -240,62 +240,42 @@ void
 js_DestroyContext(JSContext *cx, JSDestroyContextMode mode)
 {
     JSRuntime *rt = cx->runtime;
     JS_AbortIfWrongThread(rt);
 
     JS_ASSERT(!cx->enumerators);
 
 #ifdef JS_THREADSAFE
-    /*
-     * For API compatibility we support destroying contexts with non-zero
-     * cx->outstandingRequests but we assume that all JS_BeginRequest calls
-     * on this cx contributes to cx->thread->data.requestDepth and there is no
-     * JS_SuspendRequest calls that set aside the counter.
-     */
-    JS_ASSERT(cx->outstandingRequests <= cx->runtime->requestDepth);
+    JS_ASSERT(cx->outstandingRequests == 0);
 #endif
 
     if (mode != JSDCM_NEW_FAILED) {
         if (JSContextCallback cxCallback = rt->cxCallback) {
             /*
              * JSCONTEXT_DESTROY callback is not allowed to fail and must
              * return true.
              */
             DebugOnly<JSBool> callbackStatus = cxCallback(cx, JSCONTEXT_DESTROY);
             JS_ASSERT(callbackStatus);
         }
     }
 
     JS_LOCK_GC(rt);
     JS_REMOVE_LINK(&cx->link);
     bool last = !rt->hasContexts();
-    if (last || mode == JSDCM_FORCE_GC || mode == JSDCM_MAYBE_GC
-#ifdef JS_THREADSAFE
-        || cx->outstandingRequests != 0
-#endif
-        ) {
+    if (last || mode == JSDCM_FORCE_GC || mode == JSDCM_MAYBE_GC) {
         JS_ASSERT(!rt->gcRunning);
 
 #ifdef JS_THREADSAFE
         rt->gcHelperThread.waitBackgroundSweepEnd();
 #endif
         JS_UNLOCK_GC(rt);
 
         if (last) {
-#ifdef JS_THREADSAFE
-            /*
-             * If this thread is not in a request already, begin one now so
-             * that we wait for any racing GC started on a not-last context to
-             * finish, before we plow ahead and unpin atoms.
-             */
-            if (cx->runtime->requestDepth == 0)
-                JS_BeginRequest(cx);
-#endif
-
             /*
              * Dump remaining type inference results first. This printing
              * depends on atoms still existing.
              */
             {
                 AutoLockGC lock(rt);
                 for (CompartmentsIter c(rt); !c.done(); c.next())
                     c->types.print(cx, false);
@@ -303,37 +283,25 @@ js_DestroyContext(JSContext *cx, JSDestr
 
             /* Unpin all common atoms before final GC. */
             js_FinishCommonAtoms(cx);
 
             /* Clear debugging state to remove GC roots. */
             for (CompartmentsIter c(rt); !c.done(); c.next())
                 c->clearTraps(cx);
             JS_ClearAllWatchPoints(cx);
-        }
 
-#ifdef JS_THREADSAFE
-        /* Destroying a context implicitly calls JS_EndRequest(). */
-        while (cx->outstandingRequests != 0)
-            JS_EndRequest(cx);
-#endif
-
-        if (last) {
             js_GC(cx, NULL, GC_NORMAL, gcreason::LAST_CONTEXT);
 
-            /* Take the runtime down, now that it has no contexts or atoms. */
-            JS_LOCK_GC(rt);
-        } else {
-            if (mode == JSDCM_FORCE_GC)
-                js_GC(cx, NULL, GC_NORMAL, gcreason::DESTROY_CONTEXT);
-            else if (mode == JSDCM_MAYBE_GC)
-                JS_MaybeGC(cx);
-
-            JS_LOCK_GC(rt);
+        } else if (mode == JSDCM_FORCE_GC) {
+            js_GC(cx, NULL, GC_NORMAL, gcreason::DESTROY_CONTEXT);
+        } else if (mode == JSDCM_MAYBE_GC) {
+            JS_MaybeGC(cx);
         }
+        JS_LOCK_GC(rt);
     }
 #ifdef JS_THREADSAFE
     rt->gcHelperThread.waitBackgroundSweepEnd();
 #endif
     JS_UNLOCK_GC(rt);
     Foreground::delete_(cx);
 }
 
@@ -347,31 +315,16 @@ js_ContextIterator(JSRuntime *rt, JSBool
         lockIf.construct(rt);
     cx = JSContext::fromLinkField(cx ? cx->link.next : rt->contextList.next);
     if (&cx->link == &rt->contextList)
         cx = NULL;
     *iterp = cx;
     return cx;
 }
 
-JS_FRIEND_API(JSContext *)
-js_NextActiveContext(JSRuntime *rt, JSContext *cx)
-{
-    JSContext *iter = cx;
-#ifdef JS_THREADSAFE
-    while ((cx = js_ContextIterator(rt, JS_FALSE, &iter)) != NULL) {
-        if (cx->outstandingRequests && cx->runtime->requestDepth)
-            break;
-    }
-    return cx;
-#else
-    return js_ContextIterator(rt, JS_FALSE, &iter);
-#endif
-}
-
 namespace js {
 
 bool
 AutoResolving::alreadyStartedSlow() const
 {
     JS_ASSERT(link);
     AutoResolving *cursor = link;
     do {
--- a/js/src/jscntxt.h
+++ b/js/src/jscntxt.h
@@ -448,17 +448,16 @@ struct JSRuntime
     JSCList             debuggerList;
 
     /* Client opaque pointers */
     void                *data;
 
 #ifdef JS_THREADSAFE
     /* These combine to interlock the GC and new requests. */
     PRLock              *gcLock;
-    uint32_t            requestCount;
 
     js::GCHelperThread  gcHelperThread;
 #endif /* JS_THREADSAFE */
 
     uint32_t            debuggerMutations;
 
     /*
      * Security callbacks set on the runtime are used by each context unless
@@ -1359,24 +1358,16 @@ js_DestroyContext(JSContext *cx, JSDestr
 
 /*
  * If unlocked, acquire and release rt->gcLock around *iterp update; otherwise
  * the caller must be holding rt->gcLock.
  */
 extern JSContext *
 js_ContextIterator(JSRuntime *rt, JSBool unlocked, JSContext **iterp);
 
-/*
- * Iterate through contexts with active requests. The caller must be holding
- * rt->gcLock in case of a thread-safe build, or otherwise guarantee that the
- * context list is not alternated asynchroniously.
- */
-extern JS_FRIEND_API(JSContext *)
-js_NextActiveContext(JSRuntime *, JSContext *);
-
 #ifdef va_start
 extern JSBool
 js_ReportErrorVA(JSContext *cx, uintN flags, const char *format, va_list ap);
 
 extern JSBool
 js_ReportErrorNumberVA(JSContext *cx, uintN flags, JSErrorCallback callback,
                        void *userRef, const uintN errorNumber,
                        JSBool charArgs, va_list ap);
--- a/js/xpconnect/shell/xpcshell.cpp
+++ b/js/xpconnect/shell/xpcshell.cpp
@@ -2007,16 +2007,17 @@ main(int argc, char **argv, char **envp)
             JS_ClearScope(cx, glob);
             JS_GC(cx);
             JSContext *oldcx;
             cxstack->Pop(&oldcx);
             NS_ASSERTION(oldcx == cx, "JS thread context push/pop mismatch");
             cxstack = nsnull;
             JS_GC(cx);
         } //this scopes the JSAutoCrossCompartmentCall
+        JS_EndRequest(cx);
         JS_DestroyContext(cx);
     } // this scopes the nsCOMPtrs
 
     if (!XRE_ShutdownTestShell())
         NS_ERROR("problem shutting down testshell");
 
 #ifdef MOZ_CRASHREPORTER
     // Get the crashreporter service while XPCOM is still active.