1103094 - Start profiling subprocesses if the parent process is already profiling

Status: RESOLVED FIXED

(Core :: Gecko Profiler, defect, P3)

Description

[Mike Conley (:mconley) (:⚙️)]

When subprocesses are started, they should start profiling if the parent process is already profiling.

Comment 1

[Mike Conley (:mconley) (:⚙️)]

Attachment: MozReview Request: Bug 1103094 - Add accessor for nsIProfilerStartParams to nsIProfiler. r?BenWa

Bug 1103094 - Add accessor for nsIProfilerStartParams to nsIProfiler. r?BenWa

Comment 2

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8645982 [details]
MozReview Request: Bug 1103094 - Add accessor for nsIProfilerStartParams to nsIProfiler. r?BenWa

Bug 1103094 - Add accessor for nsIProfilerStartParams to nsIProfiler. r?BenWa

Comment 3

[Mike Conley (:mconley) (:⚙️)]

Attachment: MozReview Request: Bug 1103094 - Start profiling subprocesses if the parent process is already profiling. r?BenWa

Bug 1103094 - Start profiling subprocesses if the parent process is already profiling. r?BenWa

Comment 4

[Mike Conley (:mconley) (:⚙️)]

Attachment: MozReview Request: Bug 1193838 - Make ProfileGatherer exist during the lifetime of a GeckoSampler. r?BenWa

Bug 1193838 - Make ProfileGatherer exist during the lifetime of a GeckoSampler. r?BenWa

Comment 5

[Mike Conley (:mconley) (:⚙️)]

Attachment: MozReview Request: Bug 1193838 - Expose ProfileGatherer as an nsISupports through nsIProfiler for process parent actors. r?BenWa

Bug 1193838 - Expose ProfileGatherer as an nsISupports through nsIProfiler for process parent actors. r?BenWa

We need to let ContentParent and PluginModuleParent get a reference to the ProfileGatherer
during the window of time that we're profiling so that if they start to die (the actor is
starting to go away), they have a gatherer they can send their last profile data to.

Comment 6

[Mike Conley (:mconley) (:⚙️)]

Attachment: MozReview Request: Bug 1193838 - Allow ProfileGatherer to gather profiles from exiting processes. r?BenWa

Bug 1193838 - Allow ProfileGatherer to gather profiles from exiting processes. r?BenWa

Comment 7

[Mike Conley (:mconley) (:⚙️)]

Attachment: MozReview Request: Add some instrumentation and logging

Add some instrumentation and logging

Comment 8

[Mike Conley (:mconley) (:⚙️)]

Attachment: MozReview Request: Make talos use async profile dumping stuff

Make talos use async profile dumping stuff

Comment 9

[Joel Maher ( :jmaher ) (UTC -8)]

we have a patch for review, but no action in the last 4+ weeks, :mconley, what is the next step here?

Comment 10

[Mike Conley (:mconley) (:⚙️)]

The goal to work on this got pushed off to this quarter, so I should hopefully be able to drive this through in the next week or so.

Comment 11

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8645982 [details]
MozReview Request: Bug 1103094 - Add accessor for nsIProfilerStartParams to nsIProfiler. r?BenWa

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/15569/diff/2-3/

Comment 12

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8669886 [details]
MozReview Request: Bug 1103094 - Start profiling subprocesses if the parent process is already profiling. r?BenWa

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/21279/diff/1-2/

Comment 13

[Mike Conley (:mconley) (:⚙️)]

Sorry it's kinda taken me forever to get back to this[1]. Also, BenWa, if you know of some other way I can do this Vector <-> Array conversion, please let me know. (I seem to recall us using Vectors to be more portable for non-Mozilla things).

Comment 14

[Benoit Girard (:BenWa)]

Comment on attachment 8645982 [details]
MozReview Request: Bug 1103094 - Add accessor for nsIProfilerStartParams to nsIProfiler. r?BenWa

https://reviewboard.mozilla.org/r/15569/#review23747

::: tools/profiler/gecko/nsIProfiler.idl:63
(Diff revision 3)
> +  readonly attribute nsIProfilerStartParams startParams;

Is it worth describing what happens if the profiling isn't started? Maybe, maybe not? I'll leave that decission up to you.

Looks like we'd hit a NS_WARN if the profiler isn't running and return a trivial 0/0/empty/empty object.

Comment 15

[Benoit Girard (:BenWa)]

Comment on attachment 8669886 [details]
MozReview Request: Bug 1103094 - Start profiling subprocesses if the parent process is already profiling. r?BenWa

https://reviewboard.mozilla.org/r/21279/#review23749

Love it, cleans up the code too!

Comment 16

[Mike Conley (:mconley) (:⚙️)]

https://reviewboard.mozilla.org/r/15569/#review23747

> Is it worth describing what happens if the profiling isn't started? Maybe, maybe not? I'll leave that decission up to you.
> 
> Looks like we'd hit a NS_WARN if the profiler isn't running and return a trivial 0/0/empty/empty object.

Even better - I'll just return set the incoming outparam to nullptr in that case. Thanks!

Comment 17

[Mike Conley (:mconley) (:⚙️)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=172f3cad7b6c

Comment 18

[Mike Conley (:mconley) (:⚙️)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f3c1c298a1fe

Comment 19

[Mike Conley (:mconley) (:⚙️)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ec3fd280607c

Comment 20

[Mike Conley (:mconley) (:⚙️)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/fd8dc51c5a5c7fa98101cc2e1486177a36739968
Bug 1103094 - Add accessor for nsIProfilerStartParams to nsIProfiler. r=BenWa

https://hg.mozilla.org/integration/mozilla-inbound/rev/f04beafaf860912b22022d0b787134646cf9c3f1
Bug 1103094 - Start profiling subprocesses if the parent process is already profiling. r=BenWa

Comment 21

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/fd8dc51c5a5c
https://hg.mozilla.org/mozilla-central/rev/f04beafaf860

Comment 22

[Mike Conley (:mconley) (:⚙️)]

Attachment: Bugnotes

http://people.mozilla.org/~mconley2/bugnotes/bug-1103094.html