Bug 1287392 - Part 1: Fix TaskTracer bugs caused by the changes of IPC. r=khuey, r=bgirard
☠☠ backed out by db56c201e562 ☠ ☠
authorThinker K.F. Li <thinker@codemud.net>
Thu, 10 Nov 2016 07:57:00 -0500
changeset 348852 606c7cb149c9609edf694e571922af7cbc8175e7
parent 348851 49d1c7361c57ac096cf28b76a0d9cf887c83a417
child 348853 ac7da68e00f545bde0975b7a945f8e53b21b3b76
push id10298
push userraliiev@mozilla.com
push dateMon, 14 Nov 2016 12:33:03 +0000
treeherdermozilla-aurora@7e29173b1641 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskhuey, bgirard
bugs1287392
milestone52.0a1
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.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.h
+++ b/tools/profiler/core/platform.h
@@ -54,27 +54,35 @@
 #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);
+}
+#elif defined(LINUX)
+#include <sys/types.h>
+pid_t gettid();
 #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