Skip to content
This repository was archived by the owner on Jul 9, 2025. It is now read-only.

Commit 1b790f3

Browse files
Olli PettayOlli Pettay
authored andcommitted
Bug 1522316, use medium high priority queue for worker->main thread control messages, r=baku
If main thread is busy handling runnables in the normal priority queue, control-type of messages from workers are possibly postponed to run after those. That can lead to bad performance, if the page expects workers to be able to proceed simultanously with the main thread. This patch makes the control messages to use medium high priority queue in order to try to ensure they are handled in timely manner. Pref dom.worker.use_medium_high_event_queue can be set to false to disable this new behavior. Differential Revision: https://phabricator.services.mozilla.com/D22128 --HG-- extra : rebase_source : 447dec6dbcccaa0206a1815c21ccf713c523fc91
1 parent c16d618 commit 1b790f3

File tree

11 files changed

+99
-27
lines changed

11 files changed

+99
-27
lines changed

dom/console/Console.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -712,6 +712,8 @@ class ConsoleWorkerRunnable : public WorkerProxyToMainThreadRunnable,
712712
// This method is called in the owning thread of the Console object.
713713
virtual void ReleaseData() = 0;
714714

715+
bool ForMessaging() const override { return true; }
716+
715717
// This must be released on the worker thread.
716718
RefPtr<Console> mConsole;
717719

dom/serviceworkers/ServiceWorkerPrivate.cpp

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -639,7 +639,9 @@ class LifeCycleEventWatcher final : public ExtendableEventCallback {
639639
[self]() { self->ReportResult(false); });
640640
if (NS_WARN_IF(!mWorkerRef)) {
641641
mCallback->SetResult(false);
642-
nsresult rv = workerPrivate->DispatchToMainThread(mCallback);
642+
// Using DispatchToMainThreadForMessaging so that state update on
643+
// the main thread doesn't happen too soon.
644+
nsresult rv = workerPrivate->DispatchToMainThreadForMessaging(mCallback);
643645
Unused << NS_WARN_IF(NS_FAILED(rv));
644646
return false;
645647
}
@@ -655,7 +657,9 @@ class LifeCycleEventWatcher final : public ExtendableEventCallback {
655657
}
656658

657659
mCallback->SetResult(aResult);
658-
nsresult rv = mWorkerRef->Private()->DispatchToMainThread(mCallback);
660+
// Using DispatchToMainThreadForMessaging so that state update on
661+
// the main thread doesn't happen too soon.
662+
nsresult rv = mWorkerRef->Private()->DispatchToMainThreadForMessaging(mCallback);
659663
if (NS_WARN_IF(NS_FAILED(rv))) {
660664
MOZ_CRASH("Failed to dispatch life cycle event handler.");
661665
}

dom/workers/WorkerDebugger.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -400,7 +400,8 @@ void WorkerDebugger::PostMessageToDebugger(const nsAString& aMessage) {
400400

401401
RefPtr<PostDebuggerMessageRunnable> runnable =
402402
new PostDebuggerMessageRunnable(this, aMessage);
403-
if (NS_FAILED(mWorkerPrivate->DispatchToMainThread(runnable.forget()))) {
403+
if (NS_FAILED(mWorkerPrivate->DispatchToMainThreadForMessaging(
404+
runnable.forget()))) {
404405
NS_WARNING("Failed to post message to debugger on main thread!");
405406
}
406407
}
@@ -422,7 +423,8 @@ void WorkerDebugger::ReportErrorToDebugger(const nsAString& aFilename,
422423

423424
RefPtr<ReportDebuggerErrorRunnable> runnable =
424425
new ReportDebuggerErrorRunnable(this, aFilename, aLineno, aMessage);
425-
if (NS_FAILED(mWorkerPrivate->DispatchToMainThread(runnable.forget()))) {
426+
if (NS_FAILED(mWorkerPrivate->DispatchToMainThreadForMessaging(
427+
runnable.forget()))) {
426428
NS_WARNING("Failed to report error to debugger on main thread!");
427429
}
428430
}

dom/workers/WorkerPrivate.cpp

Lines changed: 38 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1032,8 +1032,8 @@ class WorkerPrivate::MemoryReporter final : public nsIMemoryReporter {
10321032

10331033
WorkerPrivate* workerPrivate = GetCurrentThreadWorkerPrivate();
10341034
MOZ_ASSERT(workerPrivate);
1035-
MOZ_ALWAYS_SUCCEEDS(
1036-
workerPrivate->DispatchToMainThread(mFinishCollectRunnable.forget()));
1035+
MOZ_ALWAYS_SUCCEEDS(workerPrivate->DispatchToMainThreadForMessaging(
1036+
mFinishCollectRunnable.forget()));
10371037
}
10381038
};
10391039

@@ -1337,9 +1337,9 @@ nsresult WorkerPrivate::Dispatch(already_AddRefed<WorkerRunnable> aRunnable,
13371337
return DispatchLockHeld(std::move(aRunnable), aSyncLoopTarget, lock);
13381338
}
13391339

1340-
nsresult WorkerPrivate::DispatchLockHeld(already_AddRefed<WorkerRunnable> aRunnable,
1341-
nsIEventTarget* aSyncLoopTarget,
1342-
const MutexAutoLock& aProofOfLock) {
1340+
nsresult WorkerPrivate::DispatchLockHeld(
1341+
already_AddRefed<WorkerRunnable> aRunnable, nsIEventTarget* aSyncLoopTarget,
1342+
const MutexAutoLock& aProofOfLock) {
13431343
// May be called on any thread!
13441344
RefPtr<WorkerRunnable> runnable(aRunnable);
13451345

@@ -1378,8 +1378,7 @@ nsresult WorkerPrivate::DispatchLockHeld(already_AddRefed<WorkerRunnable> aRunna
13781378
// they should not be delivered to a frozen worker. But frozen workers
13791379
// aren't drawing from the thread's main event queue anyway, only from
13801380
// mControlQueue.
1381-
rv = mThread->DispatchAnyThread(WorkerThreadFriendKey(),
1382-
runnable.forget());
1381+
rv = mThread->DispatchAnyThread(WorkerThreadFriendKey(), runnable.forget());
13831382
}
13841383

13851384
if (NS_WARN_IF(NS_FAILED(rv))) {
@@ -2125,6 +2124,8 @@ WorkerPrivate::WorkerPrivate(WorkerPrivate* aParent,
21252124
// that ThrottledEventQueue can only be created on the main thread at the
21262125
// moment.
21272126
if (aParent) {
2127+
mMainThreadEventTargetForMessaging =
2128+
aParent->mMainThreadEventTargetForMessaging;
21282129
mMainThreadEventTarget = aParent->mMainThreadEventTarget;
21292130
mMainThreadDebuggeeEventTarget = aParent->mMainThreadDebuggeeEventTarget;
21302131
return;
@@ -2141,7 +2142,14 @@ WorkerPrivate::WorkerPrivate(WorkerPrivate* aParent,
21412142

21422143
// Throttle events to the main thread using a ThrottledEventQueue specific to
21432144
// this tree of worker threads.
2144-
mMainThreadEventTarget = ThrottledEventQueue::Create(target);
2145+
mMainThreadEventTargetForMessaging = ThrottledEventQueue::Create(target);
2146+
if (StaticPrefs::dom_worker_use_medium_high_event_queue()) {
2147+
mMainThreadEventTarget =
2148+
ThrottledEventQueue::Create(GetMainThreadSerialEventTarget(),
2149+
nsIRunnablePriority::PRIORITY_MEDIUMHIGH);
2150+
} else {
2151+
mMainThreadEventTarget = mMainThreadEventTargetForMessaging;
2152+
}
21452153
mMainThreadDebuggeeEventTarget = ThrottledEventQueue::Create(target);
21462154
if (IsParentWindowPaused() || IsFrozen()) {
21472155
MOZ_ALWAYS_SUCCEEDS(mMainThreadDebuggeeEventTarget->SetIsPaused(true));
@@ -2254,9 +2262,7 @@ already_AddRefed<WorkerPrivate> WorkerPrivate::Constructor(
22542262
return worker.forget();
22552263
}
22562264

2257-
nsresult
2258-
WorkerPrivate::SetIsDebuggerReady(bool aReady)
2259-
{
2265+
nsresult WorkerPrivate::SetIsDebuggerReady(bool aReady) {
22602266
AssertIsOnParentThread();
22612267
MutexAutoLock lock(mMutex);
22622268

@@ -2750,7 +2756,7 @@ void WorkerPrivate::DoRunLoop(JSContext* aCx) {
27502756
// If the worker thread is spamming the main thread faster than it can
27512757
// process the work, then pause the worker thread until the main thread
27522758
// catches up.
2753-
size_t queuedEvents = mMainThreadEventTarget->Length() +
2759+
size_t queuedEvents = mMainThreadEventTargetForMessaging->Length() +
27542760
mMainThreadDebuggeeEventTarget->Length();
27552761
if (queuedEvents > 5000) {
27562762
// Note, postMessage uses mMainThreadDebuggeeEventTarget!
@@ -2783,6 +2789,22 @@ void WorkerPrivate::AfterProcessNextEvent() {
27832789
MOZ_ASSERT(CycleCollectedJSContext::Get()->RecursionDepth());
27842790
}
27852791

2792+
nsIEventTarget* WorkerPrivate::MainThreadEventTargetForMessaging() {
2793+
return mMainThreadEventTargetForMessaging;
2794+
}
2795+
2796+
nsresult WorkerPrivate::DispatchToMainThreadForMessaging(nsIRunnable* aRunnable,
2797+
uint32_t aFlags) {
2798+
nsCOMPtr<nsIRunnable> r = aRunnable;
2799+
return DispatchToMainThreadForMessaging(r.forget(), aFlags);
2800+
}
2801+
2802+
nsresult WorkerPrivate::DispatchToMainThreadForMessaging(
2803+
already_AddRefed<nsIRunnable> aRunnable, uint32_t aFlags) {
2804+
return mMainThreadEventTargetForMessaging->Dispatch(std::move(aRunnable),
2805+
aFlags);
2806+
}
2807+
27862808
nsIEventTarget* WorkerPrivate::MainThreadEventTarget() {
27872809
return mMainThreadEventTarget;
27882810
}
@@ -3148,9 +3170,12 @@ void WorkerPrivate::ScheduleDeletion(WorkerRanOrNot aRanOrNot) {
31483170
NS_WARNING("Failed to dispatch runnable!");
31493171
}
31503172
} else {
3173+
// Note, this uses the lower priority DispatchToMainThreadForMessaging for
3174+
// dispatching TopLevelWorkerFinishedRunnable to the main thread so that
3175+
// other relevant runnables are guaranteed to run before it.
31513176
RefPtr<TopLevelWorkerFinishedRunnable> runnable =
31523177
new TopLevelWorkerFinishedRunnable(this);
3153-
if (NS_FAILED(DispatchToMainThread(runnable.forget()))) {
3178+
if (NS_FAILED(DispatchToMainThreadForMessaging(runnable.forget()))) {
31543179
NS_WARNING("Failed to dispatch runnable!");
31553180
}
31563181
}

dom/workers/WorkerPrivate.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -401,6 +401,15 @@ class WorkerPrivate : public RelativeTimeline {
401401
// Get the event target to use when dispatching to the main thread
402402
// from this Worker thread. This may be the main thread itself or
403403
// a ThrottledEventQueue to the main thread.
404+
nsIEventTarget* MainThreadEventTargetForMessaging();
405+
406+
nsresult DispatchToMainThreadForMessaging(
407+
nsIRunnable* aRunnable, uint32_t aFlags = NS_DISPATCH_NORMAL);
408+
409+
nsresult DispatchToMainThreadForMessaging(
410+
already_AddRefed<nsIRunnable> aRunnable,
411+
uint32_t aFlags = NS_DISPATCH_NORMAL);
412+
404413
nsIEventTarget* MainThreadEventTarget();
405414

406415
nsresult DispatchToMainThread(nsIRunnable* aRunnable,
@@ -973,6 +982,7 @@ class WorkerPrivate : public RelativeTimeline {
973982
PRThread* mPRThread;
974983

975984
// Accessed from main thread
985+
RefPtr<ThrottledEventQueue> mMainThreadEventTargetForMessaging;
976986
RefPtr<ThrottledEventQueue> mMainThreadEventTarget;
977987

978988
// Accessed from worker thread and destructing thread

dom/workers/WorkerRunnable.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -619,7 +619,10 @@ bool WorkerProxyToMainThreadRunnable::Dispatch(WorkerPrivate* aWorkerPrivate) {
619619
MOZ_ASSERT(!mWorkerRef);
620620
mWorkerRef = new ThreadSafeWorkerRef(workerRef);
621621

622-
if (NS_WARN_IF(NS_FAILED(aWorkerPrivate->DispatchToMainThread(this)))) {
622+
if (ForMessaging()
623+
? NS_WARN_IF(NS_FAILED(
624+
aWorkerPrivate->DispatchToMainThreadForMessaging(this)))
625+
: NS_WARN_IF(NS_FAILED(aWorkerPrivate->DispatchToMainThread(this)))) {
623626
ReleaseWorker();
624627
RunBackOnWorkerThreadForCleanup(aWorkerPrivate);
625628
return false;

dom/workers/WorkerRunnable.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -397,6 +397,8 @@ class WorkerProxyToMainThreadRunnable : public Runnable {
397397
public:
398398
bool Dispatch(WorkerPrivate* aWorkerPrivate);
399399

400+
virtual bool ForMessaging() const { return false; }
401+
400402
private:
401403
NS_IMETHOD Run() override;
402404

dom/workers/WorkerScope.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -661,7 +661,7 @@ void ServiceWorkerGlobalScope::SetOnfetch(
661661
if (aCallback) {
662662
if (mWorkerPrivate->WorkerScriptExecutedSuccessfully()) {
663663
RefPtr<Runnable> r = new ReportFetchListenerWarningRunnable(mScope);
664-
mWorkerPrivate->DispatchToMainThread(r.forget());
664+
mWorkerPrivate->DispatchToMainThreadForMessaging(r.forget());
665665
}
666666
mWorkerPrivate->SetFetchHandlerWasAdded();
667667
}
@@ -678,7 +678,7 @@ void ServiceWorkerGlobalScope::EventListenerAdded(nsAtom* aType) {
678678

679679
if (mWorkerPrivate->WorkerScriptExecutedSuccessfully()) {
680680
RefPtr<Runnable> r = new ReportFetchListenerWarningRunnable(mScope);
681-
mWorkerPrivate->DispatchToMainThread(r.forget());
681+
mWorkerPrivate->DispatchToMainThreadForMessaging(r.forget());
682682
}
683683

684684
mWorkerPrivate->SetFetchHandlerWasAdded();

modules/libpref/init/StaticPrefList.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -469,6 +469,12 @@ VARCACHE_PREF(
469469
RelaxedAtomicUint32, 30000 /* 30 seconds */
470470
)
471471

472+
VARCACHE_PREF(
473+
"dom.worker.use_medium_high_event_queue",
474+
dom_worker_use_medium_high_event_queue,
475+
RelaxedAtomicBool, true
476+
)
477+
472478
// Enable content type normalization of XHR uploads via MIME Sniffing standard
473479
// Disabled for now in bz1499136
474480
VARCACHE_PREF(

xpcom/threads/ThrottledEventQueue.cpp

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -62,21 +62,31 @@ class ThrottledEventQueue::Inner final : public nsISupports {
6262
// The runnable which is dispatched to the underlying base target. Since
6363
// we only execute one event at a time we just re-use a single instance
6464
// of this class while there are events left in the queue.
65-
class Executor final : public Runnable {
65+
class Executor final : public Runnable, public nsIRunnablePriority {
6666
// The Inner whose runnables we execute. mInner->mExecutor points
6767
// to this executor, forming a reference loop.
6868
RefPtr<Inner> mInner;
6969

70+
~Executor() = default;
71+
7072
public:
7173
explicit Executor(Inner* aInner)
7274
: Runnable("ThrottledEventQueue::Inner::Executor"), mInner(aInner) {}
7375

76+
NS_DECL_ISUPPORTS_INHERITED
77+
7478
NS_IMETHODIMP
7579
Run() override {
7680
mInner->ExecuteRunnable();
7781
return NS_OK;
7882
}
7983

84+
NS_IMETHODIMP
85+
GetPriority(uint32_t* aPriority) override {
86+
*aPriority = mInner->mPriority;
87+
return NS_OK;
88+
}
89+
8090
#ifdef MOZ_COLLECTING_RUNNABLE_TELEMETRY
8191
NS_IMETHODIMP
8292
GetName(nsACString& aName) override { return mInner->CurrentName(aName); }
@@ -103,14 +113,17 @@ class ThrottledEventQueue::Inner final : public nsISupports {
103113
// Used from any thread; protected by mMutex.
104114
nsCOMPtr<nsIRunnable> mExecutor;
105115

116+
const uint32_t mPriority;
117+
106118
// True if this queue is currently paused.
107119
// Used from any thread; protected by mMutex.
108120
bool mIsPaused;
109121

110-
explicit Inner(nsISerialEventTarget* aBaseTarget)
122+
explicit Inner(nsISerialEventTarget* aBaseTarget, uint32_t aPriority)
111123
: mMutex("ThrottledEventQueue"),
112124
mIdleCondVar(mMutex, "ThrottledEventQueue:Idle"),
113125
mBaseTarget(aBaseTarget),
126+
mPriority(aPriority),
114127
mIsPaused(false) {}
115128

116129
~Inner() {
@@ -231,12 +244,13 @@ class ThrottledEventQueue::Inner final : public nsISupports {
231244
}
232245

233246
public:
234-
static already_AddRefed<Inner> Create(nsISerialEventTarget* aBaseTarget) {
247+
static already_AddRefed<Inner> Create(nsISerialEventTarget* aBaseTarget,
248+
uint32_t aPriority) {
235249
MOZ_ASSERT(NS_IsMainThread());
236250
MOZ_ASSERT(ClearOnShutdown_Internal::sCurrentShutdownPhase ==
237251
ShutdownPhase::NotInShutdown);
238252

239-
RefPtr<Inner> ref = new Inner(aBaseTarget);
253+
RefPtr<Inner> ref = new Inner(aBaseTarget, aPriority);
240254
return ref.forget();
241255
}
242256

@@ -332,6 +346,9 @@ class ThrottledEventQueue::Inner final : public nsISupports {
332346

333347
NS_IMPL_ISUPPORTS(ThrottledEventQueue::Inner, nsISupports);
334348

349+
NS_IMPL_ISUPPORTS_INHERITED(ThrottledEventQueue::Inner::Executor, Runnable,
350+
nsIRunnablePriority)
351+
335352
NS_IMPL_ISUPPORTS(ThrottledEventQueue, ThrottledEventQueue, nsIEventTarget,
336353
nsISerialEventTarget);
337354

@@ -341,11 +358,11 @@ ThrottledEventQueue::ThrottledEventQueue(already_AddRefed<Inner> aInner)
341358
}
342359

343360
already_AddRefed<ThrottledEventQueue> ThrottledEventQueue::Create(
344-
nsISerialEventTarget* aBaseTarget) {
361+
nsISerialEventTarget* aBaseTarget, uint32_t aPriority) {
345362
MOZ_ASSERT(NS_IsMainThread());
346363
MOZ_ASSERT(aBaseTarget);
347364

348-
RefPtr<Inner> inner = Inner::Create(aBaseTarget);
365+
RefPtr<Inner> inner = Inner::Create(aBaseTarget, aPriority);
349366

350367
RefPtr<ThrottledEventQueue> ref = new ThrottledEventQueue(inner.forget());
351368
return ref.forget();

0 commit comments

Comments
 (0)