Bug 1344118 - Fix the profiler's sleeping threads optimization. r=jseward.
authorNicholas Nethercote <nnethercote@mozilla.com>
Fri, 03 Mar 2017 15:32:11 +1100
changeset 493416 4e196d802c7be7f3a3c147cc6c5b6406656584b1
parent 493415 ed21480d66e66fb04b09cd0eeba991e179fc7928
child 493417 bcb46d32449b7f9b53af8bc9512ec373c557c23a
push id47755
push userbmo:dkeeler@mozilla.com
push dateFri, 03 Mar 2017 22:32:10 +0000
reviewersjseward
bugs1344118
milestone54.0a1
Bug 1344118 - Fix the profiler's sleeping threads optimization. r=jseward. When ProfilerBuffer::reset() is called, DuplicateLastSample() will start failing for all sleeping threads because there will be no prior thread data in the buffer to duplicate. But the sampling loop doesn't detect such failure. This causes two problems: - Missing samples. - CPU usage goes through the roof, because each time around the sampling loop the length of the failing search increases. The fix is simple: detect failure in the sampling loop and do a normal sample in that case. The patch also removes ThreadInfo::DuplicateLastSample(), because it just calls onto ProfileBuffer::DuplicateLastSample().
tools/profiler/core/ProfileBuffer.h
tools/profiler/core/ProfileBufferEntry.cpp
tools/profiler/core/ThreadInfo.cpp
tools/profiler/core/ThreadInfo.h
tools/profiler/core/platform-linux-android.cpp
tools/profiler/core/platform-macos.cpp
tools/profiler/core/platform-win32.cpp
--- a/tools/profiler/core/ProfileBuffer.h
+++ b/tools/profiler/core/ProfileBuffer.h
@@ -19,17 +19,17 @@ public:
 
   ~ProfileBuffer();
 
   void addTag(const ProfileBufferEntry& aTag);
   void StreamSamplesToJSON(SpliceableJSONWriter& aWriter, int aThreadId, double aSinceTime,
                            JSContext* cx, UniqueStacks& aUniqueStacks);
   void StreamMarkersToJSON(SpliceableJSONWriter& aWriter, int aThreadId, double aSinceTime,
                            UniqueStacks& aUniqueStacks);
-  void DuplicateLastSample(int aThreadId, const mozilla::TimeStamp& aStartTime);
+  bool DuplicateLastSample(int aThreadId, const mozilla::TimeStamp& aStartTime);
 
   void addStoredMarker(ProfilerMarker* aStoredMarker);
 
   // The following two methods are not signal safe! They delete markers.
   void deleteExpiredStoredMarkers();
   void reset();
 
   size_t SizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf) const;
--- a/tools/profiler/core/ProfileBufferEntry.cpp
+++ b/tools/profiler/core/ProfileBufferEntry.cpp
@@ -746,50 +746,52 @@ int ProfileBuffer::FindLastSampleOfThrea
            readPos !=  (mReadPos + mEntrySize - 1) % mEntrySize;
            readPos  =   (readPos + mEntrySize - 1) % mEntrySize) {
     ProfileBufferEntry entry = mEntries[readPos];
     if (entry.isThreadId() && entry.mTagInt == aThreadId) {
       return readPos;
     }
   }
 
+  // This is rare. It typically happens after ProfileBuffer::reset() occurs.
   return -1;
 }
 
-void
+bool
 ProfileBuffer::DuplicateLastSample(int aThreadId, const TimeStamp& aStartTime)
 {
   int lastSampleStartPos = FindLastSampleOfThread(aThreadId);
   if (lastSampleStartPos == -1) {
-    return;
+    return false;
   }
 
   MOZ_ASSERT(mEntries[lastSampleStartPos].isThreadId());
 
   addTag(mEntries[lastSampleStartPos]);
 
   // Go through the whole entry and duplicate it, until we find the next one.
   for (int readPos = (lastSampleStartPos + 1) % mEntrySize;
        readPos != mWritePos;
        readPos = (readPos + 1) % mEntrySize) {
     switch (mEntries[readPos].kind()) {
       case ProfileBufferEntry::Kind::ThreadId:
         // We're done.
-        return;
+        return true;
       case ProfileBufferEntry::Kind::Time:
         // Copy with new time
         addTag(ProfileBufferEntry::Time((TimeStamp::Now() -
                                          aStartTime).ToMilliseconds()));
         break;
       case ProfileBufferEntry::Kind::Marker:
         // Don't copy markers
         break;
       default:
         // Copy anything else we don't know about
         addTag(mEntries[readPos]);
         break;
     }
   }
+  return true;
 }
 
 // END ProfileBuffer
 ////////////////////////////////////////////////////////////////////////
 
--- a/tools/profiler/core/ThreadInfo.cpp
+++ b/tools/profiler/core/ThreadInfo.cpp
@@ -231,23 +231,16 @@ ThreadInfo::FlushSamplesAndMarkers(Profi
 }
 
 mozilla::Mutex&
 ThreadInfo::GetMutex()
 {
   return *mMutex.get();
 }
 
-void
-ThreadInfo::DuplicateLastSample(ProfileBuffer* aBuffer,
-                                const TimeStamp& aStartTime)
-{
-  aBuffer->DuplicateLastSample(mThreadId, aStartTime);
-}
-
 size_t
 ThreadInfo::SizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf) const
 {
   size_t n = aMallocSizeOf(this);
 
   n += aMallocSizeOf(mName.get());
   n += mPseudoStack->SizeOfIncludingThis(aMallocSizeOf);
 
--- a/tools/profiler/core/ThreadInfo.h
+++ b/tools/profiler/core/ThreadInfo.h
@@ -60,19 +60,16 @@ public:
   mozilla::Mutex& GetMutex();
   void StreamJSON(ProfileBuffer* aBuffer, SpliceableJSONWriter& aWriter,
                   double aSinceTime = 0);
 
   // Call this method when the JS entries inside the buffer are about to
   // become invalid, i.e., just before JS shutdown.
   void FlushSamplesAndMarkers(ProfileBuffer* aBuffer);
 
-  void DuplicateLastSample(ProfileBuffer* aBuffer,
-                           const mozilla::TimeStamp& aStartTime);
-
   ThreadResponsiveness* GetThreadResponsiveness() { return &mRespInfo; }
 
   void UpdateThreadResponsiveness() {
     mRespInfo.Update(mIsMainThread, mThread);
   }
 
   void StreamSamplesAndMarkers(ProfileBuffer* aBuffer,
                                SpliceableJSONWriter& aWriter,
--- a/tools/profiler/core/platform-linux-android.cpp
+++ b/tools/profiler/core/platform-linux-android.cpp
@@ -361,18 +361,18 @@ SigprofSender(void* aArg)
       for (uint32_t i = 0; i < gRegisteredThreads->size(); i++) {
         ThreadInfo* info = (*gRegisteredThreads)[i];
 
         // This will be null if we're not interested in profiling this thread.
         if (!info->HasProfile() || info->IsPendingDelete()) {
           continue;
         }
 
-        if (info->Stack()->CanDuplicateLastSampleDueToSleep()) {
-          info->DuplicateLastSample(gBuffer, gStartTime);
+        if (info->Stack()->CanDuplicateLastSampleDueToSleep() &&
+            gBuffer->DuplicateLastSample(info->ThreadId(), gStartTime)) {
           continue;
         }
 
         info->UpdateThreadResponsiveness();
 
         int threadId = info->ThreadId();
         MOZ_ASSERT(threadId != my_tid);
 
--- a/tools/profiler/core/platform-macos.cpp
+++ b/tools/profiler/core/platform-macos.cpp
@@ -158,18 +158,18 @@ public:
         for (uint32_t i = 0; i < gRegisteredThreads->size(); i++) {
           ThreadInfo* info = (*gRegisteredThreads)[i];
 
           // This will be null if we're not interested in profiling this thread.
           if (!info->HasProfile() || info->IsPendingDelete()) {
             continue;
           }
 
-          if (info->Stack()->CanDuplicateLastSampleDueToSleep()) {
-            info->DuplicateLastSample(gBuffer, gStartTime);
+          if (info->Stack()->CanDuplicateLastSampleDueToSleep() &&
+              gBuffer->DuplicateLastSample(info->ThreadId(), gStartTime)) {
             continue;
           }
 
           info->UpdateThreadResponsiveness();
 
           SampleContext(info, isFirstProfiledThread);
           isFirstProfiledThread = false;
         }
--- a/tools/profiler/core/platform-win32.cpp
+++ b/tools/profiler/core/platform-win32.cpp
@@ -167,18 +167,18 @@ class SamplerThread
         for (uint32_t i = 0; i < gRegisteredThreads->size(); i++) {
           ThreadInfo* info = (*gRegisteredThreads)[i];
 
           // This will be null if we're not interested in profiling this thread.
           if (!info->HasProfile() || info->IsPendingDelete()) {
             continue;
           }
 
-          if (info->Stack()->CanDuplicateLastSampleDueToSleep()) {
-            info->DuplicateLastSample(gBuffer, gStartTime);
+          if (info->Stack()->CanDuplicateLastSampleDueToSleep() &&
+              gBuffer->DuplicateLastSample(info->ThreadId(), gStartTime)) {
             continue;
           }
 
           info->UpdateThreadResponsiveness();
 
           SampleContext(info, isFirstProfiledThread);
           isFirstProfiledThread = false;
         }