This patch aims to reduce lock contention when dispatching to TaskQueue,
by reducing the frequency with which the runner thread needs to acquire
the TaskQueue monitor.
Prior to this change, if there is a backlog of queued runnables,
TaskQueue::Runner would acquire mQueueMonitor 3 times per event. After
this change, mQueueMonitor will only be acquired when the set of tasks
read the last time the monitor was held have all been processed.
TaskQueue would need to change to use an unbounded lock-free MPSC
queue in order to further reduce contention when dispatching. Doing so
is a much larger project, and would require more substantial changes
to the TaskQueue type to make state management (especially around
shutdown) avoid locking.
Differential Revision: https://phabricator.services.mozilla.com/D261278
In addition, remove the TaskQueueTracker from the BackgroundEventTarget and let it just support shutdown tasks, too.
On a target that SUPPORTS_SHUTDOWN_TASK_DISPATCH, TaskQueue's runner already ensures to drain the queue before the target can fully shutdown. However, eventual TaskQueue shutdown tasks were not yet executed in this case.
We thus register a shutdown task that shuts the TaskQueue down on the target if we have at least one own shutdown task. Note that this will keep the TaskQueue alive until target shutdown if there is no earlier, explicit shutdown on behalf of the consumer.
Note that this means that a TaskQueue may or may not fully enter its internal shutdown state when the target dies, depending on consumers having registered a shutdown task to it. But subsequent dispatches will always fail in the same way, and there is no way to query the shutdown state, anyways.
Differential Revision: https://phabricator.services.mozilla.com/D272151
Introduces SUPPORTS_SHUTDOWN_TASKS and SUPPORTS_SHUTDOWN_TASK_DISPATCH flags.
This just wires the function through all event targets without ever being used.
Differential Revision: https://phabricator.services.mozilla.com/D272150
By now, nobody really uses TaskQueue::mName. Bug 1999508 wants to introduce logging and in order to avoid copying strings just for that, we enforce names to be literals.
In a few cases this reduces the level of detail provided in the name (WebRTC, IndexedDB), but given it never showed up (except maybe while debugging) we can probably live without it.
Differential Revision: https://phabricator.services.mozilla.com/D279500
By now, nobody really uses TaskQueue::mName. Bug 1999508 wants to introduce logging and in order to avoid copying strings just for that, we enforce names to be literals.
In a few cases this reduces the level of detail provided in the name (WebRTC, IndexedDB), but given it never showed up (except maybe while debugging) we can probably live without it.
Differential Revision: https://phabricator.services.mozilla.com/D279500
The linked bug has been intermittently failing. I think this could be because we are in shutdown. Improving the return code from the TaskQueue::DispatchLocked in case dispatch wasn't successful because we were shutting down.
Differential Revision: https://phabricator.services.mozilla.com/D274046
This is just a refactoring to have less copy&pasted code instead of more once we enable more targets for shutdown tasks.
Differential Revision: https://phabricator.services.mozilla.com/D258804
This captures stacks during runnable dispatch which is really
helpful for understanding what's going on and debugging flows.
It's conditional on the Flows profiler feature so that we don't
take the performance cost all the time.
Differential Revision: https://phabricator.services.mozilla.com/D264474
This patch aims to reduce lock contention when dispatching to TaskQueue, by reducing the frequency with which the runner thread needs to acquire the TaskQueue monitor.
Prior to this change, if there is a backlog of queued runnables, TaskQueue::Runner would acquire mQueueMonitor 3 times per event. After this change, mQueueMonitor will only be acquired when the set of tasks read the last time the monitor was held have all been processed.
TaskQueue would need to change to use an unbounded lock-free MPSC queue in order to further reduce contention when dispatching. Doing so is a much larger project, and would require more substantial changes to the TaskQueue type to make state management (especially around shutdown) avoid locking.
Differential Revision: https://phabricator.services.mozilla.com/D261278
This way our WebrtcTaskQueueWrapper can natively set up the RAII class
webrtc::CurrentTaskQueueSetter on the thread the TaskQueue runs on.
Then all tasks running on the TaskQueue can identify as being on the correct
webrtc::TaskQueueBase, regardless of how they were dispatched.
Differential Revision: https://phabricator.services.mozilla.com/D251427
This reverts commit d53f26b530.
Revert "Bug 1968812 - Copy WebrtcTaskQueueWrapper.h to WebrtcTaskQueueWrapper.cpp. r=bwc"
This reverts commit b0d97303dc.
Revert "Bug 1968812 - In WebrtcTaskQueueWrapper, inject the CurrentTaskQueueSetter through the TaskQueue::Observer interface instead of wrapping tasks. r=bwc"
This reverts commit 9b6c76197d.
Revert "Bug 1968812 - Add an Observer to TaskQueue. r=xpcom-reviewers,nika"
This reverts commit 6d63b62958.
Revert "Bug 1968812 - Rename TaskQueueWrapper to WebrtcTaskQueueWrapper to better indicate what it wraps. r=bwc"
This reverts commit 087b7fd11e.
Revert "Bug 1968812 - Add gtest. r=bwc"
This reverts commit 85928698b2.
This way our WebrtcTaskQueueWrapper can natively set up the RAII class
webrtc::CurrentTaskQueueSetter on the thread the TaskQueue runs on.
Then all tasks running on the TaskQueue can identify as being on the correct
webrtc::TaskQueueBase, regardless of how they were dispatched.
Differential Revision: https://phabricator.services.mozilla.com/D251427
In future parts, TaskQueue will require extra initialization to be performed
which cannot happen in a constructor, as it takes references to the TaskQueue
object itself, which will require the introduction of a helper method. This
patch switches all callers of the TaskQueue constructor to use the new method.
Differential Revision: https://phabricator.services.mozilla.com/D142604
This makes the behaviour of TaskQueue more in-line with nsThread, which may be
useful when converting code to using taskqueue instead of threads.
Differential Revision: https://phabricator.services.mozilla.com/D129838
The Runnable markers in the profilers are handy, but miss many types of
runnables. This includes most of those that wrap another runnable and run that
at a (possibly) later time.
AbstractThread, TaskDispatcher and TaskQueue does this for e.g. tail dispatched
tasks.
TaskQueueWrapper does this when wrapping webrtc tasks (and Mozilla Runnables) to
be run in a Mozilla TaskQueue with some overhead on the stack.
DelayedRunnable wraps a runnable to be run after a timeout.
It would perhaps be better in many cases to ignore the intermediate runnables,
but I haven't seen a straight forward way to achieve this.
More detailed data could be added on a case by case basis, for instance the
delay for a DelayedRunnable (incl. actual vs. target delay) or the scope of a
task in which a tail-dispatched runnable was dispatched. But this is also true
for the status quo (for instance the time from dispatch to run) so I leave these
ideas as future work.
Differential Revision: https://phabricator.services.mozilla.com/D135027
The Runnable markers in the profilers are handy, but miss many types of
runnables. This includes most of those that wrap another runnable and run that
at a (possibly) later time.
AbstractThread, TaskDispatcher and TaskQueue does this for e.g. tail dispatched
tasks.
TaskQueueWrapper does this when wrapping webrtc tasks (and Mozilla Runnables) to
be run in a Mozilla TaskQueue with some overhead on the stack.
DelayedRunnable wraps a runnable to be run after a timeout.
It would perhaps be better in many cases to ignore the intermediate runnables,
but I haven't seen a straight forward way to achieve this.
More detailed data could be added on a case by case basis, for instance the
delay for a DelayedRunnable (incl. actual vs. target delay) or the scope of a
task in which a tail-dispatched runnable was dispatched. But this is also true
for the status quo (for instance the time from dispatch to run) so I leave these
ideas as future work.
Differential Revision: https://phabricator.services.mozilla.com/D135027
It looks like we can do a mostly direct swap from `std::Queue` to `mozilla::Queue` without needing to expose any new API, so that's what I've done here.
Differential Revision: https://phabricator.services.mozilla.com/D123523
DelayedDispatch, in all current implementations, will set up a timer sync and
then Dispatch() a runnable. Since the timer is set up before the Dispatch, there
is a theoretical chance that the timer fires and dispatches a TimerEvent to the
target thread before DelayedDispatch managed to do so. When this happens the
internal DelayedDispatch runnable exits early, i.e., in practice it never runs.
The chance increases dramatically if the Dispatch() to the target in question is
tail dispatched, since the time between DelayedDispatch and the tail could be
non-trivial.
This patch removes the assert that checks that all DelayedRunnables that have
run have also been scheduled, since per the above no such guarantee exists.
Differential Revision: https://phabricator.services.mozilla.com/D112876
This assertion currently only happens on debug builds, but attempts to dispatch delayed runnables after we have started or finished cancelling all delayed runnables in a TaskQueue may be causing shutdown hangs.
Differential Revision: https://phabricator.services.mozilla.com/D113839
DelayedDispatch, in all current implementations, will set up a timer sync and
then Dispatch() a runnable. Since the timer is set up before the Dispatch, there
is a theoretical chance that the timer fires and dispatches a TimerEvent to the
target thread before DelayedDispatch managed to do so. When this happens the
internal DelayedDispatch runnable exits early, i.e., in practice it never runs.
The chance increases dramatically if the Dispatch() to the target in question is
tail dispatched, since the time between DelayedDispatch and the tail could be
non-trivial.
This patch removes the assert that checks that all DelayedRunnables that have
run have also been scheduled, since per the above no such guarantee exists.
Differential Revision: https://phabricator.services.mozilla.com/D112876
Because DelayedRunnables are fire-and-forget, there is no way for a targeted
EventTarget to clean them up on shutdown. Thus if a timer fires after
EventTarget shutdown it will fail to dispatch the timer event, and avoid
releasing the timer callback because it's not on the targeted thread. This
causes a leak as there is a ref-cycle between nsTimerImpl::mCallback and
DelayedRunnable::mTimer.
This patch adds nsIDelayedRunnableObserver for a target to observe which
DelayedRunnables are relying on their timer to run them. This allows the target
to schedule a shutdown task to cancel those timers and release the runnables on
the target thread.
Supported DelayedRunnable targets with this patch are TaskQueues,
eventqueue-based nsThreads and XPCOMThreadWrappers that wrap a supported
nsThread.
An assertion makes sure at runtime that future new uses of DelayedRunnable
target nsIDelayedRunnableObserver-supported event targets.
Differential Revision: https://phabricator.services.mozilla.com/D109781
Because DelayedRunnables are fire-and-forget, there is no way for a targeted
EventTarget to clean them up on shutdown. Thus if a timer fires after
EventTarget shutdown it will fail to dispatch the timer event, and avoid
releasing the timer callback because it's not on the targeted thread. This
causes a leak as there is a ref-cycle between nsTimerImpl::mCallback and
DelayedRunnable::mTimer.
This patch adds nsIDelayedRunnableObserver for a target to observe which
DelayedRunnables are relying on their timer to run them. This allows the target
to schedule a shutdown task to cancel those timers and release the runnables on
the target thread.
Supported DelayedRunnable targets with this patch are TaskQueues,
eventqueue-based nsThreads and XPCOMThreadWrappers that wrap a supported
nsThread.
An assertion makes sure at runtime that future new uses of DelayedRunnable
target nsIDelayedRunnableObserver-supported event targets.
Differential Revision: https://phabricator.services.mozilla.com/D109781
In the future, we may want to extend GetCurrentThreadSerialEventTarget to return the actual nsISerialEventTarget used to dispatch the task.
Differential Revision: https://phabricator.services.mozilla.com/D80353
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
We extract the dealing of direct tasks from the TailDispatcher object and move into to the existing nsThread and TaskQueue classes.
TaskQueue is made to work with do_QueryInterface.
We continue accessing the direct tasks dispatcher via the TailDispatcher for now, which itself will forward the dealing of such tasks to the underlying thread.
Differential Revision: https://phabricator.services.mozilla.com/D79093
Nika Layzell