Also, deCOM the declared types of the persistent member-variables where these
objects get stored (to just match the actual concrete type); and remove
unnecessary nullchecks after infallible allocation.
Differential Revision: https://phabricator.services.mozilla.com/D303510
This function already has a local RefPtr<T> that goes out of scope (and drops
the refcount) at the end of the function. Since some callers capture the
return value in a RefPtr, it makes sense to just directly return that existing
reference instead of doing a Release()/AddRef() dance.
Other callers don't bother to use the returned value, and that's OK, because
RefPtr as a return-type allows itself to be ignored (unlike already_AddRefed);
and there's another reference that exists in thread local storage which keeps
the new nsThread alive.
Differential Revision: https://phabricator.services.mozilla.com/D303509
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 page size on arm64 macOS and iOS is 16k, yet we try to create stacks with
sizes which are not a multiple of 16k in some situations. Unify these stack
size calculations, and ensure they are always a multiple of the system page
size.
On arm64 macOS, the posix threads implementation is detecting stack sizes which
are a multiple of 4k, and rounding them up to a multiple of 16k to improve
compatibility, but that is not done on iOS.
Differential Revision: https://phabricator.services.mozilla.com/D236402
The original intention of adding NS_DISPATCH_AT_END always when on the
same pool was to reduce the risk of spinning up unneeded threads.
Bug 1891664 introduced two changes that make this now unwanted:
- we always wait for the dispatching thread to pick up the event
- we give threads a grace timeout before shutting them down
So before bug 1891664 landed, this flag would just have influenced
if we create a new thread, but if there was an idle thread, the event
would just process immediately and in parallel without any latency.
Now the event will wait for the dispatching thread to become idle if
NS_DISPATCH_AT_END is set, which might increase the latency if we are
not at the end of the dispatching event as we are instead in the case of
TaskQueue dispatches.
What's more, the grace timeout reduces the risk of noise from frequent
thread creation and destruction, such that creating a new thread when
there is load we can immediately serve is actually best for latency.
Differential Revision: https://phabricator.services.mozilla.com/D212399
Have idleThreadGraceTimeout and idleThreadMaximumTimeout instead of just idleThreadTimeout.
Clarify that idleThreadMaximumTimeout is only affecting allowed idle threads.
Make idle threads end only after at minimum idleThreadGraceTimeout even if they are in excess.
Remove the idleThreadTimeoutRegressive setting.
Introduce a "most recently used" priority for notifying idle threads to
avoid excessive round-robin through all available idle threads.
The management of the linked list has constant time, adding thus only
minimal overhead wrt to the previous wasIdle flags we had.
As a side effect (and coming from the investigations in bug 1891732) to
some extent this can help to improve the "logical thread affinity",
together with trying to keep events dispatched with NS_DISPATCH_AT_END
on the dispatching thread as much as possible, which should help
TaskQueue a lot with affinity.
Differential Revision: https://phabricator.services.mozilla.com/D209884
This changes comes with several different refactorings all rolled into one,
unfotunately I couldn't find a way to pull them apart:
- First of all annotations now can either recorded (that is, we copy the value
and have the crash reporting code own the copy) or registered. Several
annotations are changed to use this functionality so that we don't need to
update them as their value change.
- The code in the exception handler is modified to read the annotations from
the mozannotation_client crate. This has the unfortunate side-effect that
we need three different bits of code to serialize them: one for annotations
read from a child process, one for reading annotations from the main process
outside of the exception handler and one for reading annotations from the
main process within the exception handler. As we move to fully
out-of-process crash reporting the last two methods will go away.
- The mozannotation_client crate now doesn't record annotation types anymore.
I realized as I was working on this that storing types at runtime has two
issues: the first one is that buggy code might change the type of an
annotation (that is record it under two different types at two different
moments), the second issue is that types might become corrupt during a
crash, so better enforce them at annotation-writing time. The end result is
that the mozannotation_* crates now only store byte buffers, track the
format the data is stored in (null-terminated string, fixed size buffer,
etc...) but not the type of data each annotation is supposed to contain.
- Which brings us to the next change: concrete types for annotations are now
enforced when they're written out. If an annotation doesn't match the
expected type it's skipped. Storing an annotation with the wrong type will
also trigger an assertion in debug builds.
Differential Revision: https://phabricator.services.mozilla.com/D195248
This both reduces the number of registered static destructors, and avoids the
assertion in the LinkedList destructor without depending on a StaticAutoPointer
(which requires an allocation & additional indirection) by using the new
NeverDestroyed type from part 4.
Differential Revision: https://phabricator.services.mozilla.com/D189249
This fixes the issue described in comment 7 by doing a few changes to how threads are created, and members on nsThreadManager are managed.
* The thread list is moved to be a member of `nsThreadManager`, which is
already a statically allocated singleton.
* The flag for whether new thread creation is allowed is moved from being an
atomic to being guarded by the same mutex as the thread list, meaning that
it could be checked and a new thread could be added to the list atomically.
* Threads which require shutdown are initialized atomically from the point of
view of the thread manager, meaning that the check in ShutdownNonMainThreads
is guaranteed to catch all successfully started nsThread instances.
Differential Revision: https://phabricator.services.mozilla.com/D188623
This fixes the issue described in comment 7 by doing a few changes to how threads are created, and members on nsThreadManager are managed.
* The thread list is moved to be a member of `nsThreadManager`, which is
already a statically allocated singleton.
* The flag for whether new thread creation is allowed is moved from being an
atomic to being guarded by the same mutex as the thread list, meaning that
it could be checked and a new thread could be added to the list atomically.
* Threads which require shutdown are initialized atomically from the point of
view of the thread manager, meaning that the check in ShutdownNonMainThreads
is guaranteed to catch all successfully started nsThread instances.
Differential Revision: https://phabricator.services.mozilla.com/D188623
This will be used to deprioritize whole thread pools. The concept works as such:
- Set a thread pool's priority at any time
- Each thread will ensure their priority mirrors the parent pool's priority when it is finished processing events.
I chose to have threads only set themselves to the background when they finish processing events because an event dispatched at an expected priority should be completed at said priority. This ensures that any event dispatch that may expect normal priority (i.e. not operating in the background) has a chance to run at the established priority.
This patch enrolls the background IO to low priority on its creation. It also includes the MacOS implementation of the thread priorities, which utilizes mac's QoS service. According to MacOS's QoS structure, main threads operate at `user-interactive` and offthread work is done at a `default` state, meaning that the OS decides how to set the thread's priority somewhere between `utility` and `user-interactive`. When a thread is put in the background, it must be manually raised back to the expected priority, which means most threads must set themselves to `default` again and the main thread must set itself to `user-interactive`. I demonstrated this in the `nsThread` implementation by checking for the main thread and if this task is running on the main thread somehow, then we use the correct setting.
Rather than having the thread pools dispatch their own deprioritization tasks, I've decided to make this the job of the nsThread. In following bugs we will put entire processes' nsThreads in the background, which will require more use of the new nsThread codepaths.
This behavior is currently restricted to nightly only. On platforms with no implementation, nothing will change.
Differential Revision: https://phabricator.services.mozilla.com/D146534
This was always being tracked internlly to implement
nsThreadPool::IsOnCurrentThread, however this patch exposes the getter
publicly.
This will be used to assert if GetCurrentSerialEventTarget() is called from a
threadpool thread without a TaskQueue in the next part.
Differential Revision: https://phabricator.services.mozilla.com/D166603
This was always being tracked internlly to implement
nsThreadPool::IsOnCurrentThread, however this patch exposes the getter
publicly.
This will be used to assert if GetCurrentSerialEventTarget() is called from a
threadpool thread without a TaskQueue in the next part.
Differential Revision: https://phabricator.services.mozilla.com/D166603
Some debug infrastructure like MOZ_WEAKPTR_INIT_THREAD_SAFETY_CHECK that can apparently be triggered by nsComponentManagerImpl::gComponentManager->FreeService() seem to rely on the existence of a serial event target even if they do not post any events. So it seems sound to keep a representation of the main thread as nsThread object until after final XPCOM shutdown.
But nsThreadManager::ShutdownMainThread() does more, it processes the last round of events, removes the thread's observer and closes down the background event target. We do not want to move these operations to happen later than before, such that we split the nsThread release into a separate function and move that together with AbstractThread::ShutdownMainThread() behind FreeService().
Depends on D160628
Differential Revision: https://phabricator.services.mozilla.com/D162497
Lando