1329861 - Disable async stack reporting when profiling Firefox using the Gecko profiler

Status: RESOLVED WONTFIX

(Core :: Gecko Profiler, defect)

Description

[(no longer active)]

While we find a solution to bug 1280819, we should probably consider disabling the async stack capture when using Gecko Profiler at least.

Note that in bug 1326134, we added reporting of this pref name to the profiles.  Not sure if we should rip it out here since in theory the user can still enable the pref while the profiler is active.

Comment 1

[(no longer active)]

Attachment: Disable async stack capturing while profiling using the Gecko Profiler

Comment 2

[Markus Stange [:mstange]]

Comment on attachment 8825271 [details] [diff] [review]
Disable async stack capturing while profiling using the Gecko Profiler

Review of attachment 8825271 [details] [diff] [review]:
-----------------------------------------------------------------

Ok... setting prefs while the profiler is not really following the principle of least surprise, but the alternative is worse. So let's do it.

::: tools/profiler/gecko/nsProfiler.cpp
@@ +32,5 @@
>  NS_IMPL_ISUPPORTS(nsProfiler, nsIProfiler)
>  
>  nsProfiler::nsProfiler()
> +  : mLockedForPrivateBrowsing(false),
> +    mAsyncStacksWereEnabled(false)

Gecko style is to put the comma at the beginning of the next line

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e39a1f381b85
Disable async stack capturing while profiling using the Gecko Profiler; r=mstange

Comment 4

[Boris Zbarsky [:bzbarsky]]

This is probably a good idea, however it means we won't be able to catch things like 1326105...

I'm not sure I have a good suggestion, though.  :(

I just wish async stack capture were less expensive.

Comment 5

[Phil Ringnalda (:philor)]

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/5c27bf4d948a for https://treeherder.mozilla.org/logviewer.html#?job_id=67510313&repo=mozilla-inbound and https://treeherder.mozilla.org/logviewer.html#?job_id=67510166&repo=mozilla-inbound (and beware, 330 second hangs kill the run, so there may be other tests that would have run after those which would also fail).

Comment 6

[(no longer active)]

After some more thought, this is probably not the right place to set this pref, given that StartProfiler() and StopProfiler() are also called by the devtools profiler.  Since this feature is only used in devtools, disabling it for devtools profiler seems questionable.  Instead, I'm going to do this in the Gecko Profiler add-on in this PR: <https://github.com/mstange/Gecko-Profiler-Addon/pull/18>

Comment 7

[Jared Hirsch [:jhirsch] (he/him) (Needinfo please)]

For future reference, the Gecko Profiler addon PR mentioned in comment 6 has migrated to a new URL:

https://github.com/devtools-html/Gecko-Profiler-Addon/pull/18