Bug 1484173 - Replace the mNeedsComposite counter with a timestamp. r=sotaro
authorKartikaya Gupta <kgupta@mozilla.com>
Tue, 16 Oct 2018 05:36:14 +0000
changeset 441431 23ac21b643c2f398affbdfc7a59f97af70733a9c
parent 441430 b032c73f6f69d6e01fd86125ec1ce183061bdb78
child 441432 4fa33210cbc490a1dc353e2288b507138de81772
push id34864
push usercsabou@mozilla.com
push dateTue, 16 Oct 2018 16:24:06 +0000
treeherdermozilla-central@f2e35ed6a692 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssotaro
bugs1484173
milestone64.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 1484173 - Replace the mNeedsComposite counter with a timestamp. r=sotaro The mNeedsComposite counter was used to force a composite immediately if the scheduler received a number of composite requests without actually getting a vsync. It was necessary on Fennec because of main-thread contention. However it was wrong because it assumes only a single composite gets requested per vsync interval, which is not true. Instead of using a counter this patch uses a timestamp to ensure that we only force the vsync after two vsync intervals have elapsed. Depends on D8765 Differential Revision: https://phabricator.services.mozilla.com/D8766
gfx/layers/ipc/CompositorVsyncScheduler.cpp
gfx/layers/ipc/CompositorVsyncScheduler.h
--- a/gfx/layers/ipc/CompositorVsyncScheduler.cpp
+++ b/gfx/layers/ipc/CompositorVsyncScheduler.cpp
@@ -71,17 +71,16 @@ CompositorVsyncScheduler::Observer::Dest
   mOwner = nullptr;
 }
 
 CompositorVsyncScheduler::CompositorVsyncScheduler(CompositorVsyncSchedulerOwner* aVsyncSchedulerOwner,
                                                    widget::CompositorWidget* aWidget)
   : mVsyncSchedulerOwner(aVsyncSchedulerOwner)
   , mLastCompose(TimeStamp::Now())
   , mIsObservingVsync(false)
-  , mNeedsComposite(0)
   , mVsyncNotificationsSkipped(0)
   , mWidget(aWidget)
   , mCurrentCompositeTaskMonitor("CurrentCompositeTaskMonitor")
   , mCurrentCompositeTask(nullptr)
   , mCurrentVRTaskMonitor("CurrentVRTaskMonitor")
   , mCurrentVRTask(nullptr)
 {
   mVsyncObserver = new Observer(this);
@@ -109,17 +108,17 @@ CompositorVsyncScheduler::Destroy()
   if (!mVsyncObserver) {
     // Destroy was already called on this object.
     return;
   }
   UnobserveVsync();
   mVsyncObserver->Destroy();
   mVsyncObserver = nullptr;
 
-  mNeedsComposite = 0;
+  mCompositeRequestedAt = TimeStamp();
   CancelCurrentCompositeTask();
 }
 
 void
 CompositorVsyncScheduler::PostCompositeTask(TimeStamp aCompositeTimestamp)
 {
   MonitorAutoLock lock(mCurrentCompositeTaskMonitor);
   if (mCurrentCompositeTask == nullptr && CompositorThreadHolder::Loop()) {
@@ -157,28 +156,31 @@ CompositorVsyncScheduler::ScheduleCompos
     return;
   }
 
   if (mAsapScheduling) {
     // Used only for performance testing purposes, and when recording/replaying
     // to ensure that graphics are up to date.
     PostCompositeTask(TimeStamp::Now());
 #ifdef MOZ_WIDGET_ANDROID
-  } else if (mNeedsComposite >= 2 && mIsObservingVsync) {
-    // uh-oh, we already requested a composite at least twice so far, and a
+  } else if (mIsObservingVsync && mCompositeRequestedAt &&
+      (TimeStamp::Now() - mCompositeRequestedAt) >= mVsyncSchedulerOwner->GetVsyncInterval() * 2) {
+    // uh-oh, we already requested a composite at least two vsyncs ago, and a
     // composite hasn't happened yet. It is possible that the vsync observation
     // is blocked on the main thread, so let's just composite ASAP and not
     // wait for the vsync. Note that this should only ever happen on Fennec
     // because there content runs in the same process as the compositor, and so
     // content can actually block the main thread in this process.
     PostCompositeTask(TimeStamp::Now());
 #endif
   } else {
-    mNeedsComposite++;
-    if (!mIsObservingVsync && mNeedsComposite) {
+    if (!mCompositeRequestedAt) {
+      mCompositeRequestedAt = TimeStamp::Now();
+    }
+    if (!mIsObservingVsync && mCompositeRequestedAt) {
       ObserveVsync();
       // Starting to observe vsync is an async operation that goes
       // through the main thread of the UI process. It's possible that
       // we're blocking there waiting on a composite, so schedule an initial
       // one now to get things started.
       PostCompositeTask(TimeStamp::Now());
     }
   }
@@ -231,18 +233,18 @@ CompositorVsyncScheduler::Composite(Time
     if (mVsyncSchedulerOwner->IsPendingComposite()) {
       // If previous composite is still on going, finish it and wait for the
       // next vsync.
       mVsyncSchedulerOwner->FinishPendingComposite();
       return;
     }
   }
 
-  if (mNeedsComposite || mAsapScheduling) {
-    mNeedsComposite = 0;
+  if (mCompositeRequestedAt || mAsapScheduling) {
+    mCompositeRequestedAt = TimeStamp();
     mLastCompose = aVsyncTimestamp;
 
     // Tell the owner to do a composite
     mVsyncSchedulerOwner->CompositeToTarget(nullptr, nullptr);
 
     mVsyncNotificationsSkipped = 0;
 
     TimeDuration compositeFrameTotal = TimeStamp::Now() - aVsyncTimestamp;
@@ -257,17 +259,17 @@ void
 CompositorVsyncScheduler::ForceComposeToTarget(gfx::DrawTarget* aTarget, const IntRect* aRect)
 {
   MOZ_ASSERT(CompositorThreadHolder::IsInCompositorThread());
 
   /**
    * bug 1138502 - There are cases such as during long-running window resizing
    * events where we receive many force-composites. We also continue to get
    * vsync notifications. Because the force-composites trigger compositing and
-   * clear the mNeedsComposite counter, the vsync notifications will not need
+   * clear the mCompositeRequestedAt timestamp, the vsync notifications will not need
    * to do anything and so will increment the mVsyncNotificationsSkipped counter
    * to indicate the vsync was ignored. If this happens enough times, we will
    * disable listening for vsync entirely. On the next force-composite we will
    * enable listening for vsync again, and continued force-composites and vsyncs
    * will cause oscillation between observing vsync and not.
    * On some platforms, enabling/disabling vsync is not free and this
    * oscillating behavior causes a performance hit. In order to avoid this
    * problem, we reset the mVsyncNotificationsSkipped counter to keep vsync
@@ -279,24 +281,24 @@ CompositorVsyncScheduler::ForceComposeTo
   MOZ_ASSERT(mVsyncSchedulerOwner);
   mVsyncSchedulerOwner->CompositeToTarget(aTarget, aRect);
 }
 
 bool
 CompositorVsyncScheduler::NeedsComposite()
 {
   MOZ_ASSERT(CompositorThreadHolder::IsInCompositorThread());
-  return mNeedsComposite;
+  return (bool)mCompositeRequestedAt;
 }
 
 bool
 CompositorVsyncScheduler::FlushPendingComposite()
 {
   MOZ_ASSERT(CompositorThreadHolder::IsInCompositorThread());
-  if (mNeedsComposite) {
+  if (mCompositeRequestedAt) {
     CancelCurrentCompositeTask();
     ForceComposeToTarget(nullptr, nullptr);
     return true;
   }
   return false;
 }
 
 void
--- a/gfx/layers/ipc/CompositorVsyncScheduler.h
+++ b/gfx/layers/ipc/CompositorVsyncScheduler.h
@@ -136,17 +136,17 @@ private:
     CompositorVsyncScheduler* mOwner;
   };
 
   CompositorVsyncSchedulerOwner* mVsyncSchedulerOwner;
   TimeStamp mLastCompose;
 
   bool mAsapScheduling;
   bool mIsObservingVsync;
-  uint32_t mNeedsComposite;
+  TimeStamp mCompositeRequestedAt;
   int32_t mVsyncNotificationsSkipped;
   widget::CompositorWidget* mWidget;
   RefPtr<CompositorVsyncScheduler::Observer> mVsyncObserver;
 
   mozilla::Monitor mCurrentCompositeTaskMonitor;
   RefPtr<CancelableRunnable> mCurrentCompositeTask;
 
   mozilla::Monitor mCurrentVRTaskMonitor;