Bug 1376507 - Handle a list of contexts instead of a single context. r=billm
☠☠ backed out by d4cea477b7e4 ☠ ☠
authorBlake Kaplan <mrbkap@gmail.com>
Mon, 28 Aug 2017 16:05:11 -0700
changeset 378142 266611b269cc876858346826d178a1e8fef0e0d4
parent 378141 04ecce0d1392376ddc40f1dd6655bbbc90bc6a8a
child 378143 d7cae06749f131a1fdffed1926fe526f7cfc89ee
push id94412
push userarchaeopteryx@coole-files.de
push dateFri, 01 Sep 2017 08:46:09 +0000
treeherdermozilla-inbound@d56571d7f1be [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbillm
bugs1376507
milestone57.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1376507 - Handle a list of contexts instead of a single context. r=billm This might be prematurely optimized as it uses two lists (one list of active contexts and one list of inactive contexts) but I was really attracted by the idea of being able to answer questions like "is any context active" by only looking at a single context and not having to iterate the whole list every time we needed to do anything. It is really important that nobody touches any of the timestamps (or the mActive member) outside of the Watchdog lock. I thought about trying to encapsulate that data in its own class, but that felt like overkill. Let me know if you disagree. There are still a couple of uses of XPCJSContext::Get that probably need to be stamped out, but I think doing so will depend on the details of how we map JSContexts to XPCJSContext (and XPCJSRuntimes). I think that should wait for a separate bug. MozReview-Commit-ID: 9UZlh7Jutne
js/xpconnect/src/XPCJSContext.cpp
js/xpconnect/src/xpcprivate.h
--- a/js/xpconnect/src/XPCJSContext.cpp
+++ b/js/xpconnect/src/XPCJSContext.cpp
@@ -74,20 +74,23 @@ using namespace mozilla;
 using namespace xpc;
 using namespace JS;
 using mozilla::dom::PerThreadAtomCache;
 using mozilla::dom::AutoEntryScript;
 
 static void WatchdogMain(void* arg);
 class Watchdog;
 class WatchdogManager;
-class AutoLockWatchdog {
+class MOZ_RAII AutoLockWatchdog final
+{
+    MOZ_DECL_USE_GUARD_OBJECT_NOTIFIER
     Watchdog* const mWatchdog;
+
   public:
-    explicit AutoLockWatchdog(Watchdog* aWatchdog);
+    explicit AutoLockWatchdog(Watchdog* aWatchdog MOZ_GUARD_OBJECT_NOTIFIER_PARAM);
     ~AutoLockWatchdog();
 };
 
 class Watchdog
 {
   public:
     explicit Watchdog(WatchdogManager* aManager)
       : mManager(aManager)
@@ -227,24 +230,21 @@ class Watchdog
 #define PREF_MAX_SCRIPT_RUN_TIME_CHROME "dom.max_chrome_script_run_time"
 #define PREF_MAX_SCRIPT_RUN_TIME_EXT_CONTENT "dom.max_ext_content_script_run_time"
 
 class WatchdogManager : public nsIObserver
 {
   public:
 
     NS_DECL_ISUPPORTS
-    explicit WatchdogManager(XPCJSContext* aContext) : mContext(aContext)
+    explicit WatchdogManager()
     {
         // All the timestamps start at zero.
         PodArrayZero(mTimestamps);
 
-        // Enable the watchdog, if appropriate.
-        RefreshWatchdog();
-
         // Register ourselves as an observer to get updates on the pref.
         mozilla::Preferences::AddStrongObserver(this, "dom.use_watchdog");
         mozilla::Preferences::AddStrongObserver(this, PREF_MAX_SCRIPT_RUN_TIME_CONTENT);
         mozilla::Preferences::AddStrongObserver(this, PREF_MAX_SCRIPT_RUN_TIME_CHROME);
         mozilla::Preferences::AddStrongObserver(this, PREF_MAX_SCRIPT_RUN_TIME_EXT_CONTENT);
     }
 
   protected:
@@ -269,81 +269,107 @@ class WatchdogManager : public nsIObserv
 
     NS_IMETHOD Observe(nsISupports* aSubject, const char* aTopic,
                        const char16_t* aData) override
     {
         RefreshWatchdog();
         return NS_OK;
     }
 
+    void
+    RegisterContext(XPCJSContext* aContext)
+    {
+        MOZ_ASSERT(NS_IsMainThread());
+        AutoLockWatchdog lock(mWatchdog);
+
+        if (aContext->mActive == XPCJSContext::CONTEXT_ACTIVE) {
+            mActiveContexts.insertBack(aContext);
+        } else {
+            mInactiveContexts.insertBack(aContext);
+        }
+
+        // Enable the watchdog, if appropriate.
+        RefreshWatchdog();
+    }
+
+    void
+    UnregisterContext(XPCJSContext* aContext)
+    {
+        MOZ_ASSERT(NS_IsMainThread());
+        AutoLockWatchdog lock(mWatchdog);
+
+        // aContext must be in one of our two lists, simply remove it.
+        aContext->LinkedListElement<XPCJSContext>::remove();
+    }
+
     // Context statistics. These live on the watchdog manager, are written
     // from the main thread, and are read from the watchdog thread (holding
     // the lock in each case).
     void RecordContextActivity(XPCJSContext* aContext, bool active)
     {
         // The watchdog reads this state, so acquire the lock before writing it.
         MOZ_ASSERT(NS_IsMainThread());
-        Maybe<AutoLockWatchdog> lock;
-        if (mWatchdog)
-            lock.emplace(mWatchdog);
+        AutoLockWatchdog lock(mWatchdog);
 
         // Write state.
         aContext->mLastStateChange = PR_Now();
         aContext->mActive = active ? XPCJSContext::CONTEXT_ACTIVE :
             XPCJSContext::CONTEXT_INACTIVE;
+        UpdateContextLists(aContext);
 
         // The watchdog may be hibernating, waiting for the context to go
         // active. Wake it up if necessary.
         if (active && mWatchdog && mWatchdog->Hibernating())
             mWatchdog->WakeUp();
     }
-    bool IsContextActive() { return mContext->mActive == XPCJSContext::CONTEXT_ACTIVE; }
-    PRTime TimeSinceLastContextStateChange()
+
+    bool IsAnyContextActive()
+    {
+        return !mActiveContexts.isEmpty();
+    }
+    PRTime TimeSinceLastActiveContext()
     {
-        // Called on the watchdog thread with the lock held.
+        // Must be called on the watchdog thread with the lock held.
         MOZ_ASSERT(!NS_IsMainThread());
-        return PR_Now() - GetContextTimestamp(mContext);
+        if (mWatchdog)
+            PR_ASSERT_CURRENT_THREAD_OWNS_LOCK(mWatchdog->GetLock());
+        MOZ_ASSERT(mActiveContexts.isEmpty());
+        MOZ_ASSERT(!mInactiveContexts.isEmpty());
+
+        // We store inactive contexts with the most recently added inactive
+        // context at the end of the list.
+        return PR_Now() - mInactiveContexts.getLast()->mLastStateChange;
     }
 
-    // Note - Because of the context activity timestamp, these are read and
-    // written from both threads.
     void RecordTimestamp(WatchdogTimestampCategory aCategory)
     {
-        // Calls to get a context state change must include a context.
+        // Must be called on the watchdog thread with the lock held.
+        MOZ_ASSERT(!NS_IsMainThread());
+        if (mWatchdog)
+            PR_ASSERT_CURRENT_THREAD_OWNS_LOCK(mWatchdog->GetLock());
         MOZ_ASSERT(aCategory != TimestampContextStateChange,
                    "Use RecordContextActivity to update this");
 
-        // The watchdog thread always holds the lock when it runs.
-        Maybe<AutoLockWatchdog> maybeLock;
-        if (NS_IsMainThread() && mWatchdog)
-            maybeLock.emplace(mWatchdog);
-
         mTimestamps[aCategory] = PR_Now();
     }
 
-    PRTime GetContextTimestamp(XPCJSContext* aContext)
+    PRTime GetContextTimestamp(XPCJSContext* aContext,
+                               const AutoLockWatchdog& aProofOfLock)
     {
-        Maybe<AutoLockWatchdog> maybeLock;
-        if (NS_IsMainThread() && mWatchdog)
-            maybeLock.emplace(mWatchdog);
         return aContext->mLastStateChange;
     }
 
-    PRTime GetTimestamp(WatchdogTimestampCategory aCategory)
+    PRTime GetTimestamp(WatchdogTimestampCategory aCategory,
+                        const AutoLockWatchdog& aProofOfLock)
     {
         MOZ_ASSERT(aCategory != TimestampContextStateChange,
                    "Use GetContextTimestamp to retrieve this");
-        // The watchdog thread always holds the lock when it runs.
-        Maybe<AutoLockWatchdog> maybeLock;
-        if (NS_IsMainThread() && mWatchdog)
-            maybeLock.emplace(mWatchdog);
         return mTimestamps[aCategory];
     }
 
-    XPCJSContext* Context() { return mContext; }
     Watchdog* GetWatchdog() { return mWatchdog; }
 
     void RefreshWatchdog()
     {
         bool wantWatchdog = Preferences::GetBool("dom.use_watchdog", true);
         if (wantWatchdog != !!mWatchdog) {
             if (wantWatchdog)
                 StartWatchdog();
@@ -374,34 +400,72 @@ class WatchdogManager : public nsIObserv
 
     void StopWatchdog()
     {
         MOZ_ASSERT(mWatchdog);
         mWatchdog->Shutdown();
         mWatchdog = nullptr;
     }
 
+    template<class Callback>
+    void ForAllActiveContexts(Callback&& aCallback)
+    {
+        // This function must be called on the watchdog thread with the lock held.
+        MOZ_ASSERT(!NS_IsMainThread());
+        if (mWatchdog)
+            PR_ASSERT_CURRENT_THREAD_OWNS_LOCK(mWatchdog->GetLock());
+
+        for (auto* context = mActiveContexts.getFirst(); context;
+             context = context->LinkedListElement<XPCJSContext>::getNext()) {
+            if (!aCallback(context)) {
+                return;
+            }
+        }
+    }
+
   private:
-    XPCJSContext* mContext;
+    void UpdateContextLists(XPCJSContext* aContext)
+    {
+        // Given aContext whose activity state or timestamp has just changed,
+        // put it back in the proper position in the proper list.
+        aContext->LinkedListElement<XPCJSContext>::remove();
+        auto& list = aContext->mActive == XPCJSContext::CONTEXT_ACTIVE ?
+            mActiveContexts : mInactiveContexts;
+
+        // Either the new list is empty or aContext must be more recent than
+        // the existing last element.
+        MOZ_ASSERT_IF(!list.isEmpty(),
+                      list.getLast()->mLastStateChange < aContext->mLastStateChange);
+        list.insertBack(aContext);
+    }
+
+    LinkedList<XPCJSContext> mActiveContexts;
+    LinkedList<XPCJSContext> mInactiveContexts;
     nsAutoPtr<Watchdog> mWatchdog;
 
     // We store ContextStateChange on the contexts themselves.
     PRTime mTimestamps[kWatchdogTimestampCategoryCount - 1];
 };
 
 NS_IMPL_ISUPPORTS(WatchdogManager, nsIObserver)
 
-AutoLockWatchdog::AutoLockWatchdog(Watchdog* aWatchdog) : mWatchdog(aWatchdog)
+AutoLockWatchdog::AutoLockWatchdog(Watchdog* aWatchdog MOZ_GUARD_OBJECT_NOTIFIER_PARAM_IN_IMPL)
+  : mWatchdog(aWatchdog)
 {
-    PR_Lock(mWatchdog->GetLock());
+    MOZ_GUARD_OBJECT_NOTIFIER_INIT;
+    if (mWatchdog) {
+        PR_Lock(mWatchdog->GetLock());
+    }
 }
 
 AutoLockWatchdog::~AutoLockWatchdog()
 {
-    PR_Unlock(mWatchdog->GetLock());
+    if (mWatchdog) {
+        PR_Unlock(mWatchdog->GetLock());
+    }
 }
 
 static void
 WatchdogMain(void* arg)
 {
     mozilla::AutoProfilerRegisterThread registerThread("JS Watchdog");
     NS_SetCurrentThreadName("JS Watchdog");
 
@@ -410,18 +474,18 @@ WatchdogMain(void* arg)
 
     // Lock lasts until we return
     AutoLockWatchdog lock(self);
 
     MOZ_ASSERT(self->Initialized());
     MOZ_ASSERT(!self->ShuttingDown());
     while (!self->ShuttingDown()) {
         // Sleep only 1 second if recently (or currently) active; otherwise, hibernate
-        if (manager->IsContextActive() ||
-            manager->TimeSinceLastContextStateChange() <= PRTime(2*PR_USEC_PER_SEC))
+        if (manager->IsAnyContextActive() ||
+            manager->TimeSinceLastActiveContext() <= PRTime(2*PR_USEC_PER_SEC))
         {
             self->Sleep(PR_TicksPerSecond());
         } else {
             manager->RecordTimestamp(TimestampWatchdogHibernateStart);
             self->Hibernate();
             manager->RecordTimestamp(TimestampWatchdogHibernateStop);
         }
 
@@ -437,39 +501,49 @@ WatchdogMain(void* arg)
         // invoke the interrupt callback after only half the timeout has
         // elapsed. The callback simply records the fact that it was called in
         // the mSlowScriptSecondHalf flag. Then we wait another (timeout/2)
         // seconds and invoke the callback again. This time around it sees
         // mSlowScriptSecondHalf is set and so it shows the slow script
         // dialog. If the computer is put to sleep during one of the (timeout/2)
         // periods, the script still has the other (timeout/2) seconds to
         // finish.
-        PRTime usecs = self->MinScriptRunTimeSeconds() * PR_USEC_PER_SEC / 2;
-        if (manager->IsContextActive() &&
-            manager->TimeSinceLastContextStateChange() >= usecs)
-        {
+        if (manager->IsAnyContextActive()) {
             bool debuggerAttached = false;
             nsCOMPtr<nsIDebug2> dbg = do_GetService("@mozilla.org/xpcom/debug;1");
             if (dbg)
                 dbg->GetIsDebuggerAttached(&debuggerAttached);
-            if (!debuggerAttached)
-                JS_RequestInterruptCallback(manager->Context()->Context());
+            if (debuggerAttached) {
+                // We won't be interrupting these scripts anyway.
+                continue;
+            }
+
+            PRTime usecs = self->MinScriptRunTimeSeconds() * PR_USEC_PER_SEC / 2;
+            manager->ForAllActiveContexts([usecs, manager, &lock](XPCJSContext* aContext) -> bool {
+                auto timediff = PR_Now() - manager->GetContextTimestamp(aContext, lock);
+                if (timediff > usecs) {
+                    JS_RequestInterruptCallback(aContext->Context());
+                    return true;
+                }
+                return false;
+            });
         }
     }
 
     // Tell the manager that we've shut down.
     self->Finished();
 }
 
 PRTime
 XPCJSContext::GetWatchdogTimestamp(WatchdogTimestampCategory aCategory)
 {
+    AutoLockWatchdog lock(mWatchdogManager->GetWatchdog());
     return aCategory == TimestampContextStateChange ?
-        mWatchdogManager->GetContextTimestamp(this) :
-        mWatchdogManager->GetTimestamp(aCategory);
+        mWatchdogManager->GetContextTimestamp(this, lock) :
+        mWatchdogManager->GetTimestamp(aCategory, lock);
 }
 
 void
 xpc::SimulateActivityCallback(bool aActive)
 {
     XPCJSContext::ActivityCallback(XPCJSContext::Get(), aActive);
 }
 
@@ -801,19 +875,24 @@ XPCJSContext::~XPCJSContext()
     js::SetActivityCallback(Context(), nullptr, nullptr);
 
     // Clear any pending exception.  It might be an XPCWrappedJS, and if we try
     // to destroy it later we will crash.
     SetPendingException(nullptr);
 
     xpc_DelocalizeContext(Context());
 
-    if (mWatchdogManager->GetWatchdog())
-        mWatchdogManager->StopWatchdog();
-    mWatchdogManager->Shutdown();
+    // If we're the last XPCJSContext around, clean up the watchdog manager.
+    mWatchdogManager->UnregisterContext(this);
+    if (--sInstanceCount == 0) {
+        if (mWatchdogManager->GetWatchdog())
+            mWatchdogManager->StopWatchdog();
+        mWatchdogManager->Shutdown();
+        sWatchdogInstance = nullptr;
+    }
 
     if (mCallContext)
         mCallContext->SystemIsBeingShutDown();
 
     auto rtPrivate = static_cast<PerThreadAtomCache*>(JS_GetContextPrivate(Context()));
     delete rtPrivate;
     JS_SetContextPrivate(Context(), nullptr);
 
@@ -822,25 +901,28 @@ XPCJSContext::~XPCJSContext()
     gTlsContext.set(nullptr);
 }
 
 XPCJSContext::XPCJSContext()
  : mCallContext(nullptr),
    mAutoRoots(nullptr),
    mResolveName(JSID_VOID),
    mResolvingWrapper(nullptr),
-   mWatchdogManager(new WatchdogManager(this)),
+   mWatchdogManager(GetWatchdogManager()),
    mSlowScriptSecondHalf(false),
    mTimeoutAccumulated(false),
    mPendingResult(NS_OK),
    mActive(CONTEXT_INACTIVE),
    mLastStateChange(PR_Now())
 {
     MOZ_COUNT_CTOR_INHERITED(XPCJSContext, CycleCollectedJSContext);
     MOZ_RELEASE_ASSERT(!gTlsContext.get());
+    MOZ_ASSERT(mWatchdogManager);
+    ++sInstanceCount;
+    mWatchdogManager->RegisterContext(this);
     gTlsContext.set(this);
 }
 
 /* static */ XPCJSContext*
 XPCJSContext::Get()
 {
     return gTlsContext.get();
 }
@@ -1021,16 +1103,37 @@ XPCJSContext::Initialize(XPCJSContext* a
 #ifdef FUZZING
     Preferences::RegisterCallback(ReloadPrefsCallback, "fuzzing.enabled", this);
 #endif
 
     return NS_OK;
 }
 
 // static
+uint32_t
+XPCJSContext::sInstanceCount;
+
+// static
+StaticRefPtr<WatchdogManager>
+XPCJSContext::sWatchdogInstance;
+
+// static
+WatchdogManager*
+XPCJSContext::GetWatchdogManager()
+{
+    if (sWatchdogInstance) {
+        return sWatchdogInstance;
+    }
+
+    MOZ_ASSERT(sInstanceCount == 0);
+    sWatchdogInstance = new WatchdogManager();
+    return sWatchdogInstance;
+}
+
+// static
 void
 XPCJSContext::InitTLS()
 {
     MOZ_RELEASE_ASSERT(gTlsContext.init());
 }
 
 // static
 XPCJSContext*
--- a/js/xpconnect/src/xpcprivate.h
+++ b/js/xpconnect/src/xpcprivate.h
@@ -390,16 +390,17 @@ public:
 #endif
     }
 
 private:
     mozilla::Maybe<StringType> mStrings[2];
 };
 
 class XPCJSContext final : public mozilla::CycleCollectedJSContext
+                         , public mozilla::LinkedListElement<XPCJSContext>
 {
 public:
     static void InitTLS();
     static XPCJSContext* NewXPCJSContext(XPCJSContext* aPrimaryContext);
     static XPCJSContext* Get();
 
     XPCJSRuntime* Runtime() const;
 
@@ -486,17 +487,22 @@ private:
 
     MOZ_IS_CLASS_INIT
     nsresult Initialize(XPCJSContext* aPrimaryContext);
 
     XPCCallContext*          mCallContext;
     AutoMarkingPtr*          mAutoRoots;
     jsid                     mResolveName;
     XPCWrappedNative*        mResolvingWrapper;
-    RefPtr<WatchdogManager>  mWatchdogManager;
+    WatchdogManager*         mWatchdogManager;
+
+    // Number of XPCJSContexts currently alive.
+    static uint32_t         sInstanceCount;
+    static mozilla::StaticRefPtr<WatchdogManager> sWatchdogInstance;
+    static WatchdogManager* GetWatchdogManager();
 
     // If we spend too much time running JS code in an event handler, then we
     // want to show the slow script UI. The timeout T is controlled by prefs. We
     // invoke the interrupt callback once after T/2 seconds and set
     // mSlowScriptSecondHalf to true. After another T/2 seconds, we invoke the
     // interrupt callback again. Since mSlowScriptSecondHalf is now true, it
     // shows the slow script UI. The reason we invoke the callback twice is to
     // ensure that putting the computer to sleep while running a script doesn't