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 322159 606c7cb149c9609edf694e571922af7cbc8175e7
parent 322158 49d1c7361c57ac096cf28b76a0d9cf887c83a417
child 322160 ac7da68e00f545bde0975b7a945f8e53b21b3b76
push id30941
push userkwierso@gmail.com
push dateFri, 11 Nov 2016 21:56:54 +0000
treeherdermozilla-central@fc104971a4db [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskhuey, bgirard
bugs1287392
milestone52.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.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