deprecate pthreadid (#56209)

Summary:
Pull Request resolved: pytorch/pytorch#56209

Pull Request resolved: #172

in this diff of the stack, we remove the threadId field from the ClientTraceActivity as our step towards the deprecation

Reviewed By: ilia-cher

Differential Revision: D27662747

fbshipit-source-id: 040ba040390680a0fc63ddc8149c6fad940439fc

Author: chaekit

libkineto/include/ActivityProfilerInterface.h

@@ -9,6 +9,7 @@
 
 #include <memory>
 #include <set>
+#include <thread>
 #include <vector>
 
 #include "ActivityType.h"
@@ -76,6 +77,10 @@ class ActivityProfilerInterface {
   virtual bool enableForRegion(const std::string& match) {
     return true;
   }
+
+  // Maps kernel thread id -> pthread id for CPU ops.
+  // Client must record any new kernel thread where the activity has occured.
+  virtual void recordThreadInfo(pid_t tid, pthread_t pthreadId) {}
 };
 
 } // namespace libkineto

libkineto/include/ClientTraceActivity.h

@@ -73,7 +73,6 @@ struct ClientTraceActivity : TraceActivity {
   int64_t correlation{0};
   int device{-1};
   // TODO: Add OS abstraction
-  pthread_t pthreadId{};
   int32_t sysThreadId{0};
   std::string opType;
 

libkineto/src/ActivityProfiler.cpp

@@ -183,11 +183,9 @@ void ActivityProfiler::processCpuTrace(
   CpuGpuSpanPair& span_pair = recordTraceSpan(cpuTrace.span, cpuTrace.gpuOpCount);
   TraceSpan& cpu_span = span_pair.first;
   for (auto const& act : cpuTrace.activities) {
-    VLOG(2) << act.correlationId() << ": OP " << act.opType
-            << " tid: " << act.pthreadId;
+    VLOG(2) << act.correlationId() << ": OP " << act.opType;
     if (logTrace) {
       logger.handleCpuActivity(act, cpu_span);
-      recordThreadInfo(act.sysThreadId, act.pthreadId);
     }
     // Stash event so we can look it up later when processing GPU trace
     externalEvents_.insertEvent(&act);

libkineto/src/ActivityProfiler.h

@@ -102,6 +102,15 @@ class ActivityProfiler {
     return *config_;
   }
 
+  inline void recordThreadInfo(pid_t tid, pthread_t pthreadId) {
+    std::lock_guard<std::mutex> guard(mutex_);
+    if (threadInfo_.find((int32_t)pthreadId) == threadInfo_.end()) {
+      threadInfo_.emplace(
+          (int32_t)pthreadId,
+          ThreadInfo((int32_t) tid, getThreadName(tid)));
+    }
+  }
+
  private:
   class ExternalEventMap {
    public:
@@ -256,14 +265,6 @@ class ActivityProfiler {
         disabledTraceSpans_.end();
   }
 
-  inline void recordThreadInfo(pid_t tid, pthread_t pthreadId) {
-    if (threadInfo_.find((int32_t)pthreadId) == threadInfo_.end()) {
-      threadInfo_.emplace(
-          (int32_t)pthreadId,
-          ThreadInfo((int32_t) tid, getThreadName(tid)));
-    }
-  }
-
   void resetTraceData();
 
   void addOverheadSample(profilerOverhead& counter, int64_t overhead) {

libkineto/src/ActivityProfilerController.h

@@ -57,6 +57,10 @@ class ActivityProfilerController {
     return profiler_->transferCpuTrace(std::move(cpuTrace));
   }
 
+  void recordThreadInfo(pid_t tid, pthread_t pthreadId) {
+    profiler_->recordThreadInfo(tid, pthreadId); 
+  }
+
  private:
   void profilerLoop();
 

libkineto/src/ActivityProfilerProxy.cpp

@@ -84,4 +84,8 @@ bool ActivityProfilerProxy::enableForRegion(const std::string& match) {
   return controller_->traceInclusionFilter(match);
 }
 
+void ActivityProfilerProxy::recordThreadInfo(pid_t tid, pthread_t pthreadId) {
+  controller_->recordThreadInfo(tid, pthreadId); 
+}
+
 } // namespace libkineto

libkineto/src/ActivityProfilerProxy.h

@@ -40,6 +40,8 @@ class ActivityProfilerProxy : public ActivityProfilerInterface {
 
   bool isActive() override;
 
+  void recordThreadInfo(pid_t tid, pthread_t pthreadId) override;
+
   void scheduleTrace(const std::string& configStr) override;
   void scheduleTrace(const Config& config);
 

libkineto/test/ActivityProfilerTest.cpp

@@ -45,7 +45,6 @@ struct MockCpuActivityBuffer : public CpuTraceBuffer {
     op.startTime = startTime;
     op.endTime = endTime;
     op.device = 0;
-    op.pthreadId = pthread_self();
     op.sysThreadId = 123;
     op.correlation = correlation;
     activities.push_back(std::move(op));
@@ -253,6 +252,8 @@ TEST_F(ActivityProfilerTest, SyncTrace) {
   profiler.startTrace(start_time);
   profiler.stopTrace(start_time + microseconds(duration_us));
 
+  profiler.recordThreadInfo(123, pthread_self());
+
   // Log some cpu ops
   auto cpuOps = std::make_unique<MockCpuActivityBuffer>(
       start_time_us, start_time_us + duration_us);
@@ -338,6 +339,8 @@ TEST_F(ActivityProfilerTest, CorrelatedTimestampTest) {
   // When launching kernel, the CPU event should always precede the GPU event.
   int64_t kernelLaunchTime = 120;
 
+  profiler.recordThreadInfo(123, pthread_self());
+
   // set up CPU event
   auto cpuOps = std::make_unique<MockCpuActivityBuffer>(
       start_time_us, start_time_us + duration_us);