Bug 719176. Add the ability to maintain a temporary buffer of samples. r=ehsan
authorJeff Muizelaar <jmuizelaar@mozilla.com>
Wed, 18 Jan 2012 18:07:46 -0500
changeset 85028 40130b602934
parent 85027 2059cef905c4
child 85029 26678d3cbf83
push id21888
push userbmo@edmorley.co.uk
push date2012-01-21 02:32 +0000
treeherdermozilla-central@099ec081e8aa [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersehsan
bugs719176
milestone12.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 719176. Add the ability to maintain a temporary buffer of samples. r=ehsan This lets us make the decision about whether to discard samples later than when we record them which is important for about:jank.
tools/profiler/sps/TableTicker.cpp
--- a/tools/profiler/sps/TableTicker.cpp
+++ b/tools/profiler/sps/TableTicker.cpp
@@ -153,16 +153,17 @@ private:
 };
 
 #define PROFILE_MAX_ENTRY 100000
 class Profile
 {
 public:
   Profile(int aEntrySize)
     : mWritePos(0)
+    , mLastFlushPos(0)
     , mReadPos(0)
     , mEntrySize(aEntrySize)
   {
     mEntries = new ProfileEntry[mEntrySize];
     mNeedsSharedLibraryInfo = true;
   }
 
   ~Profile()
@@ -175,60 +176,129 @@ public:
     // Called from signal, call only reentrant functions
     mEntries[mWritePos] = aTag;
     mWritePos = (mWritePos + 1) % mEntrySize;
     if (mWritePos == mReadPos) {
       // Keep one slot open
       mEntries[mReadPos] = ProfileEntry();
       mReadPos = (mReadPos + 1) % mEntrySize;
     }
+    // we also need to move the flush pos to ensure we
+    // do not pass it
+    if (mWritePos == mLastFlushPos) {
+      mLastFlushPos = (mLastFlushPos + 1) % mEntrySize;
+    }
+  }
+
+  // flush the new entries
+  void flush()
+  {
+    mLastFlushPos = mWritePos;
+  }
+
+  // discards all of the entries since the last flush()
+  // NOTE: that if mWritePos happens to wrap around past
+  // mLastFlushPos we actually only discard mWritePos - mLastFlushPos entries
+  //
+  // r = mReadPos
+  // w = mWritePos
+  // f = mLastFlushPos
+  //
+  //     r          f    w
+  // |-----------------------------|
+  // |   abcdefghijklmnopq         | -> 'abcdefghijklmnopq'
+  // |-----------------------------|
+  //
+  //
+  // mWritePos and mReadPos have passed mLastFlushPos
+  //                      f
+  //                    w r
+  // |-----------------------------|
+  // |ABCDEFGHIJKLMNOPQRSqrstuvwxyz|
+  // |-----------------------------|
+  //                       w
+  //                       r
+  // |-----------------------------|
+  // |ABCDEFGHIJKLMNOPQRSqrstuvwxyz| -> ''
+  // |-----------------------------|
+  //
+  //
+  // mWritePos will end up the same as mReadPos
+  //                r
+  //              w f
+  // |-----------------------------|
+  // |ABCDEFGHIJKLMklmnopqrstuvwxyz|
+  // |-----------------------------|
+  //                r
+  //                w
+  // |-----------------------------|
+  // |ABCDEFGHIJKLMklmnopqrstuvwxyz| -> ''
+  // |-----------------------------|
+  //
+  //
+  // mWritePos has moved past mReadPos
+  //      w r       f
+  // |-----------------------------|
+  // |ABCDEFdefghijklmnopqrstuvwxyz|
+  // |-----------------------------|
+  //        r       w
+  // |-----------------------------|
+  // |ABCDEFdefghijklmnopqrstuvwxyz| -> 'defghijkl'
+  // |-----------------------------|
+
+  void erase()
+  {
+    mWritePos = mLastFlushPos;
   }
 
   void ToString(StringBuilder &profile)
   {
     if (mNeedsSharedLibraryInfo) {
       // Can't be called from signal because
       // getting the shared library information can call non-reentrant functions.
       mSharedLibraryInfo = SharedLibraryInfo::GetInfoForSelf();
     }
 
+    //XXX: this code is not thread safe and needs to be fixed
     int oldReadPos = mReadPos;
-    while (mReadPos != mWritePos) {
+    while (mReadPos != mLastFlushPos) {
       profile.Append(mEntries[mReadPos].TagToString(this).c_str());
       mReadPos = (mReadPos + 1) % mEntrySize;
     }
     mReadPos = oldReadPos;
   }
 
   void WriteProfile(FILE* stream)
   {
     if (mNeedsSharedLibraryInfo) {
       // Can't be called from signal because
       // getting the shared library information can call non-reentrant functions.
       mSharedLibraryInfo = SharedLibraryInfo::GetInfoForSelf();
     }
 
+    //XXX: this code is not thread safe and needs to be fixed
     int oldReadPos = mReadPos;
-    while (mReadPos != mWritePos) {
+    while (mReadPos != mLastFlushPos) {
       string tag = mEntries[mReadPos].TagToString(this);
       fwrite(tag.data(), 1, tag.length(), stream);
       mReadPos = (mReadPos + 1) % mEntrySize;
     }
     mReadPos = oldReadPos;
   }
 
   SharedLibraryInfo& getSharedLibraryInfo()
   {
     return mSharedLibraryInfo;
   }
 private:
   // Circular buffer 'Keep One Slot Open' implementation
   // for simplicity
   ProfileEntry *mEntries;
   int mWritePos; // points to the next entry we will write to
+  int mLastFlushPos; // points to the next entry since the last flush()
   int mReadPos;  // points to the next entry we will read to
   int mEntrySize;
   bool mNeedsSharedLibraryInfo;
   SharedLibraryInfo mSharedLibraryInfo;
 };
 
 class SaveProfileTask;
 
@@ -247,16 +317,17 @@ class TableTicker: public Sampler {
               const char** aFeatures, uint32_t aFeatureCount)
     : Sampler(aInterval, true)
     , mProfile(aEntrySize)
     , mStack(aStack)
     , mSaveRequested(false)
   {
     mUseStackWalk = hasFeature(aFeatures, aFeatureCount, "stackwalk");
     mProfile.addTag(ProfileEntry('m', "Start"));
+    //XXX: It's probably worth splitting the jank profiler out from the regular profiler at some point
     mJankOnly = hasFeature(aFeatures, aFeatureCount, "jank");
   }
 
   ~TableTicker() { if (IsActive()) Stop(); }
 
   virtual void SampleStack(TickSample* sample) {}
 
   // Called within a signal. This function must be reentrant
@@ -416,50 +487,73 @@ void doSampleStackTrace(Stack *aStack, P
       }
       aProfile.addTag(ProfileEntry('s', aStack->mStack[i], pc));
     } else {
       aProfile.addTag(ProfileEntry('c', aStack->mStack[i]));
     }
   }
 }
 
+/* used to keep track of the last event that we sampled during */
+unsigned int sLastSampledEventGeneration = 0;
+
+/* a counter that's incremented everytime we get responsiveness event
+ * note: it might also be worth tracking everytime we go around
+ * the event loop */
+unsigned int sCurrentEventGeneration = 0;
+/* we don't need to worry about overflow because we only treat the
+ * case of them being the same as special. i.e. we only run into
+ * a problem if 2^32 events happen between samples that we need
+ * to know are associated with different events */
+
 void TableTicker::Tick(TickSample* sample)
 {
   // Marker(s) come before the sample
   int i = 0;
   const char *marker = mStack->getMarker(i++);
   for (int i = 0; marker != NULL; i++) {
     mProfile.addTag(ProfileEntry('m', marker));
     marker = mStack->getMarker(i++);
   }
   mStack->mQueueClearMarker = true;
 
+  // if we are on a different event we can discard any temporary samples
+  // we've kept around
+  if (sLastSampledEventGeneration != sCurrentEventGeneration) {
+    // XXX: we also probably want to add an entry to the profile to help
+    // distinguish which samples are part of the same event. That, or record
+    // the event generation in each sample
+    mProfile.erase();
+  }
+  sLastSampledEventGeneration = sCurrentEventGeneration;
+
   bool recordSample = true;
   if (mJankOnly) {
     recordSample = false;
     // only record the events when we have a we haven't seen a tracer event for 100ms
     if (!sLastTracerEvent.IsNull()) {
       TimeDuration delta = sample->timestamp - sLastTracerEvent;
       if (delta.ToMilliseconds() > 100.0) {
           recordSample = true;
       }
     }
   }
 
-  if (recordSample) {
 #if defined(USE_BACKTRACE) || defined(USE_NS_STACKWALK)
-    if (mUseStackWalk) {
-      doBacktrace(mProfile);
-    } else {
-      doSampleStackTrace(mStack, mProfile, sample);
-    }
+  if (mUseStackWalk) {
+    doBacktrace(mProfile);
+  } else {
+    doSampleStackTrace(mStack, mProfile, sample);
+  }
 #else
-    doSampleStackTrace(mStack, mProfile, sample);
+  doSampleStackTrace(mStack, mProfile, sample);
 #endif
-  }
+
+  if (recordSample)
+    mProfile.flush();
 
   if (!mJankOnly && !sLastTracerEvent.IsNull() && sample) {
     TimeDuration delta = sample->timestamp - sLastTracerEvent;
     mProfile.addTag(ProfileEntry('r', delta.ToMilliseconds()));
   }
 }
 
 string ProfileEntry::TagToString(Profile *profile)
@@ -654,16 +748,17 @@ void mozilla_sampler_responsiveness(Time
       //for(size_t i = 0; i < 100; i++) {
       //  sResponsivenessTimes[i] = 0;
       //}
       //sResponsivenessLoc = 0;
     }
     TimeDuration delta = aTime - sLastTracerEvent;
     sResponsivenessTimes[sResponsivenessLoc++] = delta.ToMilliseconds();
   }
+  sCurrentEventGeneration++;
 
   sLastTracerEvent = aTime;
 }
 
 const double* mozilla_sampler_get_responsiveness()
 {
   return sResponsivenessTimes;
 }