Bug 970494 - Markers should be time based and not sample based. r=bgirard
authorViktor Stanchev <vstanche@mozilla.com>
Tue, 25 Feb 2014 10:40:45 -0500
changeset 170946 93c87caa78559b44dc7c65552e2c126f0430db37
parent 170945 c9f4f70e46e17db15c78caed8621d8cc4774010b
child 170947 a6a0700bf77546590aab05f9bbedccd837de2bad
push id270
push userpvanderbeken@mozilla.com
push dateThu, 06 Mar 2014 09:24:21 +0000
reviewersbgirard
bugs970494
milestone30.0a1
Bug 970494 - Markers should be time based and not sample based. r=bgirard
tools/profiler/ProfileEntry.cpp
tools/profiler/PseudoStack.h
tools/profiler/platform.cpp
--- a/tools/profiler/ProfileEntry.cpp
+++ b/tools/profiler/ProfileEntry.cpp
@@ -335,19 +335,21 @@ void ThreadProfile::BuildJSObject(Builde
     b.DefineProperty(profile, "name", mName);
   }
 
   b.DefineProperty(profile, "tid", static_cast<int>(mThreadId));
 
   typename Builder::RootedArray samples(b.context(), b.CreateArray());
   b.DefineProperty(profile, "samples", samples);
 
+  typename Builder::RootedArray markers(b.context(), b.CreateArray());
+  b.DefineProperty(profile, "markers", markers);
+
   typename Builder::RootedObject sample(b.context());
   typename Builder::RootedArray frames(b.context());
-  typename Builder::RootedArray markers(b.context());
 
   int readPos = mReadPos;
   while (readPos != mLastFlushPos) {
     // Number of tag consumed
     int incBy = 1;
     ProfileEntry entry = mEntries[readPos];
 
     // Read ahead to the next tag, if it's a 'd' tag process it now
@@ -360,23 +362,17 @@ void ThreadProfile::BuildJSObject(Builde
 
     if (readAheadPos != mLastFlushPos && mEntries[readAheadPos].mTagName == 'd') {
       tagStringData = processDynamicTag(readPos, &incBy, tagBuff);
     }
 
     switch (entry.mTagName) {
       case 'm':
         {
-          if (sample) {
-            if (!markers) {
-              markers = b.CreateArray();
-              b.DefineProperty(sample, "marker", markers);
-            }
-            entry.getMarker()->BuildJSObject(b, markers);
-          }
+          entry.getMarker()->BuildJSObject(b, markers);
         }
         break;
       case 'r':
         {
           if (sample) {
             b.DefineProperty(sample, "responsiveness", entry.mTagFloat);
           }
         }
@@ -403,18 +399,16 @@ void ThreadProfile::BuildJSObject(Builde
         }
         break;
       case 's':
         sample = b.CreateObject();
         b.DefineProperty(sample, "name", tagStringData);
         frames = b.CreateArray();
         b.DefineProperty(sample, "frames", frames);
         b.ArrayPush(samples, sample);
-        // Created lazily
-        markers = nullptr;
         // Fall though to create a label for the 's' tag
       case 'c':
       case 'l':
         {
           if (sample) {
             typename Builder::RootedObject frame(b.context(), b.CreateObject());
             if (entry.mTagName == 'l') {
               // Bug 753041
--- a/tools/profiler/PseudoStack.h
+++ b/tools/profiler/PseudoStack.h
@@ -106,37 +106,41 @@ template<typename T>
 class ProfilerLinkedList;
 class JSAObjectBuilder;
 class JSCustomArray;
 class ThreadProfile;
 class ProfilerMarker {
   friend class ProfilerLinkedList<ProfilerMarker>;
 public:
   ProfilerMarker(const char* aMarkerName,
-         ProfilerMarkerPayload* aPayload = nullptr);
+         ProfilerMarkerPayload* aPayload = nullptr,
+         float aTime = 0);
 
   ~ProfilerMarker();
 
   const char* GetMarkerName() const {
     return mMarkerName;
   }
 
   template<typename Builder> void
   BuildJSObject(Builder& b, typename Builder::ArrayHandle markers) const;
 
   void SetGeneration(int aGenID);
 
   bool HasExpired(int aGenID) const {
     return mGenID + 2 <= aGenID;
   }
 
+  float GetTime();
+
 private:
   char* mMarkerName;
   ProfilerMarkerPayload* mPayload;
   ProfilerMarker* mNext;
+  float mTime;
   int mGenID;
 };
 
 // Foward declaration
 typedef struct _UnwinderThreadBuffer UnwinderThreadBuffer;
 
 /**
  * This struct is used to add a mNext field to UnwinderThreadBuffer objects for
@@ -315,19 +319,19 @@ public:
     mPendingUWTBuffers.addLinkedUWTBuffer(aBuff);
   }
 
   UWTBufferLinkedList* getLinkedUWTBuffers()
   {
     return mPendingUWTBuffers.getLinkedUWTBuffers();
   }
 
-  void addMarker(const char *aMarkerStr, ProfilerMarkerPayload *aPayload)
+  void addMarker(const char *aMarkerStr, ProfilerMarkerPayload *aPayload, float aTime)
   {
-    ProfilerMarker* marker = new ProfilerMarker(aMarkerStr, aPayload);
+    ProfilerMarker* marker = new ProfilerMarker(aMarkerStr, aPayload, aTime);
     mPendingMarkers.addMarker(marker);
   }
 
   void addStoredMarker(ProfilerMarker *aStoredMarker) {
     mPendingMarkers.addStoredMarker(aStoredMarker);
   }
 
   void updateGeneration(int aGenID) {
--- a/tools/profiler/platform.cpp
+++ b/tools/profiler/platform.cpp
@@ -106,44 +106,52 @@ ThreadInfo::~ThreadInfo() {
 
   if (mProfile)
     delete mProfile;
 
   Sampler::FreePlatformData(mPlatformData);
 }
 
 ProfilerMarker::ProfilerMarker(const char* aMarkerName,
-    ProfilerMarkerPayload* aPayload)
+    ProfilerMarkerPayload* aPayload,
+    float aTime)
   : mMarkerName(strdup(aMarkerName))
   , mPayload(aPayload)
+  , mTime(aTime)
 {
 }
 
 ProfilerMarker::~ProfilerMarker() {
   free(mMarkerName);
   delete mPayload;
 }
 
 void
 ProfilerMarker::SetGeneration(int aGenID) {
   mGenID = aGenID;
 }
 
+float
+ProfilerMarker::GetTime() {
+  return mTime;
+}
+
 template<typename Builder> void
 ProfilerMarker::BuildJSObject(Builder& b, typename Builder::ArrayHandle markers) const {
   typename Builder::RootedObject marker(b.context(), b.CreateObject());
   b.DefineProperty(marker, "name", GetMarkerName());
   // TODO: Store the callsite for this marker if available:
   // if have location data
   //   b.DefineProperty(marker, "location", ...);
   if (mPayload) {
     typename Builder::RootedObject markerData(b.context(),
                                               mPayload->PreparePayload(b));
     b.DefineProperty(marker, "data", markerData);
   }
+  b.DefineProperty(marker, "time", mTime);
   b.ArrayPush(markers, marker);
 }
 
 template void
 ProfilerMarker::BuildJSObject<JSCustomObjectBuilder>(JSCustomObjectBuilder& b,
                               JSCustomObjectBuilder::ArrayHandle markers) const;
 template void
 ProfilerMarker::BuildJSObject<JSObjectBuilder>(JSObjectBuilder& b,
@@ -891,15 +899,16 @@ void mozilla_sampler_add_marker(const ch
   if (profiler_in_privacy_mode()) {
     return;
   }
 
   PseudoStack *stack = tlsPseudoStack.get();
   if (!stack) {
     return;
   }
-  stack->addMarker(aMarker, payload.forget());
+  TimeDuration delta = TimeStamp::Now() - sStartTime;
+  stack->addMarker(aMarker, payload.forget(), static_cast<float>(delta.ToMilliseconds()));
 }
 
 // END externally visible functions
 ////////////////////////////////////////////////////////////////////////