Bug 1646054 - P2. Always retain dispatch flags r=froydnj
authorJean-Yves Avenard <jyavenard@mozilla.com>
Mon, 22 Jun 2020 13:55:23 +0000
changeset 536560 d8c9e055d41015b544834f380300f2fa9601aabf
parent 536559 ad3e3c470aec2cae1576b8be96e3b4ee3b7923be
child 536561 0a814322a3e611a212c6e8f3e2e5579e0fdc6a1c
push id37530
push usernbeleuzu@mozilla.com
push dateMon, 22 Jun 2020 21:47:58 +0000
treeherdermozilla-central@9b38f4b9d883 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj
bugs1646054
milestone79.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 1646054 - P2. Always retain dispatch flags r=froydnj When TaskQueue was first conceived; it was only used with AbstractThreads and with tail dispatch. By default, AbstractThread::Dispatch dropped the flags , as it was dispatching all tasks via the tail dispatcher. It was an oversight, there's no use-case where we wouldn't want the dispatch flags to be carried forward. It also simplifies the code and TaskQueue's use. Depends on D80351 Differential Revision: https://phabricator.services.mozilla.com/D80352
xpcom/threads/TaskQueue.cpp
xpcom/threads/TaskQueue.h
xpcom/threads/nsThreadManager.cpp
--- a/xpcom/threads/TaskQueue.cpp
+++ b/xpcom/threads/TaskQueue.cpp
@@ -6,29 +6,27 @@
 
 #include "mozilla/TaskQueue.h"
 
 #include "nsThreadUtils.h"
 
 namespace mozilla {
 
 TaskQueue::TaskQueue(already_AddRefed<nsIEventTarget> aTarget,
-                     const char* aName, bool aRequireTailDispatch,
-                     bool aRetainFlags)
+                     const char* aName, bool aRequireTailDispatch)
     : AbstractThread(aRequireTailDispatch),
       mTarget(aTarget),
       mQueueMonitor("TaskQueue::Queue"),
       mTailDispatcher(nullptr),
-      mShouldRetainFlags(aRetainFlags),
       mIsRunning(false),
       mIsShutdown(false),
       mName(aName) {}
 
 TaskQueue::TaskQueue(already_AddRefed<nsIEventTarget> aTarget,
-                     bool aSupportsTailDispatch, bool aRetainFlags)
+                     bool aSupportsTailDispatch)
     : TaskQueue(std::move(aTarget), "Unnamed", aSupportsTailDispatch) {}
 
 TaskQueue::~TaskQueue() {
   // No one is referencing this TaskQueue anymore, meaning no tasks can be
   // pending as all Runner hold a reference to this TaskQueue.
 }
 
 NS_IMPL_ISUPPORTS_INHERITED(TaskQueue, AbstractThread, nsIDirectTaskDispatcher);
@@ -52,21 +50,18 @@ nsresult TaskQueue::DispatchLocked(nsCOM
   if (aReason != TailDispatch && (currentThread = GetCurrent()) &&
       RequiresTailDispatch(currentThread) &&
       currentThread->IsTailDispatcherAvailable()) {
     MOZ_ASSERT(aFlags == NS_DISPATCH_NORMAL,
                "Tail dispatch doesn't support flags");
     return currentThread->TailDispatcher().AddTask(this, aRunnable.forget());
   }
 
-  // If the task queue cares about individual flags, retain them in the struct.
-  uint32_t retainFlags = mShouldRetainFlags ? aFlags : 0;
-
   LogRunnable::LogDispatch(aRunnable);
-  mTasks.push({std::move(aRunnable), retainFlags});
+  mTasks.push({std::move(aRunnable), aFlags});
 
   if (mIsRunning) {
     return NS_OK;
   }
   RefPtr<nsIRunnable> runner(new Runner(this));
   nsresult rv = mTarget->Dispatch(runner.forget(), aFlags);
   if (NS_FAILED(rv)) {
     NS_WARNING("Failed to dispatch runnable to run TaskQueue");
--- a/xpcom/threads/TaskQueue.h
+++ b/xpcom/threads/TaskQueue.h
@@ -48,21 +48,20 @@ typedef MozPromise<bool, bool, false> Sh
 // A TaskQueue does not require explicit shutdown, however it provides a
 // BeginShutdown() method that places TaskQueue in a shut down state and returns
 // a promise that gets resolved once all pending tasks have completed
 class TaskQueue : public AbstractThread, public nsIDirectTaskDispatcher {
   class EventTargetWrapper;
 
  public:
   explicit TaskQueue(already_AddRefed<nsIEventTarget> aTarget,
-                     bool aSupportsTailDispatch = false,
-                     bool aRetainFlags = false);
+                     bool aSupportsTailDispatch = false);
 
   TaskQueue(already_AddRefed<nsIEventTarget> aTarget, const char* aName,
-            bool aSupportsTailDispatch = false, bool aRetainFlags = false);
+            bool aSupportsTailDispatch = false);
 
   NS_DECL_ISUPPORTS_INHERITED
   NS_DECL_NSIDIRECTTASKDISPATCHER
 
   TaskDispatcher& TailDispatcher() override;
 
   NS_IMETHOD Dispatch(already_AddRefed<nsIRunnable> aEvent,
                       uint32_t aFlags) override {
@@ -191,21 +190,16 @@ class TaskQueue : public AbstractThread,
    private:
     Maybe<AutoTaskDispatcher> mTaskDispatcher;
     TaskQueue* mQueue;
     AbstractThread* mLastCurrentThread;
   };
 
   TaskDispatcher* mTailDispatcher;
 
-  // TaskQueues should specify if they want all tasks to dispatch with their
-  // original flags included, which means the flags will be retained in the
-  // TaskStruct.
-  bool mShouldRetainFlags;
-
   // True if we've dispatched an event to the target to execute events from
   // the queue.
   bool mIsRunning;
 
   // True if we've started our shutdown process.
   bool mIsShutdown;
   MozPromiseHolder<ShutdownPromise> mShutdownPromise;
 
--- a/xpcom/threads/nsThreadManager.cpp
+++ b/xpcom/threads/nsThreadManager.cpp
@@ -179,19 +179,17 @@ void BackgroundEventTarget::FinishShutdo
   mPool->Shutdown();
   mIOPool->Shutdown();
 }
 
 already_AddRefed<nsISerialEventTarget>
 BackgroundEventTarget::CreateBackgroundTaskQueue(const char* aName) {
   MutexAutoLock lock(mMutex);
 
-  RefPtr<TaskQueue> queue = new TaskQueue(do_AddRef(this), aName,
-                                          /*aSupportsTailDispatch=*/false,
-                                          /*aRetainFlags=*/true);
+  RefPtr<TaskQueue> queue = new TaskQueue(do_AddRef(this), aName);
   mTaskQueues.AppendElement(queue);
 
   return queue.forget();
 }
 
 extern "C" {
 // This uses the C language linkage because it's exposed to Rust
 // via the xpcom/rust/moz_task crate.