Last Comment Bug 707800 - Add more sampler labels
: Add more sampler labels
Status: RESOLVED FIXED
[Snappy:P1]
:
Product: Core
Classification: Components
Component: General (show other bugs)
: unspecified
: x86 Mac OS X
: -- normal (vote)
: mozilla11
Assigned To: Jeff Muizelaar [:jrmuizel]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-12-05 12:54 PST by Jeff Muizelaar [:jrmuizel]
Modified: 2012-02-01 13:57 PST (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Add a bunch of labels that should help (7.87 KB, patch)
2011-12-05 12:54 PST, Jeff Muizelaar [:jrmuizel]
bgirard: review+
Details | Diff | Splinter Review
Add a bunch of labels that should help v2 (10.82 KB, patch)
2011-12-07 11:54 PST, Jeff Muizelaar [:jrmuizel]
bgirard: review+
Details | Diff | Splinter Review
Rename SAMPLE_CHECKPOINT to SAMPLE_LABEL (1.87 KB, patch)
2011-12-07 12:33 PST, Jeff Muizelaar [:jrmuizel]
bgirard: review+
Details | Diff | Splinter Review

Description Jeff Muizelaar [:jrmuizel] 2011-12-05 12:54:25 PST
Created attachment 579140 [details] [diff] [review]
Add a bunch of labels that should help
Comment 1 Jeff Muizelaar [:jrmuizel] 2011-12-07 11:54:59 PST
Created attachment 579773 [details] [diff] [review]
Add a bunch of labels that should help v2
Comment 2 Jeff Muizelaar [:jrmuizel] 2011-12-07 12:33:16 PST
Created attachment 579791 [details] [diff] [review]
Rename SAMPLE_CHECKPOINT to SAMPLE_LABEL
Comment 3 Benoit Girard (:BenWa) 2011-12-07 12:34:21 PST
Comment on attachment 579791 [details] [diff] [review]
Rename SAMPLE_CHECKPOINT to SAMPLE_LABEL

You should fold these patch IMO
Comment 4 Benoit Girard (:BenWa) 2011-12-07 13:10:21 PST
I missed that you added:
'#define SAMPLER_RESPONSIVENESS(time)' to one of the patch. I don't think this should go in the tree.
Comment 5 Ed Morley [:emorley] 2011-12-09 07:01:46 PST
Please set assignee when attaching or else when landed :-)

https://hg.mozilla.org/mozilla-central/rev/3de0addfa544
https://hg.mozilla.org/mozilla-central/rev/c79de7ae8a57
Comment 6 Olli Pettay [:smaug] (vacation Aug 25-28) 2012-01-29 08:40:04 PST
Comment on attachment 579773 [details] [diff] [review]
Add a bunch of labels that should help v2


>@@ -3191,6 +3197,7 @@ nsJSContext::CycleCollectNow(nsICycleCollectorListener *aListener)
>     return;
>   }
> 
>+  SAMPLE_LABEL("GC", "CycleCollectNow");

I don't know what the first parameter means, but
why does CycleCollectNow have "GC"?
Cycle collection is cycle collection, not garbage collection.

>diff --git a/xpcom/threads/nsTimerImpl.cpp b/xpcom/threads/nsTimerImpl.cpp
>index 90d122f..3884b17 100644
>--- a/xpcom/threads/nsTimerImpl.cpp
>+++ b/xpcom/threads/nsTimerImpl.cpp
>@@ -44,6 +44,7 @@
> #include "nsThreadManager.h"
> #include "nsThreadUtils.h"
> #include "prmem.h"
>+#include "sampler.h"
> 
> using mozilla::TimeDuration;
> using mozilla::TimeStamp;
>@@ -376,6 +377,8 @@ void nsTimerImpl::Fire()
>   if (mCanceled)
>     return;
> 
>+  SAMPLE_LABEL("Timer", "Fire");
>+
Did any xpcom peer actually review this? Is it ok that xpcom/ depends always on tools/? I would have assumed some #ifdef was needed.

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