Bug 1148069 - Ensure synchronous sampling does not set JitcodeGlobalEntry's generation. (r=djvj)
authorShu-yu Guo <shu@rfrn.org>
Sat, 28 Mar 2015 01:21:04 -0700
changeset 265208 1715529498c80b2c12338949685487e573b4a51b
parent 265207 284d71badea3991f5090feaefebfb1ef2b0e31db
child 265209 0c030f97a04f4e34c138b878c4352423f5e920f9
push id4718
push userraliiev@mozilla.com
push dateMon, 11 May 2015 18:39:53 +0000
treeherdermozilla-beta@c20c4ef55f08 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdjvj
bugs1148069
milestone39.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 1148069 - Ensure synchronous sampling does not set JitcodeGlobalEntry's generation. (r=djvj)
tools/profiler/ProfileEntry.cpp
tools/profiler/ProfileEntry.h
tools/profiler/PseudoStack.h
tools/profiler/TableTicker.cpp
tools/profiler/platform.cpp
--- a/tools/profiler/ProfileEntry.cpp
+++ b/tools/profiler/ProfileEntry.cpp
@@ -107,25 +107,29 @@ ProfileBuffer::ProfileBuffer(int aEntryS
   , mReadPos(0)
   , mEntrySize(aEntrySize)
   , mGeneration(0)
 {
 }
 
 ProfileBuffer::~ProfileBuffer()
 {
-  mGeneration = INT_MAX;
-  deleteExpiredStoredMarkers();
+  while (mStoredMarkers.peek()) {
+    delete mStoredMarkers.popHead();
+  }
 }
 
 // Called from signal, call only reentrant functions
 void ProfileBuffer::addTag(const ProfileEntry& aTag)
 {
   mEntries[mWritePos++] = aTag;
   if (mWritePos == mEntrySize) {
+    // Wrapping around may result in things referenced in the buffer (e.g.,
+    // JIT code addresses and markers) being incorrectly collected.
+    MOZ_ASSERT(mGeneration != UINT32_MAX);
     mGeneration++;
     mWritePos = 0;
   }
   if (mWritePos == mReadPos) {
     // Keep one slot open.
     mEntries[mReadPos] = ProfileEntry();
     mReadPos = (mReadPos + 1) % mEntrySize;
   }
@@ -134,17 +138,17 @@ void ProfileBuffer::addTag(const Profile
 void ProfileBuffer::addStoredMarker(ProfilerMarker *aStoredMarker) {
   aStoredMarker->SetGeneration(mGeneration);
   mStoredMarkers.insert(aStoredMarker);
 }
 
 void ProfileBuffer::deleteExpiredStoredMarkers() {
   // Delete markers of samples that have been overwritten due to circular
   // buffer wraparound.
-  int generation = mGeneration;
+  uint32_t generation = mGeneration;
   while (mStoredMarkers.peek() &&
          mStoredMarkers.peek()->HasExpired(generation)) {
     delete mStoredMarkers.popHead();
   }
 }
 
 #define DYNAMIC_MAX_STRING 512
 
--- a/tools/profiler/ProfileEntry.h
+++ b/tools/profiler/ProfileEntry.h
@@ -126,17 +126,17 @@ public:
 
   // Points to the entry at which we can start reading.
   int mReadPos;
 
   // The number of entries in our buffer.
   int mEntrySize;
 
   // How many times mWritePos has wrapped around.
-  int mGeneration;
+  uint32_t mGeneration;
 
   // Markers that marker entries in the buffer might refer to.
   ProfilerMarkerLinkedList mStoredMarkers;
 };
 
 class ThreadProfile
 {
 public:
@@ -173,17 +173,16 @@ public:
   ThreadResponsiveness* GetThreadResponsiveness() { return &mRespInfo; }
   void SetPendingDelete()
   {
     mPseudoStack = nullptr;
     mPlatformData = nullptr;
   }
 
   uint32_t bufferGeneration() const {
-    MOZ_ASSERT(mBuffer->mGeneration >= 0);
     return mBuffer->mGeneration;
   }
 
 private:
   FRIEND_TEST(ThreadProfile, InsertOneTag);
   FRIEND_TEST(ThreadProfile, InsertOneTagWithTinyBuffer);
   FRIEND_TEST(ThreadProfile, InsertTagsNoWrap);
   FRIEND_TEST(ThreadProfile, InsertTagsWrap);
--- a/tools/profiler/PseudoStack.h
+++ b/tools/profiler/PseudoStack.h
@@ -88,30 +88,30 @@ public:
 
   const char* GetMarkerName() const {
     return mMarkerName;
   }
 
   void
   StreamJSObject(JSStreamWriter& b) const;
 
-  void SetGeneration(int aGenID);
+  void SetGeneration(uint32_t aGenID);
 
-  bool HasExpired(int aGenID) const {
+  bool HasExpired(uint32_t aGenID) const {
     return mGenID + 2 <= aGenID;
   }
 
   float GetTime();
 
 private:
   char* mMarkerName;
   ProfilerMarkerPayload* mPayload;
   ProfilerMarker* mNext;
   float mTime;
-  int mGenID;
+  uint32_t mGenID;
 };
 
 // Foward declaration
 typedef struct _UnwinderThreadBuffer UnwinderThreadBuffer;
 
 /**
  * This struct is used to add a mNext field to UnwinderThreadBuffer objects for
  * use with ProfilerLinkedList. It is done this way so that UnwinderThreadBuffer
--- a/tools/profiler/TableTicker.cpp
+++ b/tools/profiler/TableTicker.cpp
@@ -481,17 +481,26 @@ void mergeStacksIntoProfile(ThreadProfil
   PseudoStack* pseudoStack = aProfile.GetPseudoStack();
   volatile StackEntry *pseudoFrames = pseudoStack->mStack;
   uint32_t pseudoCount = pseudoStack->stackSize();
 
   // Make a copy of the JS stack into a JSFrame array. This is necessary since,
   // like the native stack, the JS stack is iterated youngest-to-oldest and we
   // need to iterate oldest-to-youngest when adding entries to aProfile.
 
-  uint32_t startBufferGen = aProfile.bufferGeneration();
+  // Synchronous sampling reports an invalid buffer generation to
+  // ProfilingFrameIterator to avoid incorrectly resetting the generation of
+  // sampled JIT entries inside the JS engine. See note below concerning 'J'
+  // entries.
+  uint32_t startBufferGen;
+  if (aSample->isSamplingCurrentThread) {
+    startBufferGen = UINT32_MAX;
+  } else {
+    startBufferGen = aProfile.bufferGeneration();
+  }
   uint32_t jsCount = 0;
   JS::ProfilingFrameIterator::Frame jsFrames[1000];
   // Only walk jit stack if profiling frame iterator is turned on.
   if (pseudoStack->mRuntime && JS::IsProfilingEnabledForRuntime(pseudoStack->mRuntime)) {
     AutoWalkJSStack autoWalkJSStack;
     const uint32_t maxFrames = mozilla::ArrayLength(jsFrames);
 
     if (aSample && autoWalkJSStack.walkAllowed) {
@@ -630,24 +639,23 @@ void mergeStacksIntoProfile(ThreadProfil
     // If we reach here, there must be a native stack entry and it must be the
     // greatest entry.
     MOZ_ASSERT(nativeStackAddr);
     MOZ_ASSERT(nativeIndex >= 0);
     aProfile.addTag(ProfileEntry('l', (void*)aNativeStack.pc_array[nativeIndex]));
     nativeIndex--;
   }
 
-  MOZ_ASSERT(aProfile.bufferGeneration() >= startBufferGen);
-  uint32_t lapCount = aProfile.bufferGeneration() - startBufferGen;
-
   // Update the JS runtime with the current profile sample buffer generation.
   //
   // Do not do this for synchronous sampling, which create their own
   // ProfileBuffers.
   if (!aSample->isSamplingCurrentThread && pseudoStack->mRuntime) {
+    MOZ_ASSERT(aProfile.bufferGeneration() >= startBufferGen);
+    uint32_t lapCount = aProfile.bufferGeneration() - startBufferGen;
     JS::UpdateJSRuntimeProfilerSampleBufferGen(pseudoStack->mRuntime,
                                                aProfile.bufferGeneration(),
                                                lapCount);
   }
 }
 
 #ifdef USE_NS_STACKWALK
 static
--- a/tools/profiler/platform.cpp
+++ b/tools/profiler/platform.cpp
@@ -169,17 +169,17 @@ ProfilerMarker::ProfilerMarker(const cha
 }
 
 ProfilerMarker::~ProfilerMarker() {
   free(mMarkerName);
   delete mPayload;
 }
 
 void
-ProfilerMarker::SetGeneration(int aGenID) {
+ProfilerMarker::SetGeneration(uint32_t aGenID) {
   mGenID = aGenID;
 }
 
 float
 ProfilerMarker::GetTime() {
   return mTime;
 }