We lose some sugar but gain some safety. This seems like the right
trade. If people want sugar they should use Rust.
Differential Revision: https://phabricator.services.mozilla.com/D16918
--HG--
extra : moz-landing-system : lando
This commit adds categories to all markers. This way the profiler's
marker categories and frame label categories agree. There are a few
duplicate category properties on some of the marker payloads, but
this could be cleaned up in a follow-up if needed.
Differential Revision: https://phabricator.services.mozilla.com/D16864
--HG--
extra : moz-landing-system : lando
This is a better place for it and is more appropriate given that it
already exports to mozilla/DataMutex.h. I'll fix the rvalue reference
problems in a follow up.
Differential Revision: https://phabricator.services.mozilla.com/D16723
--HG--
rename : dom/media/eme/DataMutex.h => xpcom/threads/DataMutex.h
extra : moz-landing-system : lando
nsTimerEvent goes through a multi-step initialization for reasons that
are lost to time. We are also seeing peculiar crashes in
`nsTimerEvent::SetTimer()` that are only explainable by `SetTimer`
finding a non-null pointer where there should have been a null pointer.
The compiler ought to have been able to optimize those bits away, but no
matter: we can do the job ourselves and make the code clearer.
Since we only call `SetTimer` once, we should just move its work into
nsTimerEvent's constructor.
Doing this code movement separately will ideally make the next part of
this work easier to review. The idea is that we want to extract all the
necessary information from `timer` before we pass ownership of it into
the newly-allocated nsTimerEvent.
Unlike many of our uses of `new`, nsTimerEvent has its own definition of
`operator new`, to ensure instances are allocated through
TimerEventAllocator. And allocating with TimerEventAllocator can fail.
Later changes, however, want to assume that constructing an nsTimerEvent
can't fail, which is difficult to guarantee with the current structure.
To make that guarantee, we need to make explicit what calling `new`
does: there's an "allocate memory" step and a "construct the object"
step. The first part can fail, and that's what we care about here.
Once we have a chunk of memory, we can construct the object as normal,
secure in the knowledge that calling (placement) `new` is now guaranteed
to succeed.
We introduce GenericNonExclusivePromise that can be used to explicitly state than non-exclusive use is needed
We temporarily disable the assertion ensuring a promise is used exclusively when needed to allow for things to settle.
Differential Revision: https://phabricator.services.mozilla.com/D14025
--HG--
extra : moz-landing-system : lando
This is a best effort attempt at ensuring that the adverse impact of
reformatting the entire tree over the comments would be minimal. I've used a
combination of strategies including disabling of formatting, some manual
formatting and some changes to formatting to work around some clang-format
limitations.
Differential Revision: https://phabricator.services.mozilla.com/D13371
--HG--
extra : moz-landing-system : lando
Clang format does not always reflow comments correctly to get them
within 80 columns.
The major categories of failures I have noticed in xpcom/ are:
- Comments that are lists. I fixed these by manually getting them so
they'll be within 80 columns after clang-format runs.
- Comments intermixed with lists of things like enums, initializers,
or even fields in a class. It doesn't seem to associate the comment
with the item in the list correctly. The worst cases of these happen
when it changes initializer lists from having commas at the start of
each item to having them at the end. In the initializer comma cases,
I fixed them by making the commas at the end, so clang-format won't
mix things up. For other cases, I often moved the comment for an
item onto its own line, because it was not possible to have both the
comment and the item on the same line and stay within 80 columns.
- One odd case is nsEnumeratorUtils.cpp, where the end of line comment
after a NS_DECL macro confused clang-format and made it stop
realizing that the NS_DECL thing was a complete statement. I also
added a blank line to that file before a declaration because I think
that is better.
Depends on D13183
Differential Revision: https://phabricator.services.mozilla.com/D13184
--HG--
extra : moz-landing-system : lando
(Unless there were other profiler actions, as I'm not sure yet whether it would
be safe to skip them when the profiler is paused; another bug should
investigate that.)
Differential Revision: https://phabricator.services.mozilla.com/D11308
--HG--
extra : moz-landing-system : lando
Potentially, if the watcher notification task failed to dispatch, we would have a cycle left between the WatchManager and the OwnerType
Differential Revision: https://phabricator.services.mozilla.com/D11857
--HG--
extra : moz-landing-system : lando
Sometimes when we call ShutdownWithTimeout on a thread pool, the unresponsive
thread that we leak will actually complete before the main thread is done.
In that case, the thread will dereference the thread shutdown context, so
we must intentionally leak it too.
Differential Revision: https://phabricator.services.mozilla.com/D10645
This method is necessary because some threads might be stuck making blocking
calls. This means the thread is not processing any events, and we're unable
to safely terminate it. Our solution here is to leak the stuck threads
instead of waiting for them and potentially causing a shutdown hang.
Differential Revision: https://phabricator.services.mozilla.com/D9601
--HG--
extra : moz-landing-system : lando
Instead of creating a timer and then setting the timer's target, we can
determine the timer's target and pass it in directly when the timer is
created. This reordering of steps is slightly more efficient, since
SetTarget() is both a virtual call and requires locking, both of which
can be skipped if we know the target at timer creation time. If we're
reusing the timer, we also don't need to repeatedly set the timer's
target: we can set the target once at timer creation, and then be done.
We can do this safely here because mTaskCategory doesn't change
throughout the life of the IdleTaskRunner; we make mTaskCategory `const`
to make this more explicit to the reader.
The NS_NewTimer* family of functions, when using a custom event target,
currently go through a path that looks something like:
auto timer = createTimer()
timer->SetTarget(target);
// call the requisite Init* function
return timer;
This setup is inefficient, because SetTarget requires the timer mutex to
be acquired. The mutex acquisition here is completely unnecessary,
because the timer hasn't yet been shared out to the wider world; we can
set the timer target without acquiring the mutex at all because we know
that no sharing is possible at this point.
This patch reworks things somewhat to make that possible.
If class A is derived from class B, then an instance of class A can be
converted to B via a static cast, so a slower QI is not needed.
Differential Revision: https://phabricator.services.mozilla.com/D6861
--HG--
extra : moz-landing-system : lando
This doesn't fix the problem, but at least moves the resulting assertion
closer to the problematic caller, and makes it easier to diagnose.
Differential Revision: https://phabricator.services.mozilla.com/D7042
--HG--
extra : rebase_source : 4a016d1dce77a269b886467c9343dab125079a8f
extra : amend_source : 4e9133e70932f382e0b5df9e487978b95d05781b
These maps hold strong references which complicate nsThread lifetime handling
considerably, and only have a couple of fringe uses. We have a linked list of
active threads that the thread manager can use for its internal enumeration
purposes, and the external uses are easily done away with, so there doesn't
seem to be much reason to keep the map around.
MozReview-Commit-ID: x7dsj6C4x8
--HG--
extra : source : 5f870621361012ba459943212d8c68a9ff81cb16
extra : intermediate-source : 89a0c0874d400dd324df6fc3627c0c47d130df19
extra : histedit_source : bbd7900e3d754bde925a411c10aa30a1d6e22edd
Most of the times when we automatically create nsThread wrappers for threads
that don't already have them, we don't actually need the event targets, since
those threads don't run XPCOM event loops. Aside from wasting memory, actually
creating these event loops can lead to leaks if a thread tries to dispatch a
runnable to the queue which creates a reference cycle with the thread.
Not creating the event queues for threads that don't actually need them helps
avoid those foot guns, and also makes it easier to figure out which treads
actually run XPCOM event loops.
MozReview-Commit-ID: Arck4VQqdne
--HG--
extra : source : a03a61d6d724503c3b7c5e31fe32ced1f5d1c219
extra : intermediate-source : 5152af6ab3e399216ef6db8f060c257b2ffbd330
extra : histedit_source : ef06000344416e0919f536d5720fa979d2d29c66%2C4671676b613dc3e3ec762edf5d72a2ffbe6fca3f
These maps hold strong references which complicate nsThread lifetime handling
considerably, and only have a couple of fringe uses. We have a linked list of
active threads that the thread manager can use for its internal enumeration
purposes, and the external uses are easily done away with, so there doesn't
seem to be much reason to keep the map around.
MozReview-Commit-ID: x7dsj6C4x8
--HG--
extra : rebase_source : 897e2d32d1dfee24d51459065925fb9b41fa543a
extra : source : 5f870621361012ba459943212d8c68a9ff81cb16
Most of the times when we automatically create nsThread wrappers for threads
that don't already have them, we don't actually need the event targets, since
those threads don't run XPCOM event loops. Aside from wasting memory, actually
creating these event loops can lead to leaks if a thread tries to dispatch a
runnable to the queue which creates a reference cycle with the thread.
Not creating the event queues for threads that don't actually need them helps
avoid those foot guns, and also makes it easier to figure out which treads
actually run XPCOM event loops.
MozReview-Commit-ID: Arck4VQqdne
--HG--
extra : rebase_source : fcf8fa50e748c4b54c3bb1997575d9ffd4cbaae1
extra : source : a03a61d6d724503c3b7c5e31fe32ced1f5d1c219
These maps hold strong references which complicate nsThread lifetime handling
considerably, and only have a couple of fringe uses. We have a linked list of
active threads that the thread manager can use for its internal enumeration
purposes, and the external uses are easily done away with, so there doesn't
seem to be much reason to keep the map around.
MozReview-Commit-ID: x7dsj6C4x8
--HG--
extra : rebase_source : 88c56fa4f5da97f33ade08d892c3d8c42666307e
Most of the times when we automatically create nsThread wrappers for threads
that don't already have them, we don't actually need the event targets, since
those threads don't run XPCOM event loops. Aside from wasting memory, actually
creating these event loops can lead to leaks if a thread tries to dispatch a
runnable to the queue which creates a reference cycle with the thread.
Not creating the event queues for threads that don't actually need them helps
avoid those foot guns, and also makes it easier to figure out which treads
actually run XPCOM event loops.
MozReview-Commit-ID: Arck4VQqdne
--HG--
extra : rebase_source : 02c5572b92ee48c11697d90941336e10c03d49cf
There are surprisingly many of them.
(Plus a couple of unnecessary checks after `new` calls that were nearby.)
--HG--
extra : rebase_source : 47b6d5d7c5c99b1b50b396daf7a3b67abfd74fc1
This moves the code that detects very low memory scenarios and grabs memory
reports from the main thread event-loop to the available memory tracker.
Besides removing the overhead of the check from the event-loop code this
increases the likeliness of the reports being gathered by sampling at a
higher frequency but only when we already detected a low-memory scenario. Last
but not least this add checks for low commit-space detection alongside low
virtual-memory detection.
Differential Revision: https://phabricator.services.mozilla.com/D3669
--HG--
extra : moz-landing-system : lando
This new id is added in the PerformanceInfo data and helps consumers distinguish
counters.
MozReview-Commit-ID: 7kEmqJcVggM
--HG--
extra : rebase_source : 40cca4c937f846db93ec1315036ad1bac04bc762
This takes 16 bytes off of the allocated size of each instance.
MozReview-Commit-ID: AhfN6MWvVL1
--HG--
extra : rebase_source : badc6ab690f2c4e0184ac0b51b29f81fb11279c6
extra : absorb_source : 0f685515a6946c89e9467c8b1e8548c989b1907b
extra : histedit_source : 7bfb5db39b23c1d262819c22a6e5fcd884c52504
This was moved out of nsThread in bug 1350432, but some dead code was left
behind.
MozReview-Commit-ID: BOhykHyIEPp
--HG--
extra : rebase_source : 21d4f95a019ba10851fba1efc588d6c2678aed85
Sylvestre Ledru
Nathan Froyd
Cosmin Sabou
Andrew McCreight
Jeff Muizelaar
Greg Tatum
Ehsan Akhgari
Eric Rahm
Olli Pettay
Gabriele Svelto
Jean-Yves Avenard
Gerald Squelart
Coroiu Cristina
Yaron Tausky
Brian Hackett
shindli
Valentin Gosu
Jim Blandy
Randell Jesup
Jonathan Kew
Bogdan Tara
Kris Maglione
Narcis Beleuzu
Noemi Erli
Christian Holler
Daniel Holbert
Jan de Mooij
Nicholas Nethercote
Masatoshi Kimura
Tom Ritter
Tarek Ziadé
Gurzau Raul