Bug 1429665: ipc::mscom::SpinEvent: Only spin for a short time, falling back to an event thereafter. r?aklotz draft
authorJames Teh <jteh@mozilla.com>
Mon, 15 Jan 2018 14:51:19 +1000
changeset 778233 52a1f17232690811be992e219e787582e1335870
parent 778203 7b40283bf1c7a2a3e6a8a5d00156a2f506ff465b
push id105445
push userbmo:jteh@mozilla.com
push dateFri, 06 Apr 2018 03:30:22 +0000
reviewersaklotz
bugs1429665
milestone61.0a1
Bug 1429665: ipc::mscom::SpinEvent: Only spin for a short time, falling back to an event thereafter. r?aklotz We want to spin for faster response, but we only want to spin for a very short time. If we're waiting for a while, we don't want to be burning CPU for the entire time. Therefore, only spin for 30 ms, then fall back to waiting on an event. MozReview-Commit-ID: ErAIwpsIqYz
ipc/mscom/SpinEvent.cpp
ipc/mscom/SpinEvent.h
--- a/ipc/mscom/SpinEvent.cpp
+++ b/ipc/mscom/SpinEvent.cpp
@@ -3,75 +3,94 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "mozilla/mscom/SpinEvent.h"
 
 #include "mozilla/ArrayUtils.h"
 #include "mozilla/Assertions.h"
+#include "mozilla/TimeStamp.h"
 #include "nsServiceManagerUtils.h"
 #include "nsString.h"
 #include "nsSystemInfo.h"
 
 // This gives us compiler intrinsics for the x86 PAUSE instruction
 #if defined(_MSC_VER)
 #include <intrin.h>
 #pragma intrinsic(_mm_pause)
 #define CPU_PAUSE() _mm_pause()
 #elif defined(__GNUC__) || defined(__clang__)
 #define CPU_PAUSE() __builtin_ia32_pause()
 #endif
 
 namespace mozilla {
 namespace mscom {
 
+static const TimeDuration kMaxSpinTime = TimeDuration::FromMilliseconds(30);
+bool SpinEvent::sIsMulticore = false;
+
+/* static */ bool
+SpinEvent::InitStatics()
+{
+  SYSTEM_INFO sysInfo;
+  ::GetSystemInfo(&sysInfo);
+  sIsMulticore = sysInfo.dwNumberOfProcessors > 1;
+  return true;
+}
+
 SpinEvent::SpinEvent()
   : mDone(false)
 {
-  static const bool sIsMulticore = []() {
-    SYSTEM_INFO sysInfo;
-    ::GetSystemInfo(&sysInfo);
-    return sysInfo.dwNumberOfProcessors > 1;
-  }();
+  static const bool gotStatics = InitStatics();
+  MOZ_ASSERT(gotStatics);
 
-  if (!sIsMulticore) {
-    mDoneEvent.own(::CreateEventW(nullptr, FALSE, FALSE, nullptr));
-    MOZ_ASSERT(mDoneEvent);
-  }
+  mDoneEvent.own(::CreateEventW(nullptr, FALSE, FALSE, nullptr));
+  MOZ_ASSERT(mDoneEvent);
 }
 
 bool
 SpinEvent::Wait(HANDLE aTargetThread)
 {
   MOZ_ASSERT(aTargetThread);
   if (!aTargetThread) {
     return false;
   }
 
-  if (mDoneEvent) {
-    HANDLE handles[] = {mDoneEvent, aTargetThread};
-    DWORD waitResult = ::WaitForMultipleObjects(mozilla::ArrayLength(handles),
-                                                handles, FALSE, INFINITE);
-    return waitResult == WAIT_OBJECT_0;
+  if (sIsMulticore) {
+    // Bug 1311834: Spinning allows for faster response than waiting on an
+    // event, as events are constrained by the system's timer resolution.
+    // Bug 1429665: However, we only want to spin for a very short time. If
+    // we're waiting for a while, we don't want to be burning CPU for the
+    // entire time. At that point, a few extra ms isn't going to make much
+    // difference to perceived responsiveness.
+    TimeStamp start(TimeStamp::Now());
+    while (!mDone) {
+      TimeDuration elapsed(TimeStamp::Now() - start);
+      if (elapsed >= kMaxSpinTime) {
+        break;
+      }
+      // The PAUSE instruction is a hint to the CPU that we're doing a spin
+      // loop. It is a no-op on older processors that don't support it, so
+      // it is safe to use here without any CPUID checks.
+      CPU_PAUSE();
+    }
+    if (mDone) {
+      return true;
+    }
   }
 
-  while (!mDone) {
-    // The PAUSE instruction is a hint to the CPU that we're doing a spin
-    // loop. It is a no-op on older processors that don't support it, so
-    // it is safe to use here without any CPUID checks.
-    CPU_PAUSE();
-  }
-  return true;
+  MOZ_ASSERT(mDoneEvent);
+  HANDLE handles[] = {mDoneEvent, aTargetThread};
+  DWORD waitResult = ::WaitForMultipleObjects(mozilla::ArrayLength(handles),
+                                              handles, FALSE, INFINITE);
+  return waitResult == WAIT_OBJECT_0;
 }
 
 void
 SpinEvent::Signal()
 {
-  if (mDoneEvent) {
-    ::SetEvent(mDoneEvent);
-  } else {
-    mDone = true;
-  }
+  ::SetEvent(mDoneEvent);
+  mDone = true;
 }
 
 } // namespace mscom
 } // namespace mozilla
--- a/ipc/mscom/SpinEvent.h
+++ b/ipc/mscom/SpinEvent.h
@@ -26,14 +26,16 @@ public:
   SpinEvent(const SpinEvent&) = delete;
   SpinEvent(SpinEvent&&) = delete;
   SpinEvent& operator=(SpinEvent&&) = delete;
   SpinEvent& operator=(const SpinEvent&) = delete;
 
 private:
   Atomic<bool, ReleaseAcquire>  mDone;
   nsAutoHandle                  mDoneEvent;
+  static bool InitStatics();
+  static bool sIsMulticore;
 };
 
 } // namespace mscom
 } // namespace mozilla
 
 #endif // mozilla_mscom_SpinEvent_h