This will make Firefox behavior in our performance CI tests more realistic:
These tests perform a warmup run which is killed before the regular
StartupCache write occurs. We don't really want to increase the time for
the warmup run, and I believe the behavior introduced in this patch makes
sense regardless - if there's no cache, writing out an incomplete cache
is better than waiting for more complete data and failing to write it out.
Differential Revision: https://phabricator.services.mozilla.com/D283122
It is replaced by a standard-conforming cast to (void) which is
understood the same by the compiler.
In some cases this could be replaced by `[[maybe_unused]]` but for
expression whose value is ignored, this requires storing to a temporary
variable first, so not going that way.
One notable change is that marking an object of type already_AddRefed as
unused used to implicitly leak memory. This is now made explicit.
Most of the heavy lifting was done through:
$ git grep -l '\<Unused\> <<' | xargs sed -i -e 's/Unused << /(void)/g'
$ git grep -l mozilla/Unused.h | xargs sed -i -e '/mozilla\/Unused.h/ d'
Differential Revision: https://phabricator.services.mozilla.com/D268706
It is replaced by a standard-conforming cast to (void) which is
understood the same by the compiler.
In some cases this could be replaced by `[[maybe_unused]]` but for
expression whose value is ignored, this requires storing to a temporary
variable first, so not going that way.
One notable change is that marking an object of type already_AddRefed as
unused used to implicitly leak memory. This is now made explicit.
Most of the heavy lifting was done through:
$ git grep -l '\<Unused\> <<' | xargs sed -i -e 's/Unused << /(void)/g'
$ git grep -l mozilla/Unused.h | xargs sed -i -e '/mozilla\/Unused.h/ d'
Differential Revision: https://phabricator.services.mozilla.com/D268706
It is replaced by a standard-conforming cast to (void) which is
understood the same by the compiler.
In some cases this could be replaced by `[[maybe_unused]]` but for
expression whose value is ignored, this requires storing to a temporary
variable first, so not going that way.
One notable change is that marking an object of type already_AddRefed as
unused used to implicitly leak memory. This is now made explicit.
Most of the heavy lifting was done through:
$ git grep -l '\<Unused\> <<' | xargs sed -i -e 's/Unused << /(void)/g'
$ git grep -l mozilla/Unused.h | xargs sed -i -e '/mozilla\/Unused.h/ d'
Differential Revision: https://phabricator.services.mozilla.com/D268706
It is replaced by a standard-conforming cast to (void) which is
understood the same by the compiler.
In some cases this could be replaced by `[[maybe_unused]]` but for
expression whose value is ignored, this requires storing to a temporary
variable first, so not going that way.
One notable change is that marking an object of type already_AddRefed as
unused used to implicitly leak memory. This is now made explicit.
Most of the heavy lifting was done through:
$ git grep -l '\<Unused\> <<' | xargs sed -i -e 's/Unused << /(void)/g'
$ git grep -l mozilla/Unused.h | xargs sed -i -e '/mozilla\/Unused.h/ d'
Differential Revision: https://phabricator.services.mozilla.com/D268706
The xpcshell test harness simulates a shutdown sequence for the parent
process (in testing\xpcshell\head.js) by manually advancing the shutdown
phases. This omits any call to AppShutdown::Init (which prevents us from
starting the terminator, as most likely wanted), resulting in
SetImpendingShutdown never been called in the parent process.
There might be a question here if the test harness should also simulate
a more complete shutdown sequence (currently some notifications and
phases are missing), but that is out of scope here.
For now we always set the shutdown flag on each advanceShutdownPhase
in order to ensure we can safely test against it in all configurations.
Differential Revision: https://phabricator.services.mozilla.com/D246922
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 change will effectively anticipate the fast shutdown by half a phase, making it more coherent with the phase it is defined to happen.
Depends on D160250
Differential Revision: https://phabricator.services.mozilla.com/D160628
This change will effectively anticipate the fast shutdown by half a phase, making it more coherent with the phase it is defined to happen.
Depends on D160250
Differential Revision: https://phabricator.services.mozilla.com/D160628
This change will effectively anticipate the fast shutdown by half a phase, making it more coherent with the phase it is defined to happen.
Depends on D160250
Differential Revision: https://phabricator.services.mozilla.com/D160628
This change will effectively anticipate the fast shutdown by half a phase, making it more coherent with the phase it is defined to happen.
Depends on D160250
Differential Revision: https://phabricator.services.mozilla.com/D160628
In order to get better shutdown hang reporting, we want to distinguish also the last phases, namely `XPCOMShutdownThreads`, `XPCOMShutdownMainThread` and `CCPostLastCycleCollection`.
This also makes `XPCOMShutdownNotified()` obsolete and we need to slightly re-arrange the watchdog function.
Differential Revision: https://phabricator.services.mozilla.com/D145433
The concept of a shutdown phase is meant to be a finite and named period of time where:
1. we can query if we are in or beyond a given phase
2. we may want to do a fast shutdown
3. we inform the terminator that we reached the next phase
4. if needed/wanted, we notify observers of the associated topic
5. we KillClearOnShutdown smart pointers associated with this phase
6. we do any further, phase-individual cleanup until the next phase starts
`AdvanceShutdownPhase(WithoutNotify)` now provides the functionality for 1. through 5. and must be the only place that calls `KillClearOnShutdown` in order to avoid misalignments.
In doing so, it becomes also the only caller of `MaybeFastShutdown`.
Please refer to bug 1768581 as working on this patch made us think if we should re-work the order inside AdvanceShutdownPhase.
Differential Revision: https://phabricator.services.mozilla.com/D145083
This profiler-controlling functions are used in very few places, so it's good to have them in a separate header to reduce dependencies.
On top of making GeckoProfiler.h lighter, this is actually needed for this bug, because a later patch adds MozPromise to profiler_start and others, which would have created a header loop (GeckoProfiler -> MozPromise -> Monitor -> CondVar -> GeckoProfiler) that makes the build fail.
Differential Revision: https://phabricator.services.mozilla.com/D139331
This stops destructing the stencils held by ScriptPreloader on release build,
to avoid the shutdown hang caused by thrashing.
Also:
* Move ScriptPreloader::Cleanup from xpcom-shutdown notification to ScriptPreloader destructor, given there's no reason for having 2 steps
* Replace ClearOnShutdown call with explicit function call for the singleton deletion, for simplicity
* Remove now-unused PostJSShutDown phase
Differential Revision: https://phabricator.services.mozilla.com/D130839
Sylvestre Ledru