Bug 1330184 - Allow notifying observers for profiler state changes on background threads. r=njn
MozReview-Commit-ID: GlkVwGTa2b4
--- a/tools/profiler/core/platform.cpp
+++ b/tools/profiler/core/platform.cpp
@@ -526,29 +526,16 @@ MOZ_THREAD_LOCAL(ThreadInfo*) TLSInfo::s
// This second pointer isn't ideal, but does provide a way to satisfy those
// constraints. TLSInfo manages it, except for the uses in
// profiler_call_{enter,exit}.
MOZ_THREAD_LOCAL(PseudoStack*) sPseudoStack;
// The name of the main thread.
static const char* const kMainThreadName = "GeckoMain";
-static bool
-CanNotifyObservers()
-{
- MOZ_RELEASE_ASSERT(NS_IsMainThread());
-
-#if defined(GP_OS_android)
- // Android ANR reporter uses the profiler off the main thread.
- return NS_IsMainThread();
-#else
- return true;
-#endif
-}
-
////////////////////////////////////////////////////////////////////////
// BEGIN tick/unwinding code
// TickSample contains all the information needed by Tick(). Some of it is
// pointers to long-lived things, and some of it is sampled just before the
// call to Tick().
class TickSample {
public:
@@ -1986,52 +1973,51 @@ locked_register_thread(PSLockRef aLock,
info->PollJSSampling();
}
}
CorePS::LiveThreads(aLock).push_back(info);
}
static void
+NotifyObservers(const char* aTopic, nsISupports* aSubject = nullptr)
+{
+ if (!NS_IsMainThread()) {
+ // Dispatch a task to the main thread that notifies observers.
+ // If NotifyObservers is called both on and off the main thread within a
+ // short time, the order of the notifications can be different from the
+ // order of the calls to NotifyObservers.
+ // Getting the order 100% right isn't that important at the moment, because
+ // these notifications are only observed in the parent process, where the
+ // profiler_* functions are currently only called on the main thread.
+ nsCOMPtr<nsISupports> subject = aSubject;
+ NS_DispatchToMainThread(NS_NewRunnableFunction([=] {
+ NotifyObservers(aTopic, subject);
+ }));
+ return;
+ }
+
+ if (nsCOMPtr<nsIObserverService> os = services::GetObserverService()) {
+ os->NotifyObservers(aSubject, aTopic, nullptr);
+ }
+}
+
+static void
NotifyProfilerStarted(const int aEntries, double aInterval, uint32_t aFeatures,
const char** aFilters, uint32_t aFilterCount)
{
- if (!CanNotifyObservers()) {
- return;
- }
-
- nsCOMPtr<nsIObserverService> os = mozilla::services::GetObserverService();
- if (!os) {
- return;
- }
-
nsTArray<nsCString> filtersArray;
for (size_t i = 0; i < aFilterCount; ++i) {
filtersArray.AppendElement(aFilters[i]);
}
nsCOMPtr<nsIProfilerStartParams> params =
new nsProfilerStartParams(aEntries, aInterval, aFeatures, filtersArray);
- os->NotifyObservers(params, "profiler-started", nullptr);
-}
-
-static void
-NotifyObservers(const char* aTopic)
-{
- if (!CanNotifyObservers()) {
- return;
- }
-
- nsCOMPtr<nsIObserverService> os = mozilla::services::GetObserverService();
- if (!os) {
- return;
- }
-
- os->NotifyObservers(nullptr, aTopic, nullptr);
+ NotifyObservers("profiler-started", params);
}
static void
locked_profiler_start(PSLockRef aLock, const int aEntries, double aInterval,
uint32_t aFeatures,
const char** aFilters, uint32_t aFilterCount);
void
@@ -2503,18 +2489,20 @@ profiler_stop()
if (!ActivePS::Exists(lock)) {
return;
}
samplerThread = locked_profiler_stop(lock);
}
// We notify observers with gPSMutex unlocked. Otherwise we might get a
- // deadlock, if code run by the observer calls a profiler function that locks
- // gPSMutex. (This has been seen in practise in bug 1346356.)
+ // deadlock, if code run by these functions calls a profiler function that
+ // locks gPSMutex, for example when it wants to insert a marker.
+ // (This has been seen in practise in bug 1346356, when we were still firing
+ // these notifications synchronously.)
NotifyObservers("profiler-stopped");
// We delete with gPSMutex unlocked. Otherwise we would get a deadlock: we
// would be waiting here with gPSMutex locked for SamplerThread::Run() to
// return so the join operation within the destructor can complete, but Run()
// needs to lock gPSMutex to return.
//
// Because this call occurs with gPSMutex unlocked, it -- including the final