Deprecate ClientTraceActivity and merge it with GenericTraceActivity (#56743)

Summary:
Pull Request resolved: pytorch/pytorch#56743

Pull Request resolved: #184

as part of the migration to ClientTraceActivity -> GenericTraceActivity, now that all CTA mirrors GTA's data structure, we can safely swap out the symbol name.

Reviewed By: gdankel

Differential Revision: D27353973

fbshipit-source-id: f965c979ae4562d437ac57200c6ecb58a231fde2

Author: chaekit

libkineto/include/ClientTraceActivity.h renamed to libkineto/include/GenericTraceActivity.h

@@ -16,11 +16,9 @@
 
 namespace libkineto {
 
-struct ClientTraceActivity : TraceActivity {
-  ClientTraceActivity() = default;
-  ClientTraceActivity(ClientTraceActivity&&) = default;
-  ClientTraceActivity& operator=(ClientTraceActivity&&) = default;
-  ~ClientTraceActivity() override {}
+// @lint-ignore-every CLANGTIDY cppcoreguidelines-non-private-member-variables-in-classes
+// @lint-ignore-every CLANGTIDY cppcoreguidelines-pro-type-member-init
+struct GenericTraceActivity : TraceActivity {
 
   int64_t deviceId() const override {
     return cachedPid();
@@ -43,28 +41,26 @@ struct ClientTraceActivity : TraceActivity {
   }
 
   ActivityType type() const override {
-    return ActivityType::CPU_OP;
+    return activityType;
   }
 
   const std::string name() const override {
-    return opType;
+    return activityName;
   }
 
   const TraceActivity* linkedActivity() const override {
     return nullptr;
   }
 
-  void log(ActivityLogger& logger) const override {
-    // Unimplemented by default
-  }
+  void log(ActivityLogger& logger) const override;
 
-  // Encode client side metadata as a key/value string.
+  //Encode client side metadata as a key/value string.
   void addMetadata(const std::string& key, const std::string& value) {
     auto kv = fmt::format("\"{}\": {}", key, value);
     metadata_.push_back(std::move(kv));
   }
 
-  const std::string getMetadata() const {
+   const std::string getMetadata() const {
     return fmt::format("{}", fmt::join(metadata_, ", "));
   }
 
@@ -74,7 +70,8 @@ struct ClientTraceActivity : TraceActivity {
   int device{-1};
   // TODO: Add OS abstraction
   int32_t sysThreadId{0};
-  std::string opType;
+  std::string activityName;
+  ActivityType activityType;
 
  private:
   std::vector<std::string> metadata_;

libkineto/include/libkineto.h

@@ -24,7 +24,7 @@
 #include "ActivityTraceInterface.h"
 #include "ActivityType.h"
 #include "ClientInterface.h"
-#include "ClientTraceActivity.h"
+#include "GenericTraceActivity.h"
 #include "TraceSpan.h"
 
 extern "C" {
@@ -39,7 +39,7 @@ class Config;
 struct CpuTraceBuffer {
   TraceSpan span;
   int gpuOpCount;
-  std::vector<ClientTraceActivity> activities;
+  std::vector<GenericTraceActivity> activities;
 };
 
 class LibkinetoApi {

libkineto/src/ActivityProfiler.cpp

@@ -184,7 +184,7 @@ 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;
+    VLOG(2) << act.correlationId() << ": OP " << act.activityName;
     if (logTrace) {
       logger.handleCpuActivity(act, cpu_span);
     }
@@ -228,10 +228,10 @@ inline void ActivityProfiler::handleCorrelationActivity(
 }
 #endif // HAS_CUPTI
 
-const libkineto::ClientTraceActivity&
-ActivityProfiler::ExternalEventMap::getClientTraceActivity(
+const libkineto::GenericTraceActivity&
+ActivityProfiler::ExternalEventMap::getGenericTraceActivity(
   uint32_t id, CorrelationFlowType flowType) {
-  static const libkineto::ClientTraceActivity nullOp_{};
+  static const libkineto::GenericTraceActivity nullOp_{};
 
   auto& correlationMap = getCorrelationMap(flowType);
 
@@ -246,7 +246,7 @@ ActivityProfiler::ExternalEventMap::getClientTraceActivity(
 }
 
 void ActivityProfiler::ExternalEventMap::insertEvent(
-    const libkineto::ClientTraceActivity* op) {
+    const libkineto::GenericTraceActivity* op) {
   if (events_[op->correlationId()] != nullptr) {
     LOG_EVERY_N(WARNING, 100)
         << "Events processed out of order - link will be missing";
@@ -270,8 +270,8 @@ static void initUserGpuSpan(GenericTraceActivity& userTraceActivity,
   const libkineto::TraceActivity& cpuTraceActivity,
   const libkineto::TraceActivity& gpuTraceActivity) {
   userTraceActivity.device = gpuTraceActivity.deviceId();
-  userTraceActivity.resource = gpuTraceActivity.resourceId();
   userTraceActivity.startTime = gpuTraceActivity.timestamp();
+  userTraceActivity.sysThreadId = gpuTraceActivity.resourceId();
   userTraceActivity.endTime = gpuTraceActivity.timestamp() + gpuTraceActivity.duration();
   userTraceActivity.correlation = cpuTraceActivity.correlationId();
   userTraceActivity.activityType = cpuTraceActivity.type();
@@ -331,8 +331,8 @@ inline void ActivityProfiler::handleRuntimeActivity(
   VLOG(2) << activity->correlationId
           << ": CUPTI_ACTIVITY_KIND_RUNTIME, cbid=" << activity->cbid
           << " tid=" << activity->threadId;
-  const ClientTraceActivity& ext =
-    externalEvents_.getClientTraceActivity(activity->correlationId,
+  const GenericTraceActivity& ext =
+    externalEvents_.getGenericTraceActivity(activity->correlationId,
       ExternalEventMap::CorrelationFlowType::Default);
   int32_t tid = activity->threadId;
   const auto& it = threadInfo_.find(tid);
@@ -397,8 +397,8 @@ inline void ActivityProfiler::handleGpuActivity(
   if (!loggingDisabled(ext)) {
     act.log(*logger);
     updateGpuNetSpan(act);
-    const ClientTraceActivity& extUser =
-      externalEvents_.getClientTraceActivity(act.correlationId(),
+    const GenericTraceActivity& extUser =
+      externalEvents_.getGenericTraceActivity(act.correlationId(),
         ExternalEventMap::CorrelationFlowType::User);
     // Correlated CPU activity cannot have timestamp greater than the GPU activity's
     if (!timestampsInCorrectOrder(extUser, act)) {
@@ -415,7 +415,7 @@ inline void ActivityProfiler::handleGpuActivity(
 
 template <class T>
 inline void ActivityProfiler::handleGpuActivity(const T* act, ActivityLogger* logger) {
-  const ClientTraceActivity& extDefault = externalEvents_.getClientTraceActivity(act->correlationId,
+  const GenericTraceActivity& extDefault = externalEvents_.getGenericTraceActivity(act->correlationId,
       ExternalEventMap::CorrelationFlowType::Default);
   handleGpuActivity(GpuActivity<T>(act, extDefault), logger);
 }

libkineto/src/ActivityProfiler.h

@@ -127,9 +127,9 @@ class ActivityProfiler {
     };
 
     // The correlation id of the GPU activity
-    const libkineto::ClientTraceActivity& getClientTraceActivity(
+    const libkineto::GenericTraceActivity& getGenericTraceActivity(
       uint32_t correlation_id, CorrelationFlowType flowType);
-    void insertEvent(const libkineto::ClientTraceActivity* op);
+    void insertEvent(const libkineto::GenericTraceActivity* op);
 
     void addCorrelation(uint64_t external_id, uint32_t cuda_id, CorrelationFlowType flowType);
 
@@ -145,7 +145,7 @@ class ActivityProfiler {
     // but this class also fully owns the objects it is pointing to so
     // it's not so bad. This is done for performance reasons and is an
     // implementation detail of this class that might change.
-    std::unordered_map<uint64_t, const libkineto::ClientTraceActivity*>
+    std::unordered_map<uint64_t, const libkineto::GenericTraceActivity*>
         events_;
 
     // Cuda correlation id -> external correlation id for default events

libkineto/src/GenericTraceActivity.cpp

@@ -8,10 +8,13 @@
 #include "GenericTraceActivity.h"
 #include "output_base.h"
 
-using namespace libkineto;
-
-namespace KINETO_NAMESPACE {
+namespace libkineto {
   void GenericTraceActivity::log(ActivityLogger& logger) const {
+    // TODO(T89833634): Merge handleGenericTraceActivity and handleCpuActivity
+    if (activityType == ActivityType::CPU_OP) {
+      return;
+    }
+
     logger.handleGenericActivity(*this);
   }
-} // namespace KINETO_NAMESPACE
+} // namespace libkineto

libkineto/src/GenericTraceActivity.h

@@ -18,15 +18,14 @@
 #include "CuptiActivity.h"
 #endif // HAS_CUPTI
 #include "ActivityBuffers.h"
-#include "ClientTraceActivity.h"
 #include "GenericTraceActivity.h"
 #include "ProcessInfo.h"
 #include "TraceSpan.h"
 
 namespace KINETO_NAMESPACE {
   class Config;
-  class RuntimeActivity;
   class GpuKernelActivity;
+  struct RuntimeActivity;
 }
 
 namespace libkineto {
@@ -48,7 +47,7 @@ class ActivityLogger {
   virtual void handleIterationStart(const TraceSpan& span) = 0;
 
   virtual void handleCpuActivity(
-      const libkineto::ClientTraceActivity& activity,
+      const libkineto::GenericTraceActivity& activity,
       const TraceSpan& span) = 0;
 
   virtual void handleGenericActivity(

libkineto/src/output_base.h

@@ -185,7 +185,7 @@ static std::string traceActivityJson(const TraceActivity& activity, std::string
 }
 
 void ChromeTraceLogger::handleCpuActivity(
-    const libkineto::ClientTraceActivity& op,
+    const libkineto::GenericTraceActivity& op,
     const TraceSpan& span) {
   if (!traceOf_) {
     return;

libkineto/src/output_json.cpp

@@ -16,7 +16,7 @@
 #ifdef HAS_CUPTI
 #include <cupti.h>
 #endif
-#include "ClientTraceActivity.h"
+#include "GenericTraceActivity.h"
 #include "output_base.h"
 
 namespace libkineto {
@@ -44,7 +44,7 @@ class ChromeTraceLogger : public libkineto::ActivityLogger {
   void handleIterationStart(const TraceSpan& span) override;
 
   void handleCpuActivity(
-      const libkineto::ClientTraceActivity& activity,
+      const libkineto::GenericTraceActivity& activity,
       const TraceSpan& span) override;
 
   void handleGenericActivity(

libkineto/src/output_json.h

@@ -17,7 +17,7 @@
 #endif
 
 #include "Config.h"
-#include "ClientTraceActivity.h"
+#include "GenericTraceActivity.h"
 #ifdef HAS_CUPTI
 #include "CuptiActivity.h"
 #include "CuptiActivity.tpp"
@@ -55,7 +55,7 @@ class MemoryTraceLogger : public ActivityLogger {
   }
 
   void handleCpuActivity(
-      const libkineto::ClientTraceActivity& activity,
+      const libkineto::GenericTraceActivity& activity,
       const TraceSpan& span) override {
     activities_.push_back(
         std::make_unique<CpuActivityDecorator>(activity, span));
@@ -129,7 +129,7 @@ class MemoryTraceLogger : public ActivityLogger {
 
   struct CpuActivityDecorator : public libkineto::TraceActivity {
     CpuActivityDecorator(
-        const libkineto::ClientTraceActivity& activity,
+        const libkineto::GenericTraceActivity& activity,
         const TraceSpan& span)
         : wrappee_(activity), span_(span) {}
     int64_t deviceId() const override {return wrappee_.deviceId();}
@@ -145,7 +145,7 @@ class MemoryTraceLogger : public ActivityLogger {
     void log(ActivityLogger& logger) const override {
       logger.handleCpuActivity(wrappee_, span_);
     }
-    const libkineto::ClientTraceActivity& wrappee_;
+    const libkineto::GenericTraceActivity& wrappee_;
     const TraceSpan span_;
   };