Bug 1287392 - Part 1: Fix TaskTracer bugs caused by the changes of IPC. r=khuey, r=bgirard
authorThinker K.F. Li <thinker@codemud.net>
Fri, 11 Nov 2016 20:26:00 +0100
changeset 322501 c8e5421730e37c4d9a221c877cddd334d3b7ded0
parent 322500 c861b79fc961add811816c80a4dac4d0f031d83e
child 322502 18bf88f09ab5c99f179e1a650d40b20519fb4248
push id83893
push usercbook@mozilla.com
push dateTue, 15 Nov 2016 08:10:35 +0000
treeherdermozilla-inbound@55b6cacff791 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskhuey, bgirard
bugs1287392
milestone53.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 1287392 - Part 1: Fix TaskTracer bugs caused by the changes of IPC. r=khuey, r=bgirard
ipc/chromium/src/chrome/common/ipc_message.cc
tools/profiler/core/GeckoSampler.cpp
tools/profiler/core/GeckoSampler.h
tools/profiler/core/platform-macos.cc
tools/profiler/core/platform.h
tools/profiler/tasktracer/GeckoTaskTracer.cpp
tools/profiler/tasktracer/GeckoTaskTracer.h
tools/profiler/tasktracer/TracedTaskCommon.cpp
tools/profiler/tasktracer/TracedTaskCommon.h
--- a/ipc/chromium/src/chrome/common/ipc_message.cc
+++ b/ipc/chromium/src/chrome/common/ipc_message.cc
@@ -33,19 +33,19 @@ Message::~Message() {
 Message::Message()
     : Pickle(sizeof(Header)) {
   MOZ_COUNT_CTOR(IPC::Message);
   header()->routing = header()->type = header()->flags = 0;
 #if defined(OS_POSIX)
   header()->num_fds = 0;
 #endif
 #ifdef MOZ_TASK_TRACER
-  header()->source_event_id = 0;
-  header()->parent_task_id = 0;
-  header()->source_event_type = SourceEventType::Unknown;
+  GetCurTraceInfo(&header()->source_event_id,
+                  &header()->parent_task_id,
+                  &header()->source_event_type);
 #endif
   InitLoggingVariables();
 }
 
 Message::Message(int32_t routing_id, msgid_t type, NestedLevel nestedLevel, PriorityValue priority,
                  MessageCompression compression, const char* const aName)
     : Pickle(sizeof(Header)) {
   MOZ_COUNT_CTOR(IPC::Message);
@@ -63,19 +63,19 @@ Message::Message(int32_t routing_id, msg
 #endif
   header()->interrupt_remote_stack_depth_guess = static_cast<uint32_t>(-1);
   header()->interrupt_local_stack_depth = static_cast<uint32_t>(-1);
   header()->seqno = 0;
 #if defined(OS_MACOSX)
   header()->cookie = 0;
 #endif
 #ifdef MOZ_TASK_TRACER
-  header()->source_event_id = 0;
-  header()->parent_task_id = 0;
-  header()->source_event_type = SourceEventType::Unknown;
+  GetCurTraceInfo(&header()->source_event_id,
+                  &header()->parent_task_id,
+                  &header()->source_event_type);
 #endif
   InitLoggingVariables(aName);
 }
 
 Message::Message(const char* data, int data_len)
   : Pickle(sizeof(Header), data, data_len)
 {
   MOZ_COUNT_CTOR(IPC::Message);
@@ -83,38 +83,28 @@ Message::Message(const char* data, int d
 }
 
 Message::Message(Message&& other) : Pickle(mozilla::Move(other)) {
   MOZ_COUNT_CTOR(IPC::Message);
   InitLoggingVariables(other.name_);
 #if defined(OS_POSIX)
   file_descriptor_set_ = other.file_descriptor_set_.forget();
 #endif
-#ifdef MOZ_TASK_TRACER
-  header()->source_event_id = other.header()->source_event_id;
-  header()->parent_task_id = other.header()->parent_task_id;
-  header()->source_event_type = other.header()->source_event_type;
-#endif
 }
 
 void Message::InitLoggingVariables(const char* const aName) {
   name_ = aName;
 }
 
 Message& Message::operator=(Message&& other) {
   *static_cast<Pickle*>(this) = mozilla::Move(other);
   InitLoggingVariables(other.name_);
 #if defined(OS_POSIX)
   file_descriptor_set_.swap(other.file_descriptor_set_);
 #endif
-#ifdef MOZ_TASK_TRACER
-  std::swap(header()->source_event_id, other.header()->source_event_id);
-  std::swap(header()->parent_task_id, other.header()->parent_task_id);
-  std::swap(header()->source_event_type, other.header()->source_event_type);
-#endif
   return *this;
 }
 
 
 #if defined(OS_POSIX)
 bool Message::WriteFileDescriptor(const base::FileDescriptor& descriptor) {
   // We write the index of the descriptor so that we don't have to
   // keep the current descriptor as extra decoding state when deserialising.
--- a/tools/profiler/core/GeckoSampler.cpp
+++ b/tools/profiler/core/GeckoSampler.cpp
@@ -60,21 +60,16 @@
 
 #if defined(XP_WIN)
 typedef CONTEXT tickcontext_t;
 #elif defined(LINUX)
 #include <ucontext.h>
 typedef ucontext_t tickcontext_t;
 #endif
 
-#if defined(LINUX) || defined(XP_MACOSX)
-#include <sys/types.h>
-pid_t gettid();
-#endif
-
 #if defined(__arm__) && defined(ANDROID)
  // Should also work on ARM Linux, but not tested there yet.
  #define USE_EHABI_STACKWALK
 #endif
 #ifdef USE_EHABI_STACKWALK
  #include "EHABIStackWalk.h"
 #endif
 
@@ -286,16 +281,22 @@ GeckoSampler::~GeckoSampler()
   }
 #if defined(XP_WIN)
   delete mIntelPowerGadget;
 #endif
 
   // Cancel any in-flight async profile gatherering
   // requests
   mGatherer->Cancel();
+
+#ifdef MOZ_TASK_TRACER
+  if (mTaskTracer) {
+    mozilla::tasktracer::StopLogging();
+  }
+#endif
 }
 
 void GeckoSampler::HandleSaveRequest()
 {
   if (!mSaveRequested)
     return;
   mSaveRequested = false;
 
--- a/tools/profiler/core/GeckoSampler.h
+++ b/tools/profiler/core/GeckoSampler.h
@@ -78,21 +78,16 @@ class GeckoSampler: public Sampler {
 
   // Immediately captures the calling thread's call stack and returns it.
   virtual SyncProfile* GetBacktrace() override;
 
   // Called within a signal. This function must be reentrant
   virtual void RequestSave() override
   {
     mSaveRequested = true;
-#ifdef MOZ_TASK_TRACER
-    if (mTaskTracer) {
-      mozilla::tasktracer::StopLogging();
-    }
-#endif
   }
 
   virtual void HandleSaveRequest() override;
   virtual void DeleteExpiredMarkers() override;
 
   ThreadProfile* GetPrimaryThreadProfile()
   {
     if (!mPrimaryThreadProfile) {
--- a/tools/profiler/core/platform-macos.cc
+++ b/tools/profiler/core/platform-macos.cc
@@ -353,22 +353,16 @@ void Sampler::Stop() {
 }
 
 pthread_t
 Sampler::GetProfiledThread(PlatformData* aData)
 {
   return aData->profiled_pthread();
 }
 
-#include <sys/syscall.h>
-pid_t gettid()
-{
-  return (pid_t) syscall(SYS_thread_selfid);
-}
-
 /* static */ Thread::tid_t
 Thread::GetCurrentId()
 {
   return gettid();
 }
 
 bool Sampler::RegisterCurrentThread(const char* aName,
                                     PseudoStack* aPseudoStack,
--- a/tools/profiler/core/platform.h
+++ b/tools/profiler/core/platform.h
@@ -54,27 +54,32 @@
 #include "mozilla/TimeStamp.h"
 #include "mozilla/UniquePtr.h"
 #include "mozilla/Unused.h"
 #include "PlatformMacros.h"
 #include "v8-support.h"
 #include <vector>
 #include "StackTop.h"
 
-// We need a definition of gettid(), but Linux libc implementations don't
-// provide a wrapper for it (except for Bionic)
-#if defined(__linux__)
+// We need a definition of gettid(), but glibc doesn't provide a
+// wrapper for it.
+#if defined(__GLIBC__)
 #include <unistd.h>
-#if !defined(__BIONIC__)
 #include <sys/syscall.h>
 static inline pid_t gettid()
 {
   return (pid_t) syscall(SYS_gettid);
 }
-#endif
+#elif defined(XP_MACOSX)
+#include <unistd.h>
+#include <sys/syscall.h>
+static inline pid_t gettid()
+{
+  return (pid_t) syscall(SYS_thread_selfid);
+}
 #endif
 
 #ifdef XP_WIN
 #include <windows.h>
 #endif
 
 #define ASSERT(a) MOZ_ASSERT(a)
 
--- a/tools/profiler/tasktracer/GeckoTaskTracer.cpp
+++ b/tools/profiler/tasktracer/GeckoTaskTracer.cpp
@@ -15,37 +15,16 @@
 #include "mozilla/Unused.h"
 
 #include "nsString.h"
 #include "nsThreadUtils.h"
 #include "prtime.h"
 
 #include <stdarg.h>
 
-// We need a definition of gettid(), but glibc doesn't provide a
-// wrapper for it.
-#if defined(__GLIBC__)
-#include <unistd.h>
-#include <sys/syscall.h>
-static inline pid_t gettid()
-{
-  return (pid_t) syscall(SYS_gettid);
-}
-#elif defined(XP_MACOSX)
-#include <unistd.h>
-#include <sys/syscall.h>
-static inline pid_t gettid()
-{
-  return (pid_t) syscall(SYS_thread_selfid);
-}
-#elif defined(LINUX)
-#include <sys/types.h>
-pid_t gettid();
-#endif
-
 // NS_ENSURE_TRUE_VOID() without the warning on the debug build.
 #define ENSURE_TRUE_VOID(x)   \
   do {                        \
     if (MOZ_UNLIKELY(!(x))) { \
        return;                \
     }                         \
   } while(0)
 
@@ -179,17 +158,17 @@ SetLogStarted(bool aIsStartLogging)
       (*sTraceInfos)[i]->mObsolete = true;
     }
   }
 }
 
 static void
 CleanUp()
 {
-  SetLogStarted(false);
+  MOZ_ASSERT(!IsStartLogging());
   StaticMutexAutoLock lock(sMutex);
 
   if (sTraceInfos) {
     delete sTraceInfos;
     sTraceInfos = nullptr;
   }
 }
 
@@ -225,45 +204,45 @@ InitTaskTracer(uint32_t aFlags)
   if (aFlags & FORKED_AFTER_NUWA) {
     ObsoleteCurrentTraceInfos();
     return;
   }
 
   MOZ_ASSERT(!sTraceInfos);
   sTraceInfos = new nsTArray<UniquePtr<TraceInfo>>();
 
-  if (!sTraceInfoTLS.initialized()) {
-    Unused << sTraceInfoTLS.init();
-  }
+  sTraceInfoTLS.init();
 }
 
 void
 ShutdownTaskTracer()
 {
   CleanUp();
 }
 
 static void
 FreeTraceInfo(TraceInfo* aTraceInfo)
 {
   StaticMutexAutoLock lock(sMutex);
   if (aTraceInfo) {
-    sTraceInfos->RemoveElement(aTraceInfo);
+    UniquePtr<TraceInfo> traceinfo(aTraceInfo);
+    sTraceInfos->RemoveElement(traceinfo);
+    Unused << traceinfo.release(); // A dirty hack to prevent double free.
   }
 }
 
 void FreeTraceInfo()
 {
   FreeTraceInfo(sTraceInfoTLS.get());
 }
 
 TraceInfo*
 GetOrCreateTraceInfo()
 {
-  ENSURE_TRUE(sTraceInfoTLS.initialized(), nullptr);
+  ENSURE_TRUE(sTraceInfoTLS.init(), nullptr);
   ENSURE_TRUE(IsStartLogging(), nullptr);
 
   TraceInfo* info = sTraceInfoTLS.get();
   if (info && info->mObsolete) {
     // TraceInfo is obsolete: remove it.
     FreeTraceInfo(info);
     info = nullptr;
   }
--- a/tools/profiler/tasktracer/GeckoTaskTracer.h
+++ b/tools/profiler/tasktracer/GeckoTaskTracer.h
@@ -20,17 +20,16 @@
  * Source Events are usually some kinds of I/O events we're interested in, such
  * as touch events, timer events, network events, etc. When a source event is
  * created, TaskTracer records the entire chain of Tasks and nsRunnables as they
  * are dispatched to different threads and processes. It records latency,
  * execution time, etc. for each Task and nsRunnable that chains back to the
  * original source event.
  */
 
-class Task;
 class nsIRunnable;
 class nsCString;
 
 namespace mozilla {
 
 class TimeStamp;
 
 namespace tasktracer {
@@ -69,18 +68,16 @@ UniquePtr<nsTArray<nsCString>> GetLogged
 
 // Returns the timestamp when Task Tracer is enabled in this process.
 const PRTime GetStartTime();
 
 /**
  * Internal functions.
  */
 
-Task* CreateTracedTask(Task* aTask);
-
 already_AddRefed<nsIRunnable>
 CreateTracedRunnable(already_AddRefed<nsIRunnable>&& aRunnable);
 
 // Free the TraceInfo allocated on a thread's TLS. Currently we are wrapping
 // tasks running on nsThreads and base::thread, so FreeTraceInfo is called at
 // where nsThread and base::thread release themselves.
 void FreeTraceInfo();
 
--- a/tools/profiler/tasktracer/TracedTaskCommon.cpp
+++ b/tools/profiler/tasktracer/TracedTaskCommon.cpp
@@ -110,60 +110,20 @@ TracedRunnable::Run()
   nsresult rv = mOriginalObj->Run();
   LogEnd(mTaskId, mSourceEventId);
   ClearTLSTraceInfo();
 
   return rv;
 }
 
 /**
- * Implementation of class TracedTask.
- */
-TracedTask::TracedTask(Task* aOriginalObj)
-  : TracedTaskCommon()
-  , mOriginalObj(aOriginalObj)
-{
-  Init();
-  LogVirtualTablePtr(mTaskId, mSourceEventId, reinterpret_cast<uintptr_t*>(aOriginalObj));
-}
-
-TracedTask::~TracedTask()
-{
-  if (mOriginalObj) {
-    delete mOriginalObj;
-    mOriginalObj = nullptr;
-  }
-}
-
-void
-TracedTask::Run()
-{
-  SetTLSTraceInfo();
-  LogBegin(mTaskId, mSourceEventId);
-  mOriginalObj->Run();
-  LogEnd(mTaskId, mSourceEventId);
-  ClearTLSTraceInfo();
-}
-
-/**
  * CreateTracedRunnable() returns a TracedRunnable wrapping the original
  * nsIRunnable object, aRunnable.
  */
 already_AddRefed<nsIRunnable>
 CreateTracedRunnable(already_AddRefed<nsIRunnable>&& aRunnable)
 {
   nsCOMPtr<nsIRunnable> runnable = new TracedRunnable(Move(aRunnable));
   return runnable.forget();
 }
 
-/**
- * CreateTracedTask() returns a TracedTask wrapping the original Task object,
- * aTask.
- */
-Task*
-CreateTracedTask(Task* aTask)
-{
-  Task* task = new TracedTask(aTask);
-  return task;
-}
-
 } // namespace tasktracer
 } // namespace mozilla
--- a/tools/profiler/tasktracer/TracedTaskCommon.h
+++ b/tools/profiler/tasktracer/TracedTaskCommon.h
@@ -36,38 +36,25 @@ protected:
   SourceEventType mSourceEventType;
   uint64_t mSourceEventId;
   uint64_t mParentTaskId;
   uint64_t mTaskId;
   bool mIsTraceInfoInit;
 };
 
 class TracedRunnable : public TracedTaskCommon
-                     , public nsRunnable
+                     , public Runnable
 {
 public:
   NS_DECL_NSIRUNNABLE
 
   TracedRunnable(already_AddRefed<nsIRunnable>&& aOriginalObj);
 
 private:
   virtual ~TracedRunnable();
 
   nsCOMPtr<nsIRunnable> mOriginalObj;
 };
 
-class TracedTask : public TracedTaskCommon
-                 , public Task
-{
-public:
-  TracedTask(Task* aOriginalObj);
-  ~TracedTask();
-
-  virtual void Run();
-
-private:
-  Task* mOriginalObj;
-};
-
 } // namespace tasktracer
 } // namespace mozilla
 
 #endif