Last Comment Bug 745034 - Add page fault counts to GC statistics
: Add page fault counts to GC statistics
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla15
Assigned To: Bill McCloskey (:billm)
:
Mentors:
Depends on: 759459
Blocks:
  Show dependency treegraph
 
Reported: 2012-04-12 17:09 PDT by Bill McCloskey (:billm)
Modified: 2012-05-29 12:23 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (12.70 KB, patch)
2012-04-12 17:09 PDT, Bill McCloskey (:billm)
terrence: review+
Details | Diff | Review
patch to make windows builds work (2.02 KB, patch)
2012-05-07 10:21 PDT, Bill McCloskey (:billm)
dmandelin: review+
Details | Diff | Review

Description Bill McCloskey (:billm) 2012-04-12 17:09:52 PDT
Created attachment 614625 [details] [diff] [review]
patch

This counts how many page faults happened during a GC and puts it in the JSON data. It counts both hard and soft faults, since Windows is apparently unable to separate them. Hopefully that will still be valuable though.

I also changed the formatting of text and JSON a little. For the text output, it changes this:
  Slice: 3, Time: 1300ms (Pause: 18ms, Reason: ...)
to this:
  Slice: 3, Pause: 18ms (When: 1300ms, Reason: ...)
This moves the number people care about to the front and is less confusing.

I also noticed that beginArray and beginObject use putQuoted and I think they should use putKey. The consequence of this is that the slices array in JSON was called "Slices" rather than "slices". This patch fixes the problem.
Comment 1 Terrence Cole [:terrence] 2012-04-12 18:16:29 PDT
Comment on attachment 614625 [details] [diff] [review]
patch

Review of attachment 614625 [details] [diff] [review]:
-----------------------------------------------------------------

I love it.
Comment 2 Bill McCloskey (:billm) 2012-05-07 10:21:15 PDT
Created attachment 621648 [details] [diff] [review]
patch to make windows builds work

Apparently we need this psapi.lib thing in order to use GetProcessMemoryInfo, which tells us the number of page faults.
Comment 4 Ed Morley [:emorley] 2012-05-08 03:24:01 PDT
https://hg.mozilla.org/mozilla-central/rev/77a87e7c6f50
Comment 5 Justin Lebar (not reading bugmail) 2012-05-08 07:17:45 PDT
FWIW, we have code for this on *nix in xpcom/base/nsMemoryReporterManager.cpp.  We can separate hard and soft faults there, which is nice.
Comment 6 Bill McCloskey (:billm) 2012-05-08 11:07:36 PDT
(In reply to Justin Lebar [:jlebar] from comment #5)
> FWIW, we have code for this on *nix in
> xpcom/base/nsMemoryReporterManager.cpp.  We can separate hard and soft
> faults there, which is nice.

I couldn't figure out how to do this on Windows, so I just decided to combine them. Maybe later if it makes sense we can split them out on Mac and Linux.
Comment 7 Justin Lebar (not reading bugmail) 2012-05-08 13:04:01 PDT
> I couldn't figure out how to do this on Windows, so I just decided to combine them.

Yes, I don't think it's possible to split them out on Windows.  And I don't know how useful soft page faults are for detecting memory pressure.
Comment 8 Bill McCloskey (:billm) 2012-05-08 13:06:38 PDT
This bug isn't about detecting memory pressure. It's for bug triage. If someone has really long GC times, I want to be able to see if it's because they're paging a lot.

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