Last Comment Bug 674384 - Jprof improvements
: Jprof improvements
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: General (show other bugs)
: Trunk
: x86 Linux
: -- normal (vote)
: ---
Assigned To: [:jesup] on pto until 2016/8/1 Randell Jesup
:
Mentors:
Depends on:
Blocks: 1274041
  Show dependency treegraph
 
Reported: 2011-07-26 15:06 PDT by [:jesup] on pto until 2016/8/1 Randell Jesup
Modified: 2016-05-18 13:54 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-


Attachments
Jprof patch (16.28 KB, patch)
2011-07-26 15:06 PDT, [:jesup] on pto until 2016/8/1 Randell Jesup
dbaron: review+
Details | Diff | Splinter Review
The rest of the work to support multi-process jprof (for e10s) (20.60 KB, patch)
2011-07-27 20:57 PDT, [:jesup] on pto until 2016/8/1 Randell Jesup
dbaron: review+
Details | Diff | Splinter Review

Description [:jesup] on pto until 2016/8/1 Randell Jesup 2011-07-26 15:06:59 PDT
Created attachment 548605 [details] [diff] [review]
Jprof patch

Jprof's new thread option didn't clear the function hitcounts between threads.

Formatting of the jprof thread table had extra \n's, and the links were missing '#'.

Add --only-thread N to only output the data for thread N.

Add options to select which 'section' of the trace (PROF to USR1) you want to profile.  options:  --start N --end M or --last.

Thread profiling will be useful for E10s.  Per-section profiling lets you profile several actions per run, or try multiple times to catch something, etc.
It may allow near-real-time profiling without exiting the browser as well since we flush the address map and profile data on pause.


Debug-only code, enabled with --enable-jprof -- nominating for Aurora
Comment 1 [:jesup] on pto until 2016/8/1 Randell Jesup 2011-07-27 20:57:19 PDT
Created attachment 549012 [details] [diff] [review]
The rest of the work to support multi-process jprof (for e10s)

This adds the support for generating jprof files separately for each process in an e10s mozilla build.  It also adds support for modifying the filename for the log with JP_FILENAME and specifying an output directory (since it can now process multiple profile logs at once for e10s use).  The README.html is also updated.

This patch assumes the earlier one is applied.
Comment 2 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2011-08-04 16:07:08 PDT
Could you explain what thread support in a timer-based profiler means?  At each timer tick, do we allocate the state at that time to one thread, to some of them (depending on state), or to all of them?
Comment 3 [:jesup] on pto until 2016/8/1 Randell Jesup 2011-08-05 10:04:27 PDT
Yes, if you process the results with --threads (or -t), each snapshot is allocated to the current thread running when it occurred (the log file includes the thread number for each sample now).  Results are generated for each thread, and at the head of the jprof file there's a listing of threads with anchor links to each one's individual profile.  In the last release I had an early version of the thread support, but there were a few rough edges in the output.  You can process the logfile without --threads and get the traditional all-in-one jprof view.

E10s requires processing multiple logs (one per process) with one map file - see the README.html file for info.  E10s is preliminary since I'm not normally running it; I pointed one of the people working on E10s at the patches to see if they want to request any improvements.
Comment 4 christian 2011-08-09 15:03:40 PDT
Why was this nominated for tracking for Firefox 7?
Comment 5 [:jesup] on pto until 2016/8/1 Randell Jesup 2011-08-09 20:44:03 PDT
Not sure, but it may have been similar to why the previous set of jprof fixes went into FF 6 for bug 660968 - it's NPOTB -- it's debugging-support-only, so it doesn't risk the builds.  Now if dbaron would finish his review.... ;-)
Comment 6 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2011-08-11 12:16:47 PDT
Comment on attachment 548605 [details] [diff] [review]
Jprof patch

I should have decided to do this ages ago, but given my review backlog, and the failures of my attempts to find somebody else interested in reviewing jprof code, I'm just going to rubber stamp this r=dbaron.  Sorry for taking so long to decide to do that.
Comment 7 David Baron :dbaron: ⌚️UTC+2 (mostly busy through August 4; review requests must explain patch) 2011-08-11 12:17:01 PDT
Comment on attachment 549012 [details] [diff] [review]
The rest of the work to support multi-process jprof (for e10s)

I should have decided to do this ages ago, but given my review backlog, and the failures of my attempts to find somebody else interested in reviewing jprof code, I'm just going to rubber stamp this r=dbaron.  Sorry for taking so long to decide to do that.
Comment 8 [:jesup] on pto until 2016/8/1 Randell Jesup 2011-08-12 10:04:34 PDT
Checked in as http://hg.mozilla.org/mozilla-central/rev/997256d4e070

Note You need to log in before you can comment on or make changes to this bug.