Deprecate ClientTraceActivity and merge it with GenericTraceActivity

Summary: 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.

Differential Revision: D27353973

fbshipit-source-id: 7830f89a7c6ebb431f7f4e6e70c2ef679d8858db

Author: chaekit

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

@@ -16,55 +16,51 @@
 
 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 {
+  [[nodiscard]] int64_t deviceId() const override {
     return cachedPid();
   }
 
-  int64_t resourceId() const override {
+  [[nodiscard]] int64_t resourceId() const override {
     return sysThreadId;
   }
 
-  int64_t timestamp() const override {
+  [[nodiscard]] int64_t timestamp() const override {
     return startTime;
   }
 
-  int64_t duration() const override {
+  [[nodiscard]] int64_t duration() const override {
     return endTime - startTime;
   }
 
-  int64_t correlationId() const override {
+  [[nodiscard]] int64_t correlationId() const override {
     return correlation;
   }
 
-  ActivityType type() const override {
-    return ActivityType::CPU_OP;
+  [[nodiscard]] ActivityType type() const override {
+    return activityType;
   }
 
-  const std::string name() const override {
+  [[nodiscard]] const std::string name() const override {
     return opType;
   }
 
-  const TraceActivity* linkedActivity() const override {
+  [[nodiscard]] 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);
+    auto kv = fmt::format("{}: {}", key, value);
     metadata_.push_back(std::move(kv));
   }
 
-  const std::string getMetadata() const {
+  [[nodiscard]] const std::string getMetadata() const {
     return fmt::format("{}", fmt::join(metadata_, ", "));
   }
 
@@ -75,6 +71,7 @@ struct ClientTraceActivity : TraceActivity {
   // TODO: Add OS abstraction
   int32_t sysThreadId{0};
   std::string opType;
+  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/output_base.h renamed to libkineto/include/output_base.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/ActivityProfiler.cpp

@@ -227,10 +227,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);
 
@@ -245,7 +245,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";
@@ -269,12 +269,12 @@ 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.timestamp();
   userTraceActivity.endTime = gpuTraceActivity.timestamp() + gpuTraceActivity.duration();
   userTraceActivity.correlation = cpuTraceActivity.correlationId();
   userTraceActivity.activityType = cpuTraceActivity.type();
-  userTraceActivity.activityName = cpuTraceActivity.name();
+  userTraceActivity.opType = cpuTraceActivity.name();
 }
 
 void ActivityProfiler::GpuUserEventMap::insertOrExtendEvent(
@@ -330,8 +330,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);
@@ -396,8 +396,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)) {
@@ -414,7 +414,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

@@ -122,9 +122,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);
 
@@ -140,7 +140,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

@@ -10,8 +10,12 @@
 
 using namespace libkineto;
 
-namespace KINETO_NAMESPACE {
+namespace libkineto {
   void GenericTraceActivity::log(ActivityLogger& logger) const {
+    if (activityType == ActivityType::CPU_OP) {
+      return;
+    }
+
     logger.handleGenericActivity(*this);
   }
-} // namespace KINETO_NAMESPACE
+} // namespace libkineto

libkineto/src/GenericTraceActivity.h

@@ -165,7 +165,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));
@@ -123,7 +123,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();}
@@ -139,7 +139,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_;
   };