Closed Bug 1229441 Opened 9 years ago Closed 2 years ago

Profiler should not cancel gathering asynchronous profiles when stopped

Categories

(Core :: Gecko Profiler, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: mconley, Unassigned)

References

Details

STR (once bug 1193838 is RESOLVED):

1) With e10s enabled and the Gecko Profiler add-on installed, start profiling
2) In a remote tab, open the Web Console, and type:

while(true) {}

and press enter, to hang the content process.

3) Open the Browser Console, and type in:

Services.profiler.getProfileDataAsync().then((stuff) => {alert("Success! Got stuff!");}, (error) => {alert("Failure! Got error: " + error);});

and press enter

4) Stop the Profiler

ER:

After a while, a doorhanger should show up allowing you to stop the while-loop, which will unblock the content process and allow it to gather a profile, which will eventually resolve the promise with success.

AR:

The promise is rejected immediately.


Idea from https://bugzilla.mozilla.org/show_bug.cgi?id=1193838#c35 copied here:

1. Combine GeckoSampler and ProfileGatherer
2. Have GeckoSampler+ProfileGatherer be refcounted
3. Continue to offer the ProfileGatherer interface to PluginModuleParent and ContentParent, which will inc the refcount on GeckoSampler+ProfileGatherer.
4. On stopping the profiler, instead of directly deleting the GeckoSampler, have platform.cpp just drop the reference to it.
5. If an async profile gathering is still in progress, have PluginModuleParent and ContentParent ignore any profile start/stop/gather observer notifications from the profiler.
6. Once an async profile gathering is complete, have the PluginModuleParent and ContentParent check whether or not we're still profiling. If not, stop and drop the old reference to the gatherer. If still profiling, get a reference to the current GeckoSampler+ProfileGatherer and overwrite mGatherer with it (this might be the same reference, but if we've stopped and restarted the sampler while waiting for the profile to be gathered, it won't).

The consequence of this is that subprocesses that are still in the midst of sending up their profiles to the parent will not hear or obey stop/start/gather commands until the profile has finished gathering.

The API has changed quite a bit. And I didn't see a way to type in the Browser Console, and the devtools console doesn't have Services.
Also the profile-gathering function has an improved handling of non-responsive processes, it should give up after a few seconds.
So I'll tentatively mark this as fixed. Please reopen if you can reproduce something similar now.

Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED

(In reply to Gerald Squelart [:gerald] (he/him) from comment #1)

The API has changed quite a bit. And I didn't see a way to type in the Browser Console, and the devtools console doesn't have Services.

I can type at the bottom of the Browser Console, can't you?

(In reply to Julien Wajsberg [:julienw] from comment #2)

I can type at the bottom of the Browser Console, can't you?

Strange, no I cannot type in my regular Nightly's browser console, it's only showing the log.
But in a local build I get the js prompt and can type.

I tried the original STR, and as I suspected, there's a delay of about 5s after which the promise is resolved with a profile (missing the frozen process).
I think it's a better outcome than the promise being rejected, as it preserves whatever data could be extracted. The downside is that currently there is no mention of the missing process in the profile -- There are future plans to expose such issues (no bug yet).
Having a doorhanger to stop the infinite script would be nice too, but out of scope for the profiler.

So I'll keep this bug fixed.

The browser console's text box is unlocked with the devtools.chrome.enabled pref.

You need to log in before you can comment on or make changes to this bug.