726374 - [CC] clean up COLLECT_TIME_DEBUG

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Andrew McCreight [:mccr8]]

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]]

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]]

Attachment: clean up debug timer measurement

Comment 3

[Andrew McCreight [:mccr8]]

https://tbpl.mozilla.org/?tree=Try&rev=79a06f6ddea2

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

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]]

Attachment: clean up debug timer measurement

Addressed review comments.

https://hg.mozilla.org/integration/mozilla-inbound/rev/ad782d99ae8f

Comment 6

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/ad782d99ae8f