Last Comment Bug 726374 - [CC] clean up COLLECT_TIME_DEBUG
: [CC] clean up COLLECT_TIME_DEBUG
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: XPCOM (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla13
Assigned To: Andrew McCreight [:mccr8]
:
Mentors:
Depends on:
Blocks: 697134
  Show dependency treegraph
 
Reported: 2012-02-11 18:11 PST by Andrew McCreight [:mccr8]
Modified: 2012-02-13 08:49 PST (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
clean up debug timer measurement (11.66 KB, patch)
2012-02-11 19:15 PST, Andrew McCreight [:mccr8]
bugs: review+
Details | Diff | Review
clean up debug timer measurement (11.66 KB, patch)
2012-02-12 08:05 PST, Andrew McCreight [:mccr8]
continuation: review+
Details | Diff | Review

Description Andrew McCreight [:mccr8] 2012-02-11 18:11:36 PST
All of the macro stuff all over the place is a little annoying, and there are a few segments that it is missing coverage for.  I also switched to TimeStamp and cleaned up a few misc. timer things.
Comment 1 Andrew McCreight [:mccr8] 2012-02-11 18:25:56 PST
example output:

cc: ForgetSkippable() took 11ms
cc: mRuntimes[*]->BeginCycleCollection() took 2ms
cc: MarkRoots() took 85ms
cc: ScanRoots() took 14ms
cc:      CollectWhite::Root took 1ms
cc:      CollectWhite::Unlink took 8ms
cc:      CollectWhite::Unroot took 19ms
cc: CollectWhite() took 30ms
cc: ClearGraph() took 4ms
cc: total cycle collector time was 140ms
cc: visited 9849 ref counted and 41542 GCed objects, freed 47120.
cc:
Comment 2 Andrew McCreight [:mccr8] 2012-02-11 19:15:27 PST
Created attachment 596413 [details] [diff] [review]
clean up debug timer measurement
Comment 3 Andrew McCreight [:mccr8] 2012-02-11 19:18:12 PST
https://tbpl.mozilla.org/?tree=Try&rev=79a06f6ddea2
Comment 4 Olli Pettay [:smaug] 2012-02-12 05:27:45 PST
Comment on attachment 596413 [details] [diff] [review]
clean up debug timer measurement

> #endif
> 
>+
Nit, extra newline

>+#ifdef COLLECT_TIME_DEBUG
>+struct TimeLog
I'd prefer class and then public:


>+{
>+    TimeLog() : mLastCheckpoint(TimeStamp::Now()) {}
>+
>+    void
>+    Checkpoint(const char* aEvent)
>+    {
>+        TimeStamp now = TimeStamp::Now();
>+        PRInt32 dur = (PRInt32) (now - mLastCheckpoint).ToMilliseconds();
Please use C++ casting, or PRInt32(expr).
And would PRUint32 be better. That is used elsewhere.

>+    timeLog.Checkpoint("     CollectWhite::Root");
I'd prefer without spaces, since with indentation it looks like
the time is part of something above.
Comment 5 Andrew McCreight [:mccr8] 2012-02-12 08:05:15 PST
Created attachment 596481 [details] [diff] [review]
clean up debug timer measurement

Addressed review comments.

https://hg.mozilla.org/integration/mozilla-inbound/rev/ad782d99ae8f
Comment 6 Marco Bonardo [::mak] 2012-02-13 08:49:07 PST
https://hg.mozilla.org/mozilla-central/rev/ad782d99ae8f

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