Last Comment Bug 726374 - [CC] clean up COLLECT_TIME_DEBUG
Product: Core
Classification: Components
Component: XPCOM (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla13
Assigned To: Andrew McCreight [:mccr8]
: Nathan Froyd [:froydnj]
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:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

clean up debug timer measurement (11.66 KB, patch)
2012-02-11 19:15 PST, Andrew McCreight [:mccr8]
bugs: review+
Details | Diff | Splinter Review
clean up debug timer measurement (11.66 KB, patch)
2012-02-12 08:05 PST, Andrew McCreight [:mccr8]
continuation: review+
Details | Diff | Splinter 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.
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
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

>+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.
Comment 6 Marco Bonardo [::mak] 2012-02-13 08:49:07 PST

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