1274041 - make jprof work better with e10s

Status: RESOLVED FIXED

(Core :: General, defect, P4)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Jesup did some work in bug 674384, but some more work needs to be done with current e10s to make jprof usable with e10s.

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: MozReview Request: Bug 1274041 - Actually define JPROF_STATIC as it was meant to be defined. r?jesup

I've had this in my tree for a while; it just fixes what look like a
silly mistake.

Review commit: https://reviewboard.mozilla.org/r/53696/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/53696/

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: MozReview Request: Bug 1274041 - Initialize jprof in child process. r?jesup

This makes a function call in XRE_InitChildProcess for the child process that
currently exists in XREMain::XRE_mainStartup for the parent process.

This allows signals that jprof uses to be sent to a child process to
profile that child process.

Review commit: https://reviewboard.mozilla.org/r/53698/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/53698/

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: MozReview Request: Bug 1274041 - Make child process write its memory map to a different file name. r?jesup

This makes the child process write its memory map to a different file
name, just like it does for the jprof log.  It tries to share the
variables between the two so that they're both connected in the code and
consistent with each other.

Note that I haven't yet written the patch to make jprof.cpp *read* the
map from that file, so this currently requires manually renaming the
generated map with the numeric suffix to jprof-map in order to run
jprof.  I should probably write that patch eventually, but I haven't
actually needed to.

This at least prevents the child process's map file from being
overwritten by the parent's a fraction of a second later.

Review commit: https://reviewboard.mozilla.org/r/53700/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/53700/

Comment 4

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8754055 [details]
MozReview Request: Bug 1274041 - Make child process write its memory map to a different file name.  r?jesup

https://reviewboard.mozilla.org/r/53700/#review50556

::: tools/jprof/stub/libmalloc.cpp:173
(Diff revision 1)
> +  if (gIsSlave)
> +    snprintf(filename, sizeof(filename), "%s-%d", M_MAPFILE, gFilenamePID);

So (probably) jprof.map-pid
I guess we don't have easy access to the executable file's name here.

Comment 5

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8754054 [details]
MozReview Request: Bug 1274041 - Initialize jprof in child process.  r?jesup

https://reviewboard.mozilla.org/r/53698/#review50558

Comment 6

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8754053 [details]
MozReview Request: Bug 1274041 - Actually define JPROF_STATIC as it was meant to be defined.  r?jesup

https://reviewboard.mozilla.org/r/53696/#review50560

sure, though it largely doesn't matter.

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(In reply to Randell Jesup [:jesup] from comment #4)
> So (probably) jprof.map-pid
> I guess we don't have easy access to the executable file's name here.

Was there something you wanted me to change here?

Comment 8

[Randell Jesup [:jesup] (needinfo me)]

(In reply to David Baron [:dbaron] ⌚️UTC-7 (review requests must explain patch) from comment #7)
> (In reply to Randell Jesup [:jesup] from comment #4)
> > So (probably) jprof.map-pid
> > I guess we don't have easy access to the executable file's name here.
> 
> Was there something you wanted me to change here?

No - some tweaks may need to be made to jprof (leaky.cpp) and/or document how to do this in the readme.html.  That can be a separate patch/bug

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b4dd21453f98fbaa70f7f851393b40f1cb019d57
Bug 1274041 - Actually define JPROF_STATIC as it was meant to be defined.  r=jesup

https://hg.mozilla.org/integration/mozilla-inbound/rev/a634e9b19a3b2fb41f9bbd15683d0f7a7e6a85ed
Bug 1274041 - Initialize jprof in child process.  r=jesup

https://hg.mozilla.org/integration/mozilla-inbound/rev/f81a68cdd9f192c2f85ac0aca1a6d7be892a0335
Bug 1274041 - Make child process write its memory map to a different file name.  r=jesup

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/b4dd21453f98
https://hg.mozilla.org/mozilla-central/rev/a634e9b19a3b
https://hg.mozilla.org/mozilla-central/rev/f81a68cdd9f1