653311 - Jprof doesn't work for many (most?) Linux distros

Status: RESOLVED FIXED

(Core :: General, defect)

Description

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

Jprof seems to have been somewhat left behind by modern GCC/Linux distros - out of the box it fails to save any stacks on Ubuntu 8.04 LTS, and probably a bunch of others.

Largely this is due to the roll-your-own stack backtrace, which is very tied to the ABI, GCC setup and glibc.  It at minimum should use glibc's backtrace() or equivalent.  Even better would have it working under Windows as well using CaptureStackBackTrace() or equivalent.

Related bug: Bug 187053 (Support jprof on Windows)

Comment 1

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

Attachment: Patch to make jprof work on Ubuntu 8.04LTS and general improvements

Note: patch includes support for reporting jprof results on a per-thread basis, though under linux by default jprof will only capture the main thread.  There are external preload libraries (made for use with gprof) that could be adapted to make jprof capture all threads, in which case this ability would be useful.

Reviewed with JST Simulacrum - passes cleanly except for fprintf string lengths.

Improved output - includes %ages after all counts, though caller/callee count percentages can be misleading if there's a loop (just as the counts have always been misleading there).  Also includes "(self)" for recursive calls.

The only issue I anticipate might be the #includes of sys/syscall.h and execinfo.h.  Since jprof is currently a linux/glib tool, I don't think this is an issue (shouldn't break anywhere --enable-jprof isn't already broken).

Comment 2

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

Comment on attachment 528815 [details] [diff] [review]
Patch to make jprof work on Ubuntu 8.04LTS and general improvements

I probably should add myself to Contributors as well...

Requesting review from dbaron (other reviews welcome)

Comment 3

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

Comment on attachment 528815 [details] [diff] [review]
Patch to make jprof work on Ubuntu 8.04LTS and general improvements

Review of attachment 528815 [details] [diff] [review]:

Adding jim nance

Comment 4

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

Added bz; made bug 187053 blocked by this though technically it isn't.

Comment 5

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

Comment on attachment 528815 [details] [diff] [review]
Patch to make jprof work on Ubuntu 8.04LTS and general improvements

Added r=? for bzbarsky, who has worked in jprof

Comment 6

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

While waiting for reviews and my checkin privs to be renewed, I'm working on updating the README.html (and one minor tweak to the JP_PERIOD code to allow "0.001").  New patch to be uploaded RSN.  Deltas from this patch to code should be minimal, so looking at this patch won't be wasted work.

Comment 7

[Boris Zbarsky [:bzbarsky]]

I'm not really competent to review this (nor do I think it needs a second reviewer).

Comment 8

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

Ok - I'll get a quick refresh of the r= when I update the patch then, probably from Jim Nance (original author).  Thanks bz

Comment 9

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

Attachment: hg export of updated patch

Ok, I think this is done now.  Final review?

Comment 10

[jim_nance]

I'm not sure I have the power to give you the review, but it looks good to me.

Thanks for working on this.

Comment 11

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

Comment on attachment 529180 [details] [diff] [review]
hg export of updated patch

Marking r=+ for Jim Nance

Comment 12

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

I'll commit this as soon as I have commit access again (and the tree is open, etc).

Comment 13

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

Checked in as changeset 69590:9968ed6b629a

Comment 14

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

FWIW, changeset links like this:

http://hg.mozilla.org/mozilla-central/rev/9968ed6b629a

are encouraged.