769989 - Store time information in each sample

Status: RESOLVED FIXED

(Core :: Gecko Profiler, defect)

Description

[Benoit Girard (:BenWa)]

Attachment: 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)]

Cleopatra changes:
https://github.com/bgirard/cleopatra/commit/dcf4a646472ea049aacde74acf8ad8871c3e2970

Comment 2

[Benoit Girard (:BenWa)]

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

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 4

[Benoit Girard (:BenWa)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9ad624cd2791

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/9ad624cd2791