Bug 1353630 (part 5) - Allocate PseudoStack within ThreadInfo's constructor. r=jseward.
authorNicholas Nethercote <nnethercote@mozilla.com>
Thu, 06 Apr 2017 09:40:28 +1000
changeset 400230 fddfbadf8fed92917b43e88020be9feff8cb2537
parent 400229 e02bb02196aaeb03763ceada5697ad883d8c75af
child 400231 3cb1462512b7bc2c181765854e7f144f70049912
push id7391
push usermtabara@mozilla.com
push dateMon, 12 Jun 2017 13:08:53 +0000
treeherdermozilla-beta@2191d7f87e2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjseward
bugs1353630
milestone55.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 1353630 (part 5) - Allocate PseudoStack within ThreadInfo's constructor. r=jseward.
tools/profiler/core/ThreadInfo.cpp
tools/profiler/core/ThreadInfo.h
tools/profiler/core/platform.cpp
tools/profiler/tests/gtest/ThreadProfileTest.cpp
--- a/tools/profiler/core/ThreadInfo.cpp
+++ b/tools/profiler/core/ThreadInfo.cpp
@@ -15,22 +15,21 @@
 #ifdef XP_WIN
 #include <process.h>
 #define getpid _getpid
 #else
 #include <unistd.h> // for getpid()
 #endif
 
 ThreadInfo::ThreadInfo(const char* aName, int aThreadId, bool aIsMainThread,
-                       mozilla::NotNull<PseudoStack*> aPseudoStack,
                        void* aStackTop)
   : mName(strdup(aName))
   , mThreadId(aThreadId)
   , mIsMainThread(aIsMainThread)
-  , mPseudoStack(aPseudoStack)
+  , mPseudoStack(mozilla::WrapNotNull(new PseudoStack()))
   , mPlatformData(AllocPlatformData(aThreadId))
   , mStackTop(aStackTop)
   , mHasProfile(false)
   , mLastSample()
 {
   MOZ_COUNT_CTOR(ThreadInfo);
 
   // We don't have to guess on mac
--- a/tools/profiler/core/ThreadInfo.h
+++ b/tools/profiler/core/ThreadInfo.h
@@ -12,41 +12,45 @@
 
 #include "ProfileBuffer.h"
 #include "platform.h"
 
 class ThreadInfo final
 {
 public:
   ThreadInfo(const char* aName, int aThreadId, bool aIsMainThread,
-             mozilla::NotNull<PseudoStack*> aPseudoStack, void* aStackTop);
+             void* aStackTop);
 
   ~ThreadInfo();
 
   const char* Name() const { return mName.get(); }
   int ThreadId() const { return mThreadId; }
 
   bool IsMainThread() const { return mIsMainThread; }
+
   mozilla::NotNull<PseudoStack*> Stack() const { return mPseudoStack; }
 
   void SetHasProfile() { mHasProfile = true; }
 
   PlatformData* GetPlatformData() const { return mPlatformData.get(); }
   void* StackTop() const { return mStackTop; }
 
   size_t SizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf) const;
 
   ProfileBuffer::LastSample& LastSample() { return mLastSample; }
 
 private:
   mozilla::UniqueFreePtr<char> mName;
   int mThreadId;
   const bool mIsMainThread;
 
-  // The thread's PseudoStack. This is an owning pointer.
+  // The thread's PseudoStack. This is an owning pointer. It could be an inline
+  // member, but we don't do that because PseudoStack is quite large, and we
+  // have ThreadInfo vectors and so we'd end up wasting a lot of space in those
+  // vectors for excess elements.
   mozilla::NotNull<PseudoStack*> mPseudoStack;
 
   UniquePlatformData mPlatformData;
   void* mStackTop;
 
   //
   // The following code is only used for threads that are being profiled, i.e.
   // for which SetHasProfile() has been called.
--- a/tools/profiler/core/platform.cpp
+++ b/tools/profiler/core/platform.cpp
@@ -1900,30 +1900,31 @@ locked_register_thread(PS::LockRef aLock
 
   MOZ_RELEASE_ASSERT(gPS);
 
   MOZ_RELEASE_ASSERT(!FindLiveThreadInfo(aLock));
 
   if (!tlsPseudoStack.init()) {
     return;
   }
-  NotNull<PseudoStack*> stack = WrapNotNull(new PseudoStack());
-  tlsPseudoStack.set(stack);
 
   ThreadInfo* info = new ThreadInfo(aName, Thread::GetCurrentId(),
-                                    NS_IsMainThread(), stack, stackTop);
+                                    NS_IsMainThread(), stackTop);
+  NotNull<PseudoStack*> pseudoStack = info->Stack();
+
+  tlsPseudoStack.set(pseudoStack.get());
 
   if (ShouldProfileThread(aLock, info)) {
     info->SetHasProfile();
 
     if (gPS->IsActive(aLock) && gPS->FeatureJS(aLock)) {
       // This startJSSampling() call is on-thread, so we can poll manually to
       // start JS sampling immediately.
-      stack->startJSSampling();
-      stack->pollJSSampling();
+      pseudoStack->startJSSampling();
+      pseudoStack->pollJSSampling();
     }
   }
 
   gPS->LiveThreads(aLock).push_back(info);
 }
 
 static void
 NotifyProfilerStarted(const int aEntries, double aInterval,
@@ -2398,18 +2399,18 @@ locked_profiler_start(PS::LockRef aLock,
   // Dead ThreadInfos are deleted in profiler_stop(), and dead ThreadInfos
   // aren't saved when the profiler is inactive. Therefore mDeadThreads should
   // be empty here.
   MOZ_RELEASE_ASSERT(gPS->DeadThreads(aLock).empty());
 
   if (featureJS) {
     // We just called startJSSampling() on all relevant threads. We can also
     // manually poll the current thread so it starts sampling immediately.
-    if (PseudoStack* stack = tlsPseudoStack.get()) {
-      stack->pollJSSampling();
+    if (PseudoStack* pseudoStack = tlsPseudoStack.get()) {
+      pseudoStack->pollJSSampling();
     }
   }
 
 #ifdef MOZ_TASK_TRACER
   if (featureTaskTracer) {
     mozilla::tasktracer::StartLogging();
   }
 #endif
--- a/tools/profiler/tests/gtest/ThreadProfileTest.cpp
+++ b/tools/profiler/tests/gtest/ThreadProfileTest.cpp
@@ -3,45 +3,39 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "gtest/gtest.h"
 
 #include "ProfileBufferEntry.h"
 #include "ThreadInfo.h"
 
-using mozilla::NotNull;
-using mozilla::WrapNotNull;
-
 // Make sure we can initialize our thread profile
 TEST(ThreadProfile, Initialization) {
   Thread::tid_t tid = 1000;
-  ThreadInfo info("testThread", tid, true, WrapNotNull(new PseudoStack()),
-                  nullptr);
+  ThreadInfo info("testThread", tid, true, nullptr);
   info.SetHasProfile();
 }
 
 // Make sure we can record one tag and read it
 TEST(ThreadProfile, InsertOneTag) {
   Thread::tid_t tid = 1000;
-  ThreadInfo info("testThread", tid, true, WrapNotNull(new PseudoStack()),
-                  nullptr);
+  ThreadInfo info("testThread", tid, true, nullptr);
   ProfileBuffer* pb = new ProfileBuffer(10);
   pb->addTag(ProfileBufferEntry::Time(123.1));
   ASSERT_TRUE(pb->mEntries != nullptr);
   ASSERT_TRUE(pb->mEntries[pb->mReadPos].kind() ==
               ProfileBufferEntry::Kind::Time);
   ASSERT_TRUE(pb->mEntries[pb->mReadPos].mTagDouble == 123.1);
 }
 
 // See if we can insert some tags
 TEST(ThreadProfile, InsertTagsNoWrap) {
   Thread::tid_t tid = 1000;
-  ThreadInfo info("testThread", tid, true, WrapNotNull(new PseudoStack()),
-                  nullptr);
+  ThreadInfo info("testThread", tid, true, nullptr);
   ProfileBuffer* pb = new ProfileBuffer(100);
   int test_size = 50;
   for (int i = 0; i < test_size; i++) {
     pb->addTag(ProfileBufferEntry::Time(i));
   }
   ASSERT_TRUE(pb->mEntries != nullptr);
   int readPos = pb->mReadPos;
   while (readPos != pb->mWritePos) {
@@ -53,18 +47,17 @@ TEST(ThreadProfile, InsertTagsNoWrap) {
 }
 
 // See if wrapping works as it should in the basic case
 TEST(ThreadProfile, InsertTagsWrap) {
   Thread::tid_t tid = 1000;
   // we can fit only 24 tags in this buffer because of the empty slot
   int tags = 24;
   int buffer_size = tags + 1;
-  ThreadInfo info("testThread", tid, true, WrapNotNull(new PseudoStack()),
-                  nullptr);
+  ThreadInfo info("testThread", tid, true, nullptr);
   ProfileBuffer* pb = new ProfileBuffer(buffer_size);
   int test_size = 43;
   for (int i = 0; i < test_size; i++) {
     pb->addTag(ProfileBufferEntry::Time(i));
   }
   ASSERT_TRUE(pb->mEntries != nullptr);
   int readPos = pb->mReadPos;
   int ctr = 0;