Bug 1313989 - Remove MutexAutoUnlock in nsThread::GetIdleEvent. r=froydnj,mattwoodrow
authorAndreas Farre <farre@mozilla.com>
Thu, 10 Nov 2016 10:01:51 +0100
changeset 322689 188b07c3d90a5028da5cdce2be6132d51c038019
parent 322688 343cef5aeb2b1ac475e4e505d895fafdfc1e9b58
child 322690 eca45b922a55f51690937e50c3d93e4fe67b92be
push id30960
push userkwierso@gmail.com
push dateThu, 17 Nov 2016 00:42:57 +0000
treeherdermozilla-central@830ce59e0a13 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj, mattwoodrow
bugs1313989
milestone53.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 1313989 - Remove MutexAutoUnlock in nsThread::GetIdleEvent. r=froydnj,mattwoodrow Rewrite VsyncRefreshDriverTimer::GetTimerRate to always use the cached value of the vsync rate in VsyncChild to avoid processing events on the main thread. Since VsyncChild::GetTimerRate is called in VsyncRefreshDriverTimer's constructor, that cached value is bound to be set soon. This should make the period of time we need to guess in VsyncRefreshDriverTimer::GetTimerRate very short. MozReview-Commit-ID: 1bnHNXAP8jY
layout/base/nsRefreshDriver.cpp
layout/ipc/VsyncChild.cpp
layout/ipc/VsyncChild.h
xpcom/threads/nsThread.cpp
--- a/layout/base/nsRefreshDriver.cpp
+++ b/layout/base/nsRefreshDriver.cpp
@@ -442,17 +442,23 @@ public:
 
   TimeDuration GetTimerRate() override
   {
     if (mVsyncRate != TimeDuration::Forever()) {
       return mVsyncRate;
     }
 
     if (mVsyncChild) {
-      mVsyncRate = mVsyncChild->GetVsyncRate();
+      // VsyncChild::VsyncRate() is a simple getter for the cached
+      // hardware vsync rate. We depend on that
+      // VsyncChild::GetVsyncRate() being called in the constructor
+      // will result in a response with the actual vsync rate sooner
+      // or later. Until that happens VsyncChild::VsyncRate() returns
+      // TimeDuration::Forever() and we have to guess below.
+      mVsyncRate = mVsyncChild->VsyncRate();
     }
 
     // If hardware queries fail / are unsupported, we have to just guess.
     return mVsyncRate != TimeDuration::Forever()
              ? mVsyncRate
              : TimeDuration::FromMilliseconds(1000.0 / 60.0);
   }
 
--- a/layout/ipc/VsyncChild.cpp
+++ b/layout/ipc/VsyncChild.cpp
@@ -78,16 +78,22 @@ VsyncChild::GetVsyncRate()
 {
   if (mVsyncRate == TimeDuration::Forever()) {
     PVsyncChild::SendRequestVsyncRate();
   }
 
   return mVsyncRate;
 }
 
+TimeDuration
+VsyncChild::VsyncRate()
+{
+  return mVsyncRate;
+}
+
 mozilla::ipc::IPCResult
 VsyncChild::RecvVsyncRate(const float& aVsyncRate)
 {
   mVsyncRate = TimeDuration::FromMilliseconds(aVsyncRate);
   return IPC_OK();
 }
 
 } // namespace layout
--- a/layout/ipc/VsyncChild.h
+++ b/layout/ipc/VsyncChild.h
@@ -33,17 +33,25 @@ class VsyncChild final : public PVsyncCh
 public:
   // Hide the SendObserve/SendUnobserve in PVsyncChild. We add an flag
   // mObservingVsync to handle the race problem of unobserving vsync event.
   bool SendObserve();
   bool SendUnobserve();
 
   // Bind a VsyncObserver into VsyncChild after ipc channel connected.
   void SetVsyncObserver(VsyncObserver* aVsyncObserver);
+  // GetVsyncRate is a getter for mVsyncRate which sends a requests to
+  // VsyncParent to retreive the hardware vsync rate if mVsyncRate
+  // hasn't already been set.
   TimeDuration GetVsyncRate();
+  // VsyncRate is a getter for mVsyncRate which always returns
+  // mVsyncRate directly, potentially returning
+  // TimeDuration::Forever() if mVsyncRate hasn't been set by calling
+  // GetVsyncRate.
+  TimeDuration VsyncRate();
 
 private:
   VsyncChild();
   virtual ~VsyncChild();
 
   virtual mozilla::ipc::IPCResult RecvNotify(const TimeStamp& aVsyncTimestamp) override;
   virtual mozilla::ipc::IPCResult RecvVsyncRate(const float& aVsyncRate) override;
   virtual void ActorDestroy(ActorDestroyReason aActorDestroyReason) override;
--- a/xpcom/threads/nsThread.cpp
+++ b/xpcom/threads/nsThread.cpp
@@ -1093,20 +1093,17 @@ void canary_alarm_handler(int signum)
 
 void
 nsThread::GetIdleEvent(nsIRunnable** aEvent, MutexAutoLock& aProofOfLock)
 {
   MOZ_ASSERT(PR_GetCurrentThread() == mThread);
   MOZ_ASSERT(aEvent);
 
   TimeStamp idleDeadline;
-  {
-    MutexAutoUnlock unlock(mLock);
-    mIdlePeriod->GetIdlePeriodHint(&idleDeadline);
-  }
+  mIdlePeriod->GetIdlePeriodHint(&idleDeadline);
 
   if (!idleDeadline || idleDeadline < TimeStamp::Now()) {
     aEvent = nullptr;
     return;
   }
 
   mIdleEvents.GetEvent(false, aEvent, aProofOfLock);