[7.1] Fix rocPD data collection (#1439)

* [rocprofiler-systems] [ROCpd] Add OMPT callbacks to ROCpd (#1016)

* Add OMPT to ROCpd

* Use correct category

* Added wrapper functions for future control

* Formatting

* Fix naming

* Comment change

* Remove ompt_get_cb_args

* Switched to using region_sample for OMPT

* Remove relic function

* Remove get_use_rocpd that was used in this pr (one still remains)

* Rename ompt_get_args_string and reuse in tool_tracing_callback_stop

* Make lock init and destroy cb instant

* [Prototype] ROCPD Name fix

* [Prototype] ROCPD Name fix P1

* [Prototype] ROCPD Name fix P2

* ROCPD Name fix

* Var name changes

* Rewrite cb overwrite to single function

* [Important] Use parallel_data as key for parallel callback map

* Fix workflow failure

* Make cpp USE_ROCM consistent with hpp and use default constructor if USE_ROCM = 0

* Add missing ROCPROFILER_VERSION check

* Improve readability

* Make ompt storage maps thread local

* Part 1: Variable name fix, memory cleanup, and fixed asserts

* Part 2: Add comments

* Part 3: Add CI_THROW

* Part 4: Formatting

* Part 5: Move #include to cpp

* Add missing counter events handling for ROCPD (#1305)

* Add missing counter events handling for ROCPD

* Update projects/rocprofiler-systems/source/lib/rocprof-sys/library/rocprofiler-sdk/counters.cpp

* Update projects/rocprofiler-systems/source/lib/rocprof-sys/library/rocprofiler-sdk/counters.cpp

* Fixed formatting

Signed-off-by: David Galiffi <David.Galiffi@amd.com>

---------

Signed-off-by: Marjan Antic <Marjan.Antic@amd.com>
Co-authored-by: David Galiffi <David.Galiffi@amd.com>

* Update VERSION to 1.2.1

* Update CHANGELOG.md

* Add caching of category region for rocpd (#1420)

* Add caching of category region

Fix vaapi traces

Remove region_with_name

* Applied suggestions from code review

* Add some simple rocpd testing

Signed-off-by: David Galiffi <David.Galiffi@amd.com>

* Adjust `rocpd_string` validation parameters

* Adjust `rocm_marker_api` validation parameters

* Update projects/rocprofiler-systems/CHANGELOG.md

Co-authored-by: Pratik Basyal <pratik.basyal@amd.com>

* Update projects/rocprofiler-systems/CHANGELOG.md

Co-authored-by: Pratik Basyal <pratik.basyal@amd.com>

* Update projects/rocprofiler-systems/CHANGELOG.md

Co-authored-by: Pratik Basyal <pratik.basyal@amd.com>

---------

Signed-off-by: Marjan Antic <Marjan.Antic@amd.com>
Signed-off-by: David Galiffi <David.Galiffi@amd.com>
Co-authored-by: Kian Cossettini <Kian.Cossettini@amd.com>
Co-authored-by: marantic-amd <marantic@amd.com>
Co-authored-by: Milan Radosavljevic <milan.radosavljevic@amd.com>
Co-authored-by: Pratik Basyal <pratik.basyal@amd.com>

Author: 5 people

.pre-commit-config.yaml

@@ -30,9 +30,12 @@ repos:
   - repo: https://github.com/pre-commit/pre-commit-hooks
     rev: v5.0.0
     hooks:
+      - id: check-json # Check JSON files for syntax errors
       - id: check-yaml # Check YAML files for syntax errors
       - id: trailing-whitespace # Remove trailing whitespace
       - id: end-of-file-fixer # Fix files to have a newline at the end
+      - id: pretty-format-json # Pretty-format JSON files
+        args: ['--indent', '4', '--autofix']
 
   - repo: https://github.com/pre-commit/mirrors-clang-format
     rev: v18.1.8 # Version 18 as specified in contributor guide

CHANGELOG.md

@@ -4,6 +4,12 @@
 
 Full documentation for ROCm Systems Profiler is available at [https://rocm.docs.amd.com/projects/rocprofiler-systems/en/latest/](https://rocm.docs.amd.com/projects/rocprofiler-systems/en/latest/).
 
+## ROCm Systems Profiler 1.2.1 for ROCm 7.1.1
+
+### Resolved issues
+
+- Fixed an issue of OpenMP Tools (OMPT) events, GPU performance counters, VA-API, MPI, and host events failing to be collected in the `rocpd` output.
+
 ## ROCm Systems Profiler 1.2.0 for ROCm 7.1.0
 
 ### Added

VERSION

@@ -1 +1 @@
-1.2.0
+1.2.1

source/lib/core/trace_cache/buffer_storage.cpp

@@ -96,8 +96,6 @@ buffer_storage::buffer_storage(pid_t _pid)
             }
         };
 
-        ROCPROFSYS_DEBUG("Starting buffered storage flushing thread for pid %d",
-                         static_cast<int>(_pid));
         m_created_process = _pid;
         std::mutex _shutdown_condition_mutex;
         while(m_running)

source/lib/core/trace_cache/metadata_registry.cpp

@@ -21,6 +21,7 @@
 // SOFTWARE.
 
 #include "metadata_registry.hpp"
+#include "core/debug.hpp"
 #include <algorithm>
 #include <cstdint>
 
@@ -210,7 +211,7 @@ metadata_registry::get_string_list() const
     return result;
 }
 
-#if ROCPROFSYS_USE_ROCM
+#if ROCPROFSYS_USE_ROCM > 0
 
 void
 metadata_registry::add_code_object(
@@ -278,6 +279,102 @@ metadata_registry::get_kernel_symbol_list() const
     return result;
 }
 
+// As the underlying implementation of callback_name_info_t resizes the category storage
+// during emplace, this special method is required
+void
+metadata_registry::overwrite_callback_names(
+    std::initializer_list<
+        std::pair<rocprofiler_callback_tracing_kind_t, callback_rename_map_t>>
+        rename_table)
+{
+    if(rename_table.size() == 0) return;
+
+    using callback_kind_t   = rocprofiler_callback_tracing_kind_t;
+    using operation_names_t = std::vector<std::string_view>;
+
+    auto category_names = std::vector<std::string_view>{};
+    auto modified_ops   = std::map<callback_kind_t, operation_names_t>{};
+
+    auto extract_operations = [&](callback_kind_t cat) -> operation_names_t {
+        auto        items           = m_callback_tracing_info.items();
+        const auto* target_category = items[static_cast<size_t>(cat)];
+
+        auto              operations_data = target_category->items();
+        operation_names_t operation_names;
+        operation_names.reserve(operations_data.size());
+
+        for(const auto& [op_idx, op_name] : operations_data)
+            operation_names.push_back(*op_name);
+
+        return operation_names;
+    };
+
+    // Store category names
+    category_names.resize(ROCPROFILER_CALLBACK_TRACING_LAST);
+    for(callback_kind_t i = ROCPROFILER_CALLBACK_TRACING_NONE;
+        i < ROCPROFILER_CALLBACK_TRACING_LAST;
+        i = static_cast<callback_kind_t>(static_cast<int>(i) + 1))
+    {
+        category_names[i] = m_callback_tracing_info.at(i);
+    }
+
+    // Process list
+    for(const auto& category_info : rename_table)
+    {
+        auto callback_kind = category_info.first;
+        // Store operations of all following categories
+        //  as they will be deleted
+        for(callback_kind_t i =
+                static_cast<callback_kind_t>(static_cast<int>(callback_kind) + 1);
+            i < ROCPROFILER_CALLBACK_TRACING_LAST;
+            i = static_cast<callback_kind_t>(static_cast<int>(i) + 1))
+        {
+            if(modified_ops.find(i) != modified_ops.end()) break;
+            modified_ops[i] = extract_operations(i);
+        }
+
+        ROCPROFSYS_CI_THROW(modified_ops.find(callback_kind) != modified_ops.end(),
+                            "Overwriting a previously overwritten entry is forbidden");
+
+        ROCPROFSYS_CI_THROW(!modified_ops.empty() &&
+                                callback_kind >= modified_ops.begin()->first,
+                            "Category must have a larger enum value than all previously "
+                            "modified_ops categories");
+
+        // Overwrite desired category
+        auto operation_names = extract_operations(callback_kind);
+        for(const auto& [index, new_value] : category_info.second)
+        {
+            ROCPROFSYS_CI_THROW(index < 0 ||
+                                    static_cast<size_t>(index) >= operation_names.size(),
+                                "Index is invalid");
+            operation_names[index] = new_value;
+        }
+        modified_ops[callback_kind] = std::move(operation_names);
+    }
+    if(modified_ops.empty()) return;
+
+    // Emplace the changed category operations
+    for(callback_kind_t i = modified_ops.begin()->first;
+        i < ROCPROFILER_CALLBACK_TRACING_LAST;
+        i = static_cast<callback_kind_t>(static_cast<int>(i) + 1))
+    {
+        auto renaming_entry = modified_ops.find(i);
+
+        ROCPROFSYS_CI_THROW(renaming_entry == modified_ops.end(),
+                            "A category that needs to be emplaced is missing");
+
+        const auto& operations_vec = renaming_entry->second;
+        m_callback_tracing_info.emplace(i, category_names.at(i).data());
+        for(size_t op_idx = 0; op_idx < operations_vec.size(); ++op_idx)
+        {
+            m_callback_tracing_info.emplace(
+                i, static_cast<rocprofiler_tracing_operation_t>(op_idx),
+                operations_vec[op_idx].data());
+        }
+    }
+}
+
 rocprofiler::sdk::buffer_name_info_t<const char*>
 metadata_registry::get_buffer_name_info() const
 {
@@ -292,5 +389,20 @@ metadata_registry::get_callback_tracing_info() const
 
 #endif
 
+metadata_registry::metadata_registry()
+{
+#if ROCPROFSYS_USE_ROCM > 0
+    overwrite_callback_names({
+#    if(ROCPROFILER_VERSION >= 600)
+        { ROCPROFILER_CALLBACK_TRACING_OMPT,
+          { { ROCPROFILER_OMPT_ID_thread_begin, "omp_thread" },
+            { ROCPROFILER_OMPT_ID_thread_end, "omp_thread" },
+            { ROCPROFILER_OMPT_ID_parallel_begin, "omp_parallel" },
+            { ROCPROFILER_OMPT_ID_parallel_end, "omp_parallel" } } }
+#    endif
+    });
+#endif
+}
+
 }  // namespace trace_cache
 }  // namespace rocprofsys

source/lib/core/trace_cache/metadata_registry.hpp

@@ -35,6 +35,8 @@
 #    include <rocprofiler-sdk/callback_tracing.h>
 #    include <rocprofiler-sdk/cxx/name_info.hpp>
 #endif
+#include <initializer_list>
+#include <map>
 #include <set>
 #include <sstream>
 #include <stdint.h>
@@ -215,7 +217,7 @@ struct metadata_registry
 
 private:
     friend class cache_manager;
-    metadata_registry() = default;
+    metadata_registry();
     common::synchronized<info::process> m_process;
     common::synchronized<
         std::unordered_set<info::pmc, info::pmc_info_hash, info::pmc_info_equal>>
@@ -240,6 +242,14 @@ struct metadata_registry
     rocprofiler::sdk::callback_name_info_t<const char*> m_callback_tracing_info{
         rocprofiler::sdk::get_callback_tracing_names<const char*>()
     };
+
+    using callback_rename_map_t =
+        std::map<rocprofiler_tracing_operation_t, std::string_view>;
+
+    void overwrite_callback_names(
+        std::initializer_list<
+            std::pair<rocprofiler_callback_tracing_kind_t, callback_rename_map_t>>
+            rename_table);
 #endif
 };
 

source/lib/core/trace_cache/rocpd_post_processing.cpp

@@ -279,6 +279,11 @@ rocpd_post_processing::get_region_callback() const
             return tokens;
         };
 
+        if(arg_str.empty())
+        {
+            return args;
+        }
+
         auto tokens = split(arg_str, delimiter);
 
         // Ensure the number of tokens is a multiple of 4
@@ -307,12 +312,7 @@ rocpd_post_processing::get_region_callback() const
         auto  thread_primary_key =
             data_processor.map_thread_id_to_primary_key(_rs.thread_id);
 
-        auto callback_tracing_info = m_metadata.get_callback_tracing_info();
-        auto _name                 = std::string{ callback_tracing_info.at(
-            static_cast<rocprofiler_callback_tracing_kind_t>(_rs.kind),
-            static_cast<rocprofiler_tracing_operation_t>(_rs.operation)) };
-        auto name_primary_key      = data_processor.insert_string(_name.c_str());
-
+        auto name_primary_key     = data_processor.insert_string(_rs.name.c_str());
         auto category_primary_key = data_processor.insert_string(_rs.category.c_str());
 
         size_t stack_id        = _rs.correlation_id_internal;

source/lib/core/trace_cache/sample_type.hpp

@@ -131,13 +131,12 @@ struct memory_allocate_sample : storage_parsed_type_base
 struct region_sample : storage_parsed_type_base
 {
     region_sample() = default;
-    region_sample(uint64_t _thread_id, int32_t _kind, int32_t _operation,
+    region_sample(uint64_t _thread_id, std::string _name,
                   uint64_t _correlation_id_internal, uint64_t _correlation_id_ancestor,
                   uint64_t _start_timestamp, uint64_t _end_timestamp,
                   std::string _call_stack, std::string _args_str, std::string _category)
     : thread_id(_thread_id)
-    , kind(_kind)
-    , operation(_operation)
+    , name(std::move(_name))
     , correlation_id_internal(_correlation_id_internal)
     , correlation_id_ancestor(_correlation_id_ancestor)
     , start_timestamp(_start_timestamp)
@@ -147,10 +146,8 @@ struct region_sample : storage_parsed_type_base
     , category(std::move(_category))
     {}
 
-    // Identification fields
-    uint64_t thread_id;
-    int32_t  kind;
-    int32_t  operation;
+    uint64_t    thread_id;
+    std::string name;
 
     // Correlation fields
     uint64_t correlation_id_internal;

source/lib/core/trace_cache/storage_parser.cpp

@@ -164,8 +164,7 @@ storage_parser::consume_storage()
             case entry_type::region:
             {
                 region_sample _region_sample;
-                parse_data(sample.data(), _region_sample.thread_id, _region_sample.kind,
-                           _region_sample.operation,
+                parse_data(sample.data(), _region_sample.thread_id, _region_sample.name,
                            _region_sample.correlation_id_internal,
                            _region_sample.correlation_id_ancestor,
                            _region_sample.start_timestamp, _region_sample.end_timestamp,

source/lib/rocprof-sys/library/components/category_region.hpp

@@ -26,18 +26,99 @@
 #include "core/defines.hpp"
 #include "core/state.hpp"
 #include "core/timemory.hpp"
+#include "core/trace_cache/cache_manager.hpp"
 #include "library/causal/data.hpp"
 #include "library/runtime.hpp"
+#include "library/thread_info.hpp"
 #include "library/tracing.hpp"
 #include "library/tracing/annotation.hpp"
 
+#include <map>
+#include <thread>
 #include <timemory/components/gotcha/backends.hpp>
 #include <timemory/hash/types.hpp>
 #include <timemory/mpl/concepts.hpp>
 #include <timemory/mpl/types.hpp>
 #include <timemory/utility/types.hpp>
 
 #include <string_view>
+#include <utility>
+
+namespace
+{
+
+void
+cache_region(uint64_t thread_id, const std::string& name, uint64_t start_ts,
+             uint64_t end_ts, const std::string& category)
+{
+    constexpr size_t      NO_CORRELATION_ID = 0;
+    constexpr const char* CALLSTACK         = "";
+    constexpr const char* ARGUMENTS         = "";
+    rocprofsys::trace_cache::get_buffer_storage().store(
+        rocprofsys::trace_cache::entry_type::region, thread_id, name.c_str(),
+        NO_CORRELATION_ID, NO_CORRELATION_ID, start_ts, end_ts, CALLSTACK, ARGUMENTS,
+        category.c_str());
+}
+
+struct entry_key
+{
+    std::string name;
+    std::string category;
+
+    friend bool operator<(const entry_key& lhs, const entry_key& rhs)
+    {
+        if(lhs.name != rhs.name)
+        {
+            return lhs.name < rhs.name;
+        }
+
+        return lhs.category < rhs.category;
+    }
+};
+
+using timestamp_t = uint64_t;
+
+thread_local std::map<entry_key, timestamp_t> map_name_to_args;
+
+template <typename CategoryT, typename... Args>
+void
+cache_start(const char* name)
+{
+    const auto start_ts =
+        static_cast<timestamp_t>(rocprofsys::comp::wall_clock::record());
+    map_name_to_args[{ name, rocprofsys::trait::name<CategoryT>::value }] = start_ts;
+}
+
+template <typename CategoryT>
+void
+cache_stop(const char* name)
+{
+    entry_key key{ name, rocprofsys::trait::name<CategoryT>::value };
+    auto      x = map_name_to_args.find(key);
+    if(x != map_name_to_args.end())
+    {
+        map_name_to_args.erase(key);
+        auto timestamp = x->second;
+
+        const auto end_ts =
+            static_cast<timestamp_t>(rocprofsys::comp::wall_clock::record());
+        uint64_t thread_id = 0;
+
+        const auto& extended_info =
+            rocprofsys::thread_info::get(std::this_thread::get_id());
+        if(extended_info.has_value() && extended_info->index_data.has_value())
+        {
+            constexpr size_t UNKNOWN_TIME = 0;
+            thread_id                     = extended_info->index_data->system_value;
+            rocprofsys::trace_cache::get_metadata_registry().add_thread_info(
+                { getppid(), getpid(), thread_id, UNKNOWN_TIME, UNKNOWN_TIME, "{}" });
+        }
+
+        cache_region(thread_id, name, timestamp, end_ts,
+                     rocprofsys::trait::name<CategoryT>::value);
+    }
+}
+}  // namespace
 
 namespace tim
 {
@@ -192,6 +273,8 @@ category_region<CategoryT>::start(std::string_view name, Args&&... args)
             tracing::push_perfetto(CategoryT{}, name.data(), std::forward<Args>(args)...);
         }
     }
+
+    cache_start<CategoryT>(name.data());
 }
 
 template <typename CategoryT>
@@ -257,6 +340,8 @@ category_region<CategoryT>::stop(std::string_view name, Args&&... args)
                 if(get_use_causal()) causal::pop_progress_point(name);
             }
         }
+
+        cache_stop<CategoryT>(name.data());
     }
     else
     {