The default bug view has changed. See this FAQ.

Store time information in each sample

RESOLVED FIXED in mozilla16

Status

()

Core
Gecko Profiler
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: BenWa, Assigned: BenWa)

Tracking

unspecified
mozilla16
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
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.
Attachment #638172 - Flags: review?(jmuizelaar)
(Assignee)

Comment 1

5 years ago
Cleopatra changes:
https://github.com/bgirard/cleopatra/commit/dcf4a646472ea049aacde74acf8ad8871c3e2970
(Assignee)

Comment 2

5 years ago
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 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.
Attachment #638172 - Flags: review?(jmuizelaar) → review+
(Assignee)

Comment 4

5 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/9ad624cd2791
Assignee: nobody → bgirard
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla16
https://hg.mozilla.org/mozilla-central/rev/9ad624cd2791
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Assignee)

Updated

5 years ago
Depends on: 771608
You need to log in before you can comment on or make changes to this bug.