Last Comment Bug 769989 - Store time information in each sample
: Store time information in each sample
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Gecko Profiler (show other bugs)
: unspecified
: x86 Mac OS X
: -- normal (vote)
: mozilla16
Assigned To: Benoit Girard (:BenWa)
:
Mentors:
Depends on: 771608
Blocks:
  Show dependency treegraph
 
Reported: 2012-06-30 22:58 PDT by Benoit Girard (:BenWa)
Modified: 2012-07-06 12:02 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (3.70 KB, patch)
2012-06-30 22:58 PDT, Benoit Girard (:BenWa)
jmuizelaar: review+
Details | Diff | Splinter Review

Description Benoit Girard (:BenWa) 2012-06-30 22:58:59 PDT
Created attachment 638172 [details] [diff] [review]
patch

Storing the time information for each sample will let us know put an accurate timeline and detect 'time breaks' for the profiler was not able to keep the sampling rate. It will also be interesting to have the UI use this data to tell you how accurate the timings are.
Comment 1 Benoit Girard (:BenWa) 2012-07-01 09:23:13 PDT
Cleopatra changes:
https://github.com/bgirard/cleopatra/commit/dcf4a646472ea049aacde74acf8ad8871c3e2970
Comment 2 Benoit Girard (:BenWa) 2012-07-01 09:28:33 PDT
I added 'Real Interval: 1.18 ms +-1.14' to the info bar. From a few quick profiles we always run longer then the target which is expected since we don't account for the time to record a sample. We're more precise then I expected with +- 1
Comment 3 Jeff Muizelaar [:jrmuizel] 2012-07-05 12:38:16 PDT
Comment on attachment 638172 [details] [diff] [review]
patch

>diff --git a/tools/profiler/TableTicker.cpp b/tools/profiler/TableTicker.cpp
>--- a/tools/profiler/TableTicker.cpp
>+++ b/tools/profiler/TableTicker.cpp
>@@ -300,16 +300,23 @@ public:
>           break;
>         case 'r':
>           {
>             if (sample) {
>               b.DefineProperty(sample, "responsiveness", entry.mTagFloat);
>             }
>           }
>           break;
>+        case 't':
>+          {
>+            if (sample) {
>+              b.DefineProperty(sample, "time", entry.mTagFloat);

How about calling this 'timestamp'

>+            }
>+          }
>+          break;
>         case 'c':
>         case 'l':
>           {
>             if (sample) {
>               JSObject *frame = b.CreateObject();
>               if (entry.mTagName == 'l') {
>                 // Bug 753041
>                 // We need a double cast here to tell GCC that we don't want to sign
>@@ -357,16 +364,17 @@ hasFeature(const char** aFeatures, uint3
> }
> 
> class TableTicker: public Sampler {
>  public:
>   TableTicker(int aInterval, int aEntrySize, ProfileStack *aStack,
>               const char** aFeatures, uint32_t aFeatureCount)
>     : Sampler(aInterval, true)
>     , mPrimaryThreadProfile(aEntrySize, aStack)
>+    , mStartTime(TimeStamp::Now())
>     , mSaveRequested(false)
>   {
>     mUseStackWalk = hasFeature(aFeatures, aFeatureCount, "stackwalk");
> 
>     //XXX: It's probably worth splitting the jank profiler out from the regular profiler at some point
>     mJankOnly = hasFeature(aFeatures, aFeatureCount, "jank");
>     mPrimaryThreadProfile.addTag(ProfileEntry('m', "Start"));
>   }
>@@ -396,16 +404,17 @@ class TableTicker: public Sampler {
> 
> private:
>   // Not implemented on platforms which do not support backtracing
>   void doBacktrace(ThreadProfile &aProfile, TickSample* aSample);
> 
> private:
>   // This represent the application's main thread (SAMPLER_INIT)
>   ThreadProfile mPrimaryThreadProfile;
>+  TimeStamp mStartTime;
>   bool mSaveRequested;
>   bool mUseStackWalk;
>   bool mJankOnly;
> };
> 
> std::string GetSharedLibraryInfoString();
> 
> /**
>@@ -761,16 +770,21 @@ void TableTicker::Tick(TickSample* sampl
> 
>   if (recordSample)
>     mPrimaryThreadProfile.flush();
> 
>   if (!mJankOnly && !sLastTracerEvent.IsNull() && sample) {
>     TimeDuration delta = sample->timestamp - sLastTracerEvent;
>     mPrimaryThreadProfile.addTag(ProfileEntry('r', delta.ToMilliseconds()));
>   }
>+
>+  if (sample) {
>+    TimeDuration delta = sample->timestamp - mStartTime;
>+    mPrimaryThreadProfile.addTag(ProfileEntry('t', delta.ToMilliseconds()));
>+  }
> }

I'm not sure we should record this in jank mode.
Comment 5 Ryan VanderMeulen [:RyanVM] 2012-07-05 17:29:45 PDT
https://hg.mozilla.org/mozilla-central/rev/9ad624cd2791

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