Closed Bug 653311 Opened 9 years ago Closed 9 years ago

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

Categories

(Core :: General, defect)

x86
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla6

People

(Reporter: jesup, Assigned: jesup)

References

(Blocks 1 open bug)

Details

(Keywords: perf)

Attachments

(1 file, 1 obsolete file)

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)
Assignee: nobody → rjesup
Status: NEW → ASSIGNED
Keywords: perf
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 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)
Attachment #528815 - Flags: review?(dbaron)
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
Attachment #528815 - Flags: review?(jim_nance)
Added bz; made bug 187053 blocked by this though technically it isn't.
Blocks: 187053
Attachment #528815 - Flags: review?(jim_nance) → review+
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
Attachment #528815 - Flags: review?(bzbarsky)
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.
I'm not really competent to review this (nor do I think it needs a second reviewer).
Ok - I'll get a quick refresh of the r= when I update the patch then, probably from Jim Nance (original author).  Thanks bz
Ok, I think this is done now.  Final review?
Attachment #528815 - Attachment is obsolete: true
Attachment #528815 - Flags: review?(dbaron)
Attachment #528815 - Flags: review?(bzbarsky)
Attachment #529180 - Flags: review?(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 on attachment 529180 [details] [diff] [review]
hg export of updated patch

Marking r=+ for Jim Nance
Attachment #529180 - Flags: review?(jim_nance) → review+
I'll commit this as soon as I have commit access again (and the tree is open, etc).
Checked in as changeset 69590:9968ed6b629a
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
FWIW, changeset links like this:

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

are encouraged.
Target Milestone: --- → mozilla6
You need to log in before you can comment on or make changes to this bug.