I got this when clicking the Analyze button in the Profiler UI, when the Cleopatra tab was opened in the background. This seems to be a deadlock between the parent process, the content process and the plugin-container process running Flash.

I'm going to morph this a bit to reflect how we need to fix this. Bug 1105039 has a lot of the original discussion though.

Having spoken with mstange, ehsan and a few others, here's what I propose: 1) Add two new functions to nsIProfiler - something like GetProfileAsync and GetProfileDataAsync. Both of these will assume JS context, and return DOM Promises that will resolve once the profile has been assembled. Also possibly expose a boolean state of "gatheringProfiles" or something to indicate whether or not async profiles are still coming in. 2) Once the TableTicker (or some other profiler component - I guess I'll call it X, since I don't know where this stuff will go) has finished gathering the profile for the browser process, it'll send an observer notification "profiler-subprocess", which will pass some item that can be incremented by listeners of of that notification. The end-result being that once the notification has fired, X will know how many async profiles we're waiting for. 3) At this point, X will prep the SpliceableJSONWriter it's using to hold thread data, and then hold a reference to it, along with the value we got back from the observer notification saying how many async profiles we're waiting for. It should also mark the nsIProfiler "gatheringAsyncProfiles" readonly attribute we added to true. Any calls to get profiles that come in while that value is true will fail. 4) The profiler-subprocess notification will be picked up by IPDL Parents, which will send messages to their children asking for async profiles. 5) The child processes receive the messages, gather their profiles, and then send an async message back up to the parent. 6) The parent receives the message, and informs the profiler through some interface. 7) The profiler receives the profile (or an error from one of the children, which should probably result in Promise rejection), and appends it to the SpliceableJSONWriter it's holding onto. It also decrements the count of how many it's waiting for. 8) Once the counter has reached 0, resolve the Promise with the value of the SpliceableJSONWriter. Cc'ing Shu, who's been doing some pretty serious work in bug 1154115, and might want to know / have suggestions about what I'm doing in here.

I don't know too much about e10s, but this sounds fine to me. Once bug 1154115 lands, streaming time will be about ~10x faster and the data size ~20x smaller. For small enough profiles, the sync version is probably preferable to the async overhead? I wonder if we should expose some estimate of how long the profile will take to stream.

Bug 1116188 - Split up TableTicker::StreamJSON into two static functions for streaming the head and foot of a profile. r=?

Bug 1116188 - [WIP] Add async ProfileGatherer as the mechanism for gathering profiles from subprocesses. r=?

https://reviewboard.mozilla.org/r/10011/#review8803 ::: tools/profiler/nsIProfiler.idl:88 (Diff revision 1) > + > +/* > +[uuid(bc26ed86-8ac7-446c-b9af-7741bdb7970a)] > +interface nsIProfilerAsyncGatherer : nsISupports > +{ > + [noscript] void WillAppendOOPProfile(); > + [noscript] void AppendOOPProfile(in string aProfile); > +};*/ Remove dead interface. ::: tools/profiler/nsProfiler.cpp:224 (Diff revision 1) > + printf("\nGetProfileDataAsync called.\n"); Remove logging. ::: tools/profiler/nsProfiler.cpp:249 (Diff revision 1) > + printf("\nCalling the async bit.\n"); Remove logging. ::: tools/profiler/platform.cpp:588 (Diff revision 1) > + printf("\nmozilla_sampler_get_profile_data_async called.\n"); Remove logging. ::: tools/profiler/platform.cpp:594 (Diff revision 1) > + printf("\nCalling ToJSObjectAsync...\n"); Remove logging. ::: tools/profiler/platform.cpp:584 (Diff revision 1) > +JSObject *mozilla_sampler_get_profile_data_async(JSContext *aCx, I don't think this function should actually return anything. Should probably be void, or a bool / nsresult to indicate success or failure kicking things off. ::: tools/profiler/nsProfiler.cpp:26 (Diff revision 1) > +using mozilla::dom::Promise; Probably put above std::string to try to keep it alphabetical. ::: tools/profiler/TableTicker.cpp:472 (Diff revision 1) > + aTicker->SetPaused(false); The ticker shouldn't be pased here anyways - it got unpaused on 448. ::: tools/profiler/TableTicker.cpp:245 (Diff revision 1) > + printf("\nTableTicker::ToJSObjectAsync called.\n"); Remove logging ::: tools/profiler/TableTicker.cpp:250 (Diff revision 1) > + printf("\nCreating a ProfileGatherer.\n"); Remove logging ::: tools/profiler/TableTicker.cpp:252 (Diff revision 1) > + mGatherer = new ProfileGatherer(this, aCx, aSinceTime, aPromise); Nothing drops this reference. ::: tools/profiler/TableTicker.cpp:261 (Diff revision 1) > + return nullptr; Yeah, this shouldn't return a pointer to anything. Probably just be void, or bool / nsresult indicating success or failure. ::: tools/profiler/ProfileGatherer.cpp:81 (Diff revision 1) > + printf("\n HOLY SHIT RESOLVING\n"); Get rid of logging. And swears. ::: tools/profiler/ProfileGatherer.cpp:90 (Diff revision 1) > + mPromise->MaybeResolve(mCx, val); Currently crashing here with: Assertion failure: cx->runtime()->requestDepth || cx->runtime()->isHeapBusy(), at /Users/mikeconley/Projects/mozilla-central/js/src/jscntxt.cpp:1172 ::: tools/profiler/ProfileGatherer.cpp:60 (Diff revision 1) > + printf("\nSubprocess reported it will append OOP profile\n"); Remove logging. ::: tools/profiler/ProfileGatherer.cpp:48 (Diff revision 1) > + printf("\nAll profiles received - resolving\n"); Remove logging. ::: tools/profiler/ProfileGatherer.cpp:30 (Diff revision 1) > + printf("\nSubprocess sent the parent an OOP profile\n"); Remove logging. ::: tools/profiler/ProfileGatherer.cpp:51 (Diff revision 1) > + Finish(); > + Resolve(); Is there a reason why these are separate?

Comment on attachment 8614698 [details] MozReview Request: Bug 1116188 - Add async ProfileGatherer as the mechanism for gathering profiles from subprocesses. r?bgirard Bug 1116188 - [WIP] Add async ProfileGatherer as the mechanism for gathering profiles from subprocesses. r=?

Comment on attachment 8614698 [details] MozReview Request: Bug 1116188 - Add async ProfileGatherer as the mechanism for gathering profiles from subprocesses. r?bgirard Bug 1116188 - Add async ProfileGatherer as the mechanism for gathering profiles from subprocesses. r=?

https://reviewboard.mozilla.org/r/10011/#review8951 ::: tools/profiler/ProfileGatherer.h:11 (Diff revision 3) > +struct JSContext; No need for this struct. ::: tools/profiler/ProfileGatherer.h:27 (Diff revision 3) > + void Finish(); This should be private, I think. ::: tools/profiler/ProfileJSONWriter.cpp (Diff revision 3) > - MOZ_ASSERT(strlen(mChunkList[i].get()) == mChunkLengths[i]); This is something shu said he was going to remove in some patch. Should check to see where that patch is, and rebase on top of it. ::: tools/profiler/TableTicker.h:18 (Diff revision 3) > +#include "ProfileGatherer.h" mozilla/ProfileGatherer.h? ::: tools/profiler/TableTicker.cpp:248 (Diff revision 3) > + // Create ourselves a ProfileGatherer and hold a reference to it. This comment really doesn't add much. ::: tools/profiler/TableTicker.cpp:428 (Diff revision 3) > + // We'll end this array property in Finish. "in Finish" -> "when we stream the foot".

Bug 1116188 - Remove bogus assertion. r?shu

Comment on attachment 8614697 [details] MozReview Request: Bug 1116188 - Split up TableTicker::StreamJSON into two static functions for streaming the head and foot of a profile. r?bgirard Bug 1116188 - Split up TableTicker::StreamJSON into two static functions for streaming the head and foot of a profile. r?shu

Comment on attachment 8614698 [details] MozReview Request: Bug 1116188 - Add async ProfileGatherer as the mechanism for gathering profiles from subprocesses. r?bgirard Bug 1116188 - Add async ProfileGatherer as the mechanism for gathering profiles from subprocesses. r?bgirard,billm

Hey billm - for the patch I've asked review on, I really only care that I'm doing the Promise thing correctly, and the JS_ParseJSON thing correctly. Nobody in the Toronto office is really confident about it, and they suggested you, bholley or bz as potential reviewers to make sure what I'm doing there is sane.

Comment on attachment 8614697 [details] MozReview Request: Bug 1116188 - Split up TableTicker::StreamJSON into two static functions for streaming the head and foot of a profile. r?bgirard Bug 1116188 - Split up TableTicker::StreamJSON into two static functions for streaming the head and foot of a profile. r?bgirard

Comment on attachment 8614698 [details] MozReview Request: Bug 1116188 - Add async ProfileGatherer as the mechanism for gathering profiles from subprocesses. r?bgirard Bug 1116188 - Add async ProfileGatherer as the mechanism for gathering profiles from subprocesses. r?bgirard,billm

Comment on attachment 8614698 [details] MozReview Request: Bug 1116188 - Add async ProfileGatherer as the mechanism for gathering profiles from subprocesses. r?bgirard Bug 1116188 - Add async ProfileGatherer as the mechanism for gathering profiles from subprocesses. r?bgirard,billm

This isn't working with the Gecko Profiler add-on - apparently it's having a hard time getting at the nsIScriptGlobalObject in the nsProfiler bit. Will poke at this a bit.

Comment on attachment 8614698 [details] MozReview Request: Bug 1116188 - Add async ProfileGatherer as the mechanism for gathering profiles from subprocesses. r?bgirard Bug 1116188 - Add async ProfileGatherer as the mechanism for gathering profiles from subprocesses. r?bgirard,bz

Hey bz, The part I'm hoping you can look at is the creation and resolution of the DOM Promise. The creation occurs on nsProfile::GetProfileDataAsync - and I just want to make sure I'm doing it correctly with the JS Context I'm being handed. The Promise gets handed down through a lot of profiler plumbing, and ProfileGatherer holds a reference to it - I don't think you need to care about that bit. Eventually, when all profiles come in from the subprocesses, ProfileGatherer::Finish is called - this is where I parse the JSON that's been constructed and resolve the Promise. This is the other place I want to make sure I'm doing things correctly. You can safely ignore the rest of the patch, I think.

Comment on attachment 8614698 [details] MozReview Request: Bug 1116188 - Add async ProfileGatherer as the mechanism for gathering profiles from subprocesses. r?bgirard https://reviewboard.mozilla.org/r/10015/#review9127 The GetProfileDataAsync look just fine. The code in Finish() is not really handling error cases very well. It would be better if it reused a setup like the Fetch code I pointed you at. A followup is probably OK as long as it happens expeditiously, but it needs to happen, because the current code is asserting things that are just false (e.g. that JSON parsing never throws) and is either leaving exceptions on cx or ending up reporting them somewhere random and not rejecting the promise with them. Now in the common case there won't be exceptions here, I guess, but nothing really precludes them.

Comment on attachment 8614698 [details] MozReview Request: Bug 1116188 - Add async ProfileGatherer as the mechanism for gathering profiles from subprocesses. r?bgirard https://reviewboard.mozilla.org/r/10015/#review9197 ::: tools/profiler/moz.build:26 (Diff revision 6) > + 'TableTicker.h', TableTicker and ProfileJSONWriter are private API. I'd rather not export them unless we have an excellent reason. ::: tools/profiler/ProfileGatherer.cpp:27 (Diff revision 6) > +ProfileGatherer::AppendOOPProfile(const char* aProfile) Nice, very simple! ::: tools/profiler/GeckoProfilerImpl.h:160 (Diff revision 6) > + mozilla::dom::Promise* aPromise = 0) Would it be possible to have a callback here? You can move the promise into nsProfiler.cpp. I've got patches that are landing to make the profiler standalone. This would let the async stuff work with minilar effort in the standalone profiler.

Comment on attachment 8614697 [details] MozReview Request: Bug 1116188 - Split up TableTicker::StreamJSON into two static functions for streaming the head and foot of a profile. r?bgirard Bug 1116188 - Split up TableTicker::StreamJSON into two static functions for streaming the head and foot of a profile. r?bgirard

Just getting this down from my notes: IRL, BenWa has asked me to avoid breaking up the JSON streaming into a head and a foot. Our strategy is to send a new observer notification around to have ContentParent and PluginModuleChromeParent gather the profiles from their subprocesses asynchronously. When the profiles return to those Parents, the Parents stash them, and let ProfileGatherer know (without passing them to ProfileGatherer). When the number of pending profiles goes down, ProfileGatherer will then be responsible for constructing the JSON blob from start to finish. This includes having ContentParent and PluginModuleChromeParent continue to respond to the profile-subprocess observer notification (but responding with the stashed profile their processes returned, before truncating them out). This will keep StreamJSON as is, and move all responsibility for the async work into ProfileGatherer.

Comment on attachment 8614698 [details] MozReview Request: Bug 1116188 - Add async ProfileGatherer as the mechanism for gathering profiles from subprocesses. r?bgirard Bug 1116188 - [WIP] Add async ProfileGatherer as the mechanism for gathering profiles from subprocesses.

Comment on attachment 8614698 [details] MozReview Request: Bug 1116188 - Add async ProfileGatherer as the mechanism for gathering profiles from subprocesses. r?bgirard Bug 1116188 - Add async ProfileGatherer as the mechanism for gathering profiles from subprocesses. r?bgirard

https://reviewboard.mozilla.org/r/10015/#review9357 > Would it be possible to have a callback here? You can move the promise into nsProfiler.cpp. I've got patches that are landing to make the profiler standalone. This would let the async stuff work with minilar effort in the standalone profiler. We bailed on this idea in person. > TableTicker and ProfileJSONWriter are private API. I'd rather not export them unless we have an excellent reason. Ah, good to know, thanks.

Comment on attachment 8614698 [details] MozReview Request: Bug 1116188 - Add async ProfileGatherer as the mechanism for gathering profiles from subprocesses. r?bgirard Bug 1116188 - Add async ProfileGatherer as the mechanism for gathering profiles from subprocesses. r?bgirard

Comment on attachment 8614698 [details] MozReview Request: Bug 1116188 - Add async ProfileGatherer as the mechanism for gathering profiles from subprocesses. r?bgirard Bug 1116188 - Add async ProfileGatherer as the mechanism for gathering profiles from subprocesses. r?bgirard

Comment on attachment 8614698 [details] MozReview Request: Bug 1116188 - Add async ProfileGatherer as the mechanism for gathering profiles from subprocesses. r?bgirard Bug 1116188 - Add async ProfileGatherer as the mechanism for gathering profiles from subprocesses. r?bgirard

Comment on attachment 8614698 [details] MozReview Request: Bug 1116188 - Add async ProfileGatherer as the mechanism for gathering profiles from subprocesses. r?bgirard Bug 1116188 - Add async ProfileGatherer as the mechanism for gathering profiles from subprocesses. r?bgirard

Comment on attachment 8614698 [details] MozReview Request: Bug 1116188 - Add async ProfileGatherer as the mechanism for gathering profiles from subprocesses. r?bgirard Bug 1116188 - Add async ProfileGatherer as the mechanism for gathering profiles from subprocesses. r?bgirard

Bah - sorry I keep adding and removing you. I keep hitting build errors on other platforms in my try pushes.

Comment on attachment 8614698 [details] MozReview Request: Bug 1116188 - Add async ProfileGatherer as the mechanism for gathering profiles from subprocesses. r?bgirard Bug 1116188 - Add async ProfileGatherer as the mechanism for gathering profiles from subprocesses. r?bgirard

Comment on attachment 8614698 [details] MozReview Request: Bug 1116188 - Add async ProfileGatherer as the mechanism for gathering profiles from subprocesses. r?bgirard Bug 1116188 - Add async ProfileGatherer as the mechanism for gathering profiles from subprocesses. r?bgirard

Comment on attachment 8614698 [details] MozReview Request: Bug 1116188 - Add async ProfileGatherer as the mechanism for gathering profiles from subprocesses. r?bgirard Bug 1116188 - Add async ProfileGatherer as the mechanism for gathering profiles from subprocesses. r?bgirard

https://reviewboard.mozilla.org/r/10015/#review9499 ::: tools/profiler/TableTicker.h:45 (Diff revision 14) > TableTicker(double aInterval, int aEntrySize, > const char** aFeatures, uint32_t aFeatureCount, > - const char** aThreadNameFilters, uint32_t aFilterCount) > - : Sampler(aInterval, true, aEntrySize) > + const char** aThreadNameFilters, uint32_t aFilterCount); > + ~TableTicker(); I had to take the implementations of the TableTicker constructor and destructor out of the header. That way, I could forward declare ProfileGatherer, which was necessary in order to build on Linux and Windows properly. See https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Include_dependencies#Forward_declarations_and_nsRefPtr

Comment on attachment 8614698 [details] MozReview Request: Bug 1116188 - Add async ProfileGatherer as the mechanism for gathering profiles from subprocesses. r?bgirard https://reviewboard.mozilla.org/r/10015/#review9551 Ship It!