Last Comment Bug 736643 - Add timestamp support to GC/CC JSON output
: Add timestamp support to GC/CC JSON output
Status: VERIFIED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla14
Assigned To: Bill McCloskey (:billm)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-03-16 16:18 PDT by Bill McCloskey (:billm)
Modified: 2012-03-22 15:16 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (10.38 KB, patch)
2012-03-16 16:18 PDT, Bill McCloskey (:billm)
terrence: review+
Details | Diff | Splinter Review

Description Bill McCloskey (:billm) 2012-03-16 16:18:19 PDT
Created attachment 606782 [details] [diff] [review]
patch

Pretty self-explanatory. I decided to have it pass in the time so that we can use PR_Now instead of PRMJ_Now. It looks like the implementations may be different, so I'd rather use the same one that the rest of the browser uses.
Comment 1 Terrence Cole [:terrence] 2012-03-16 16:34:03 PDT
Comment on attachment 606782 [details] [diff] [review]
patch

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

Looks fine.
Comment 3 Henrik Skupin (:whimboo) 2012-03-17 05:21:05 PDT
So to calculate the beginning of the GC we would have to do timestamp-duration? Why do we want the timestamp for JSON only? Wouldn't it be helpful to have it in the console message too?
Comment 4 Bill McCloskey (:billm) 2012-03-17 09:52:42 PDT
(In reply to Henrik Skupin (:whimboo) from comment #3)
> So to calculate the beginning of the GC we would have to do
> timestamp-duration?

Yes.

> Why do we want the timestamp for JSON only? Wouldn't it
> be helpful to have it in the console message too?

The console includes a kind of timestamp (the GC(T+x), where x is the time in seconds since the first GC). We used to include an actual timestamp, but it was just a random 16-digit number that wasn't useful to anyone. I guess we could put the time of day in there, but I don't think that would be helpful either. Usually the only thing we care about is how far apart the GCs are.
Comment 5 Phil Ringnalda (:philor) 2012-03-17 17:25:45 PDT
https://hg.mozilla.org/mozilla-central/rev/31e8a3b6f4c3
Comment 6 Henrik Skupin (:whimboo) 2012-03-22 15:16:53 PDT
Verified fixed with Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:14.0) Gecko/20120321 Firefox/14.0a1 ID:20120321031151

Just to note the timestamp is in microseconds so tools will have to convert it most likely into milliseconds first.

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