SWDEV-406670: Handling repeated counters

Change-Id: I90e71463b8ba14659d55d6e916b1ad26257e8ccb

CHANGELOG.md

@@ -274,3 +274,4 @@ Example for file plugin output:
 - Some browsers caching ATT data from older kernels.
 - Navi2x GPUs required the first counter to be GRBM. This is fixed in 5.7.
 - If ROCPROFILER_METRICS_PATH environment variable is not set, the counters xml path will be taken from the following path (../libexec/rocprofiler/counters/derived_counters.xml) which is relative to librocprofiler64.so.2.0.0
+- Repeated base metrics were not being properly reused by derived counters.

src/core/counters/metrics/eval_metrics.cpp

@@ -103,42 +103,28 @@ bool metrics::ExtractMetricEvents(
           Agent::AgentInfo& agentInfo = rocprofiler::hsa_support::GetAgentInfo(gpu_agent.handle);
           fatal("input metric'%s' not supported on this hardware: %s ", metric_names[i].c_str(),
           agentInfo.getName().data());
-
       }
 
       // adding result object for derived metric
       std::lock_guard<std::mutex> lock(extract_metric_events_lock);
-      if (results_map.find(metric_names[i]) == results_map.end()) {
-        results_map[metric_names[i]] = new results_t(metric_names[i], {}, xcc_count);
-      }
 
       counters_vec = metric->GetCounters();
       if (counters_vec.empty())
         rocprofiler::fatal("bad metric '%s' is empty", metric_names[i].c_str());
 
-      for (const counter_t* counter : counters_vec) {
-        results_t* result = nullptr;
-        if (metric->GetExpr()) {
+      if (metric->GetExpr() && results_map.find(metric_names[i]) == results_map.end()) {
+        results_map[metric_names[i]] = new results_t(metric_names[i], {}, xcc_count);
+        for (const counter_t* counter : counters_vec)
           metrics_counters[metric->GetName()].insert(counter->name);
-          // add this counter event only if it wasn't repeated before
-          if (results_map.find(counter->name) != results_map.end()) {
-            // std::cout << "Metric : " << metric->GetName() << " has " << counter->name
-            //           << " which is already part of the results map!" << std::endl;
-            // continue;
-            result = results_map.at(counter->name);
-          } else {
-            // result object for base metric
-            // std::cout << "Metric : " << metric->GetName() << " : " << counter->name << std::endl;
-            result =
-                new results_t(counter->name, {}, xcc_count);  // TODO: set correct initial value
-            results_map[counter->name] = result;
-          }
-        } else {
-          // std::cout << "Counter : " << metric->GetName() << " : " << counter->name << std::endl;
-          result = results_map.at(counter->name);
-        }
-        // std::cout << "General Counter : " << metric->GetName() << " : " << counter->name <<
-        // std::endl;
+      }
+
+      for (const counter_t* counter : counters_vec) {
+        if (results_map.find(counter->name) != results_map.end())
+          continue;
+
+        results_t* result = new results_t(counter->name, {}, xcc_count);
+        results_map[counter->name] = result;
+
         const event_t* event = &(counter->event);
         const block_des_t block_des = {event->block_name, event->block_index};
         auto ret = groups_map.insert({block_des, {}});
@@ -156,13 +142,6 @@ bool metrics::ExtractMetricEvents(
           block_status.max_counters = max_block_counters;
         }
 
-        // std::cout << "Counter: " << result->name << ", Block Index: " <<
-        // counter->event.block_index
-        //           << ", Counter ID: " << counter->event.counter_id
-        //           << "\nBlock Status Group Index: " << block_status.group_index
-        //           << ", Block Max Counters: " << block_status.max_counters
-        //           << ", Block Status Counter ID: " << block_status.counter_index << std::endl;
-
         if (block_status.counter_index >= block_status.max_counters) {
           rocprofiler::fatal("Metrics specified have exceeded HW limits!");
           return false;

tests-v2/featuretests/profiler/profiler_gtest.cpp

@@ -53,7 +53,7 @@ static void init_test_path() {
     lib_path = "lib/rocprofiler/librocprofiler_tool.so";
     golden_trace_path = "share/rocprofiler/tests/featuretests/profiler/apps/goldentraces/";
     test_app_path = "share/rocprofiler/tests/featuretests/profiler/apps/";
-    metrics_path = "lib/rocprofiler/gfx_metrics.xml";
+    metrics_path = "libexec/rocprofiler/counters/derived_counters.xml";
     binary_path = "bin/rocprofv2";
     profiler_api_lib_path = "/lib";
   } else {
@@ -62,11 +62,20 @@ static void init_test_path() {
     lib_path = "librocprofiler_tool.so";
     golden_trace_path = "tests-v2/featuretests/profiler/apps/goldentraces/";
     test_app_path = "tests-v2/featuretests/profiler/apps/";
-    metrics_path = "gfx_metrics.xml";
+    metrics_path = "counters/derived_counters.xml";
     binary_path = "rocprofv2";
   }
 }
 
+
+void __attribute__((constructor)) globalsetting() {
+  init_test_path();
+  std::string app_path = GetRunningPath(running_path);
+  std::stringstream gfx_path;
+  gfx_path << app_path << metrics_path;
+  setenv("ROCPROFILER_METRICS_PATH", gfx_path.str().c_str(), true);
+}
+
 /**
  * Sets application enviornment by seting HSA_TOOLS_LIB.
  */
@@ -672,14 +681,6 @@ TEST_F(ATTCollection, WhenRunningATTItCollectsTraceData) {
  * ###################################################
  */
 
-void __attribute__((constructor)) globalsetting() {
-  init_test_path();
-  std::string app_path = GetRunningPath(running_path);
-  std::stringstream gfx_path;
-  gfx_path << app_path << metrics_path;
-  setenv("ROCPROFILER_METRICS_PATH", gfx_path.str().c_str(), true);
-}
-
 class ProfilerAPITest : public ::testing::Test {
  protected:
   void SetUp() {
@@ -780,6 +781,78 @@ TEST_F(ProfilerAPITest, WhenRunningMultipleThreadsProfilerAPIsWorkFine) {
   CheckApi(rocprofiler_finalize());
 }
 
+/*
+ * ###################################################
+ * ############ Derived metrics tests ################
+ * ###################################################
+ */
+class DerivedMetricsReuseTest : public ::testing::Test {
+ protected:
+  void SetUp() {}
+  // function to check profiler API status
+  static void CheckApi(rocprofiler_status_t status) {
+    ASSERT_EQ(status, ROCPROFILER_STATUS_SUCCESS);
+  };
+
+  // callback function to dump profiler data
+  static void FlushCallback(const rocprofiler_record_header_t* record,
+                            const rocprofiler_record_header_t* end_record,
+                            rocprofiler_session_id_t session_id,
+                            rocprofiler_buffer_id_t buffer_id) {
+    while (record < end_record) {
+      if (!record) break;
+      CheckApi(rocprofiler_next_record(record, &record, session_id, buffer_id));
+    }
+  }
+};
+
+TEST_F(DerivedMetricsReuseTest, WhenRunningRepeatedBaseMetricsAPIsWorkFine) {
+  // set global path
+  init_test_path();
+
+  // initialize profiler by creating rocprofiler object
+  CheckApi(rocprofiler_initialize());
+
+  // Counter Collection with timestamps
+  rocprofiler_session_id_t session_id;
+  std::vector<const char*> counters;
+  counters.emplace_back("GRBM_COUNT");
+  counters.emplace_back("GPUBusy");
+  counters.emplace_back("GRBM_GUI_ACTIVE");
+  counters.emplace_back("ALUStalledByLDS");
+
+  CheckApi(rocprofiler_create_session(ROCPROFILER_KERNEL_REPLAY_MODE, &session_id));
+
+  rocprofiler_buffer_id_t buffer_id;
+  CheckApi(rocprofiler_create_buffer(session_id, FlushCallback, 0x9999, &buffer_id));
+
+  rocprofiler_filter_id_t filter_id;
+  rocprofiler_filter_property_t property = {};
+  CheckApi(rocprofiler_create_filter(session_id, ROCPROFILER_COUNTERS_COLLECTION,
+                                     rocprofiler_filter_data_t{.counters_names = &counters[0]},
+                                     counters.size(), &filter_id, property));
+
+  CheckApi(rocprofiler_set_filter_buffer(session_id, filter_id, buffer_id));
+
+  // activating profiler session
+  CheckApi(rocprofiler_start_session(session_id));
+
+  // launch kernel on each thread
+  KernelLaunch();
+
+  // deactivate session
+  CheckApi(rocprofiler_terminate_session(session_id));
+
+  // dump profiler data
+  CheckApi(rocprofiler_flush_data(session_id, buffer_id));
+
+  // destroy session
+  CheckApi(rocprofiler_destroy_session(session_id));
+
+  // finalize profiler by destroying rocprofiler object
+  CheckApi(rocprofiler_finalize());
+}
+
 /*
  * ###################################################
  * ############ SPM Tests ################