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

RESOLVED FIXED in mozilla6

Status

()

Core
General
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: jesup, Assigned: jesup)

Tracking

(Blocks: 1 bug, {perf})

Trunk
mozilla6
x86
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

6 years ago
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)

Updated

6 years ago
Assignee: nobody → rjesup
(Assignee)

Updated

6 years ago
Status: NEW → ASSIGNED
Keywords: perf
(Assignee)

Comment 1

6 years ago
Created attachment 528815 [details] [diff] [review]
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).
(Assignee)

Comment 2

6 years ago
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)
(Assignee)

Comment 3

6 years ago
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)
(Assignee)

Comment 4

6 years ago
Added bz; made bug 187053 blocked by this though technically it isn't.
Blocks: 187053

Updated

6 years ago
Attachment #528815 - Flags: review?(jim_nance) → review+
(Assignee)

Comment 5

6 years ago
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)
(Assignee)

Comment 6

6 years ago
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).
(Assignee)

Comment 8

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

Comment 9

6 years ago
Created attachment 529180 [details] [diff] [review]
hg export of updated patch

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)

Comment 10

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

Thanks for working on this.
(Assignee)

Comment 11

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

Marking r=+ for Jim Nance
Attachment #529180 - Flags: review?(jim_nance) → review+
(Assignee)

Comment 12

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

Comment 13

6 years ago
Checked in as changeset 69590:9968ed6b629a
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
FWIW, changeset links like this:

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

are encouraged.

Updated

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