Bug 1435022 - Inline the SetNeedsComposite function. r?sotaro draft
authorKartikaya Gupta <kgupta@mozilla.com>
Thu, 01 Feb 2018 16:28:53 -0500
changeset 750298 1f4e754143ecfd714ec5c424a860faaf4b0cd136
parent 750297 c4942304ce803e68c86a532fa6624980627f3c3c
child 750299 9130b90c157d27882fc1278c3a3918ce187621dc
push id97612
push userkgupta@mozilla.com
push dateThu, 01 Feb 2018 21:30:35 +0000
reviewerssotaro
bugs1435022
milestone60.0a1
Bug 1435022 - Inline the SetNeedsComposite function. r?sotaro SetNeedsComposite is only ever called from one place on the compositor thread, but it has a bunch of generic boilerplate to handle being called from any thread. If we inline it we don't need the extra boilerplate and it's much simpler to follow the code. MozReview-Commit-ID: E1AcMh80KsH
gfx/layers/ipc/CompositorVsyncScheduler.cpp
gfx/layers/ipc/CompositorVsyncScheduler.h
--- a/gfx/layers/ipc/CompositorVsyncScheduler.cpp
+++ b/gfx/layers/ipc/CompositorVsyncScheduler.cpp
@@ -77,18 +77,16 @@ CompositorVsyncScheduler::CompositorVsyn
   : mVsyncSchedulerOwner(aVsyncSchedulerOwner)
   , mLastCompose(TimeStamp::Now())
   , mIsObservingVsync(false)
   , mNeedsComposite(0)
   , mVsyncNotificationsSkipped(0)
   , mWidget(aWidget)
   , mCurrentCompositeTaskMonitor("CurrentCompositeTaskMonitor")
   , mCurrentCompositeTask(nullptr)
-  , mSetNeedsCompositeMonitor("SetNeedsCompositeMonitor")
-  , mSetNeedsCompositeTask(nullptr)
   , mCurrentVRListenerTaskMonitor("CurrentVRTaskMonitor")
   , mCurrentVRListenerTask(nullptr)
 {
   mVsyncObserver = new Observer(this);
 
   // mAsapScheduling is set on the main thread during init,
   // but is only accessed after on the compositor thread.
   mAsapScheduling = gfxPrefs::LayersCompositionFrameRate() == 0 ||
@@ -111,17 +109,17 @@ CompositorVsyncScheduler::Destroy()
   if (!mVsyncObserver) {
     // Destroy was already called on this object.
     return;
   }
   UnobserveVsync();
   mVsyncObserver->Destroy();
   mVsyncObserver = nullptr;
 
-  CancelCurrentSetNeedsCompositeTask();
+  mNeedsComposite = 0;
   CancelCurrentCompositeTask();
 }
 
 void
 CompositorVsyncScheduler::PostCompositeTask(TimeStamp aCompositeTimestamp)
 {
   MonitorAutoLock lock(mCurrentCompositeTaskMonitor);
   if (mCurrentCompositeTask == nullptr && CompositorThreadHolder::Loop()) {
@@ -168,64 +166,25 @@ CompositorVsyncScheduler::ScheduleCompos
     // 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 {
-    SetNeedsComposite();
-  }
-}
-
-void
-CompositorVsyncScheduler::CancelCurrentSetNeedsCompositeTask()
-{
-  MOZ_ASSERT(CompositorThreadHolder::IsInCompositorThread());
-  MonitorAutoLock lock(mSetNeedsCompositeMonitor);
-  if (mSetNeedsCompositeTask) {
-    mSetNeedsCompositeTask->Cancel();
-    mSetNeedsCompositeTask = nullptr;
-  }
-  mNeedsComposite = 0;
-}
-
-/**
- * TODO Potential performance heuristics:
- * If a composite takes 17 ms, do we composite ASAP or wait until next vsync?
- * If a layer transaction comes after vsync, do we composite ASAP or wait until
- * next vsync?
- * How many skipped vsync events until we stop listening to vsync events?
- */
-void
-CompositorVsyncScheduler::SetNeedsComposite()
-{
-  if (!CompositorThreadHolder::IsInCompositorThread()) {
-    MonitorAutoLock lock(mSetNeedsCompositeMonitor);
-    RefPtr<CancelableRunnable> task = NewCancelableRunnableMethod(
-      "layers::CompositorVsyncScheduler::SetNeedsComposite",
-      this,
-      &CompositorVsyncScheduler::SetNeedsComposite);
-    mSetNeedsCompositeTask = task;
-    ScheduleTask(task.forget());
-    return;
-  } else {
-    MonitorAutoLock lock(mSetNeedsCompositeMonitor);
-    mSetNeedsCompositeTask = nullptr;
-  }
-
-  mNeedsComposite++;
-  if (!mIsObservingVsync && mNeedsComposite) {
-    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());
+    mNeedsComposite++;
+    if (!mIsObservingVsync && mNeedsComposite) {
+      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());
+    }
   }
 }
 
 bool
 CompositorVsyncScheduler::NotifyVsync(TimeStamp aVsyncTimestamp)
 {
   // Called from the vsync dispatch thread. When in the GPU Process, that's
   // the same as the compositor thread.
--- a/gfx/layers/ipc/CompositorVsyncScheduler.h
+++ b/gfx/layers/ipc/CompositorVsyncScheduler.h
@@ -44,17 +44,16 @@ class CompositorVsyncScheduler
 {
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(CompositorVsyncScheduler)
 
 public:
   explicit CompositorVsyncScheduler(CompositorVsyncSchedulerOwner* aVsyncSchedulerOwner,
                                     widget::CompositorWidget* aWidget);
 
   bool NotifyVsync(TimeStamp aVsyncTimestamp);
-  void SetNeedsComposite();
 
   /**
    * Do cleanup. This must be called on the compositor thread.
    */
   void Destroy();
 
   void ScheduleComposition();
   void CancelCurrentCompositeTask();
@@ -88,17 +87,16 @@ private:
   // such a task already queued. Can be called from any thread.
   void PostVRTask(TimeStamp aTimestamp);
 
   void NotifyCompositeTaskExecuted();
   void ObserveVsync();
   void UnobserveVsync();
   void DispatchTouchEvents(TimeStamp aVsyncTimestamp);
   void DispatchVREvents(TimeStamp aVsyncTimestamp);
-  void CancelCurrentSetNeedsCompositeTask();
 
   class Observer final : public VsyncObserver
   {
   public:
     explicit Observer(CompositorVsyncScheduler* aOwner);
     virtual bool NotifyVsync(TimeStamp aVsyncTimestamp) override;
     void Destroy();
   private:
@@ -121,19 +119,16 @@ private:
   uint32_t mNeedsComposite;
   int32_t mVsyncNotificationsSkipped;
   widget::CompositorWidget* mWidget;
   RefPtr<CompositorVsyncScheduler::Observer> mVsyncObserver;
 
   mozilla::Monitor mCurrentCompositeTaskMonitor;
   RefPtr<CancelableRunnable> mCurrentCompositeTask;
 
-  mozilla::Monitor mSetNeedsCompositeMonitor;
-  RefPtr<CancelableRunnable> mSetNeedsCompositeTask;
-
   mozilla::Monitor mCurrentVRListenerTaskMonitor;
   RefPtr<Runnable> mCurrentVRListenerTask;
 };
 
 } // namespace layers
 } // namespace mozilla
 
 #endif // mozilla_layers_CompositorVsyncScheduler_h