From 007285272be3fe54ba7674f61d695d6f2f84adba Mon Sep 17 00:00:00 2001 From: "Welton, Benjamin" Date: Fri, 14 Mar 2025 01:07:16 -0700 Subject: [PATCH] [SWDEV-518071] Return HSA not loaded status (device counter collection) (#242) * [SWDEV-518071] Return HSA not loaded status (device counter collection) This is a state that a caller would want to know about to understand if they got no counters because of a failure or if they were trying to collect counters too early (as is the case in the sample, which can attempt to collect counters before HSA is inited). * Minor edit * format * [SWDEV-518081] Simplify Metric Loading (#243) * [SWDEV-518071] Return HSA not loaded status (device counter collection) This is a state that a caller would want to know about to understand if they got no counters because of a failure or if they were trying to collect counters too early (as is the case in the sample, which can attempt to collect counters before HSA is inited). * [SWDEV-518324] Add AST update support Allows the ability for ASTs to be updated (instead of an unchangable static value). Adds a shared pointer return type to protect against static destructors/modifications from invalidating potentially in use AST definitions. No functionality/use changes in this PR. * [SWDEV-518593] Add updatable dimension cache + fix string issues (#252) * [SWDEV-518593] Add updatable dimension cache + fix string issues Updates dimension cache to use the same design pattern as AST/Metrics. Fixes the string scoping issue seen in ASTs, which appears here as well. * Add rocprofiler_create_counter Creates derived counters based on input from the API. This PR does three things: 1. Adds the API + test case 2. Validates that an AST can be constructed from the counter supplied. 3. Updates metrics, ast, and dimension caches to include the new metric. Metric should be available for use immediately after the call completes. Due to the regeneration of ASTs, this call should not be performed in performance sensitive code. * Suggestion fixes --------- Co-authored-by: Benjamin Welton * Minor tweak --------- Co-authored-by: Benjamin Welton Co-authored-by: Venkateshwar Reddy Kandula --------- Co-authored-by: Benjamin Welton Co-authored-by: Venkateshwar Reddy Kandula * Fixes for comments --------- Co-authored-by: Benjamin Welton Co-authored-by: Kandula, Venkateshwar reddy Co-authored-by: Venkateshwar Reddy Kandula --------- Co-authored-by: Benjamin Welton Co-authored-by: Kandula, Venkateshwar reddy Co-authored-by: Venkateshwar Reddy Kandula --- CHANGELOG.md | 1 + .../device_counting_synchronous.cpp | 62 +++-- samples/counter_collection/main.cpp | 2 +- source/include/rocprofiler-sdk/counters.h | 32 +++ .../rocprofiler-sdk/aql/tests/aql_test.cpp | 4 +- .../lib/rocprofiler-sdk/aql/tests/helpers.cpp | 4 +- source/lib/rocprofiler-sdk/counters.cpp | 96 +++++-- .../rocprofiler-sdk/counters/controller.cpp | 3 +- source/lib/rocprofiler-sdk/counters/core.cpp | 10 +- .../counters/device_counting.cpp | 6 +- .../rocprofiler-sdk/counters/dimensions.cpp | 75 ++++-- .../rocprofiler-sdk/counters/dimensions.hpp | 9 +- .../rocprofiler-sdk/counters/evaluate_ast.cpp | 185 ++++++++++---- .../rocprofiler-sdk/counters/evaluate_ast.hpp | 12 +- .../lib/rocprofiler-sdk/counters/metrics.cpp | 239 ++++++++++-------- .../lib/rocprofiler-sdk/counters/metrics.hpp | 45 ++-- .../counters/parser/tests/parser_test.cpp | 8 +- .../counters/tests/CMakeLists.txt | 4 +- .../rocprofiler-sdk/counters/tests/core.cpp | 60 ++++- .../counters/tests/device_counting.cpp | 10 +- .../counters/tests/dimension.cpp | 15 +- .../counters/tests/evaluate_ast_test.cpp | 6 +- .../counters/tests/init_order.cpp | 178 ------------- .../counters/tests/metrics_test.cpp | 54 +++- source/lib/rocprofiler-sdk/profile_config.cpp | 4 +- 25 files changed, 633 insertions(+), 491 deletions(-) delete mode 100644 source/lib/rocprofiler-sdk/counters/tests/init_order.cpp diff --git a/CHANGELOG.md b/CHANGELOG.md index 742c511db6..4847c3591b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -168,6 +168,7 @@ Full documentation for ROCprofiler-SDK is available at [rocm.docs.amd.com/projec ### Added - Added support for rocJPEG API Tracing +- Added rocprofiler_create_counter to allow for adding custom derived counters at runtime. ### Changed diff --git a/samples/counter_collection/device_counting_synchronous.cpp b/samples/counter_collection/device_counting_synchronous.cpp index 4e90b68ce1..59ce8dfabf 100644 --- a/samples/counter_collection/device_counting_synchronous.cpp +++ b/samples/counter_collection/device_counting_synchronous.cpp @@ -78,7 +78,7 @@ public: counter_sampler(rocprofiler_agent_id_t agent); // Decode the counter name of a record - const std::string& decode_record_name(const rocprofiler_record_counter_t& rec) const; + std::string decode_record_name(const rocprofiler_record_counter_t& rec) const; // Get the dimensions of a record (what CU/SE/etc the counter is for). High cost operation // should be cached if possible. @@ -86,8 +86,8 @@ public: const rocprofiler_record_counter_t& rec); // Sample the counter values for a set of counters, returns the records in the out parameter. - void sample_counter_values(const std::vector& counters, - std::vector& out); + rocprofiler_status_t sample_counter_values(const std::vector& counters, + std::vector& out); // Get the available agents on the system static std::vector get_available_agents(); @@ -165,7 +165,7 @@ counter_sampler::counter_sampler(rocprofiler_agent_id_t agent) "Could not setup buffered service"); } -const std::string& +std::string counter_sampler::decode_record_name(const rocprofiler_record_counter_t& rec) const { if(id_to_name_.empty()) @@ -179,6 +179,11 @@ counter_sampler::decode_record_name(const rocprofiler_record_counter_t& rec) con rocprofiler_counter_id_t counter_id = {.handle = 0}; rocprofiler_query_record_counter_id(rec.id, &counter_id); + if(id_to_name_.find(counter_id.handle) == id_to_name_.end()) + { + std::clog << "Unknown counter id = " << counter_id.handle << "\n"; + return "UNKNOWN"; + } return id_to_name_.at(counter_id.handle); } @@ -199,7 +204,7 @@ counter_sampler::get_record_dimensions(const rocprofiler_record_counter_t& rec) return out; } -void +rocprofiler_status_t counter_sampler::sample_counter_values(const std::vector& counters, std::vector& out) { @@ -228,15 +233,23 @@ counter_sampler::sample_counter_values(const std::vector& profile_sizes_.emplace(profile.handle, expected_size); profile_cached = cached_profiles_.find(counters); } - - out.resize(profile_sizes_.at(profile_cached->second.handle)); + try + { + out.resize(profile_sizes_.at(profile_cached->second.handle)); + } catch(const std::exception& e) + { + std::cerr << "Caught exception: " << e.what() << "\n"; + return ROCPROFILER_STATUS_ERROR; + } profile_ = profile_cached->second; rocprofiler_start_context(ctx_); + std::this_thread::sleep_for(std::chrono::milliseconds(50)); size_t out_size = out.size(); - rocprofiler_sample_device_counting_service( + auto status = rocprofiler_sample_device_counting_service( ctx_, {}, ROCPROFILER_COUNTER_FLAG_NONE, out.data(), &out_size); rocprofiler_stop_context(ctx_); out.resize(out_size); + return status; } std::vector @@ -392,22 +405,31 @@ tool_init(rocprofiler_client_finalize_t fini_func, void*) std::vector records; while(sampler && exit_toggle().load() == false) { - sampler->sample_counter_values({"SQ_WAVES"}, records); - std::clog << "Sample " << count << ":\n"; - for(const auto& record : records) + auto status = sampler->sample_counter_values({"SQ_WAVES"}, records); + if(status == ROCPROFILER_STATUS_ERROR_HSA_NOT_LOADED) { - if(!sampler) break; - auto recname = sampler->decode_record_name(record); - std::clog << "\tCounter: " << record.id << " Name: " << recname - << " Value: " << record.counter_value - << " User data: " << record.user_data.value << "\n"; - if(count == 1) + std::clog << "HSA not loaded yet....\n"; + std::this_thread::sleep_for(std::chrono::milliseconds(50)); + continue; + } + std::clog << "Sample " << count << ":\n"; + if(status == ROCPROFILER_STATUS_SUCCESS) + { + for(const auto& record : records) { if(!sampler) break; - auto dims = sampler->get_record_dimensions(record); - for(const auto& [name, pos] : dims) + auto recname = sampler->decode_record_name(record); + std::clog << "\tCounter: " << record.id << " Name: " << recname + << " Value: " << record.counter_value + << " User data: " << record.user_data.value << "\n"; + if(count == 1) { - std::clog << "\t\tDimension Name: " << name << ": " << pos << "\n"; + if(!sampler) break; + auto dims = sampler->get_record_dimensions(record); + for(const auto& [name, pos] : dims) + { + std::clog << "\t\tDimension Name: " << name << ": " << pos << "\n"; + } } } } diff --git a/samples/counter_collection/main.cpp b/samples/counter_collection/main.cpp index 03b0f92354..bdd4795a38 100644 --- a/samples/counter_collection/main.cpp +++ b/samples/counter_collection/main.cpp @@ -116,7 +116,7 @@ main(int argc, char** argv) int ntotdevice = 0; HIP_CALL(hipGetDeviceCount(&ntotdevice)); - long nitr = 5000; + long nitr = 50000; long nsync = 500; long ndevice = 0; diff --git a/source/include/rocprofiler-sdk/counters.h b/source/include/rocprofiler-sdk/counters.h index 7b2f6c092f..ce6b68f7fa 100644 --- a/source/include/rocprofiler-sdk/counters.h +++ b/source/include/rocprofiler-sdk/counters.h @@ -164,4 +164,36 @@ rocprofiler_iterate_agent_supported_counters(rocprofiler_agent_id_t /** @} */ +/** + * @brief Creates a new counter based on a derived metric provided. The counter will only + * be available for counter collection profiles created after the addition of this counter. + * Due to the regeneration of internal ASTs and dimension cache, this call may be slow and + * should generally be avoided in performance sensitive code blocks (i.e. dispatch + * callbacks). + * + * @param [in] name The name of the new counter. + * @param [in] name_len The length of the counter name. + * @param [in] expr The counter expression, formatted identically to YAML counter definitions. + * @param [in] expr_len The length of the expression. + * @param [in] agent The rocprofiler_agent_id_t specifying the agent for which to create the + * counter. + * @param [in] description The description of the new counter (optional). + * @param [in] description_len The length of the description. + * @param [out] counter_id The rocprofiler_counter_id_t of the created counter. + * @return ::rocprofiler_status_t + * @retval ROCPROFILER_STATUS_SUCCESS if the counter was successfully created. + * @retval ROCPROFILER_STATUS_ERROR_AST_GENERATION_FAILED if the counter could not be created. + * @retval ROCPROFILER_STATUS_ERROR_INVALID_ARGUMENT if a counter argument is incorrect + * @retval ROCPROFILER_STATUS_ERROR_AGENT_NOT_FOUND if the agent is not found + */ +rocprofiler_status_t +rocprofiler_create_counter(const char* name, + size_t name_len, + const char* expr, + size_t expr_len, + const char* description, + size_t description_len, + rocprofiler_agent_id_t agent, + rocprofiler_counter_id_t* counter_id) ROCPROFILER_NONNULL(1, 3, 8); + ROCPROFILER_EXTERN_C_FINI diff --git a/source/lib/rocprofiler-sdk/aql/tests/aql_test.cpp b/source/lib/rocprofiler-sdk/aql/tests/aql_test.cpp index 287935b9ce..8d8d7068ce 100644 --- a/source/lib/rocprofiler-sdk/aql/tests/aql_test.cpp +++ b/source/lib/rocprofiler-sdk/aql/tests/aql_test.cpp @@ -75,10 +75,10 @@ auto findDeviceMetrics(const hsa::AgentCache& agent, const std::unordered_set& metrics) { std::vector ret; - auto all_counters = counters::getBaseHardwareMetrics(); + auto all_counters = counters::loadMetrics()->arch_to_metric; ROCP_INFO << "Looking up counters for " << std::string(agent.name()); - auto gfx_metrics = common::get_val(all_counters, std::string(agent.name())); + auto* gfx_metrics = common::get_val(all_counters, std::string(agent.name())); if(!gfx_metrics) { ROCP_ERROR << "No counters found for " << std::string(agent.name()); diff --git a/source/lib/rocprofiler-sdk/aql/tests/helpers.cpp b/source/lib/rocprofiler-sdk/aql/tests/helpers.cpp index c0de634a0c..6c70cbbf23 100644 --- a/source/lib/rocprofiler-sdk/aql/tests/helpers.cpp +++ b/source/lib/rocprofiler-sdk/aql/tests/helpers.cpp @@ -44,11 +44,11 @@ auto findDeviceMetrics(const rocprofiler_agent_t& agent, const std::unordered_set& metrics) { std::vector ret; - auto all_counters = counters::getBaseHardwareMetrics(); + auto all_counters = counters::loadMetrics()->arch_to_metric; ROCP_INFO << "Looking up counters for " << std::string(agent.name); - auto gfx_metrics = common::get_val(all_counters, std::string(agent.name)); + auto* gfx_metrics = common::get_val(all_counters, std::string(agent.name)); if(!gfx_metrics) { ROCP_ERROR << "No counters found for " << std::string(agent.name); diff --git a/source/lib/rocprofiler-sdk/counters.cpp b/source/lib/rocprofiler-sdk/counters.cpp index 0b1af1d2c9..9b63ab6f76 100644 --- a/source/lib/rocprofiler-sdk/counters.cpp +++ b/source/lib/rocprofiler-sdk/counters.cpp @@ -28,17 +28,23 @@ #include "lib/common/container/small_vector.hpp" #include "lib/common/logging.hpp" -#include "lib/common/static_object.hpp" -#include "lib/common/synchronized.hpp" +#include "lib/common/string_entry.hpp" +#include "lib/common/utility.hpp" #include "lib/rocprofiler-sdk/agent.hpp" -#include "lib/rocprofiler-sdk/aql/helpers.hpp" #include "lib/rocprofiler-sdk/counters/dimensions.hpp" #include "lib/rocprofiler-sdk/counters/evaluate_ast.hpp" #include "lib/rocprofiler-sdk/counters/id_decode.hpp" #include "lib/rocprofiler-sdk/counters/metrics.hpp" -#include "lib/rocprofiler-sdk/hsa/agent_cache.hpp" -#include "lib/rocprofiler-sdk/hsa/queue.hpp" -#include "lib/rocprofiler-sdk/hsa/queue_controller.hpp" + +namespace +{ +const char* +get_static_string(const std::string& str) +{ + return rocprofiler::common::get_string_entry(rocprofiler::common::add_string_entry(str)) + ->c_str(); +} +} // namespace extern "C" { /** @@ -60,19 +66,21 @@ rocprofiler_query_counter_info(rocprofiler_counter_id_t counter_id, { if(version != ROCPROFILER_COUNTER_INFO_VERSION_0) return ROCPROFILER_STATUS_ERROR_INCOMPATIBLE_ABI; - const auto& id_map = *CHECK_NOTNULL(rocprofiler::counters::getMetricIdMap()); + auto metrics_map = rocprofiler::counters::loadMetrics(); + + const auto& id_map = metrics_map->id_to_metric; auto& out_struct = *static_cast(info); if(const auto* metric_ptr = rocprofiler::common::get_val(id_map, counter_id.handle)) { out_struct.id = counter_id; - out_struct.is_constant = (metric_ptr->special().empty()) ? 0 : 1; + out_struct.is_constant = (metric_ptr->constant().empty()) ? 0 : 1; out_struct.is_derived = (metric_ptr->expression().empty()) ? 0 : 1; - out_struct.name = metric_ptr->name().c_str(); - out_struct.description = metric_ptr->description().c_str(); - out_struct.block = metric_ptr->block().c_str(); - out_struct.expression = metric_ptr->expression().c_str(); + out_struct.name = get_static_string(metric_ptr->name()); + out_struct.description = get_static_string(metric_ptr->description()); + out_struct.block = get_static_string(metric_ptr->block()); + out_struct.expression = get_static_string(metric_ptr->expression()); return ROCPROFILER_STATUS_SUCCESS; } @@ -94,9 +102,9 @@ rocprofiler_query_counter_instance_count(rocprofiler_agent_id_t, size_t* instance_count) { *instance_count = 0; + auto dim_ptr = rocprofiler::counters::get_dimension_cache(); - const auto* dims = rocprofiler::common::get_val(rocprofiler::counters::get_dimension_cache(), - counter_id.handle); + const auto* dims = rocprofiler::common::get_val(dim_ptr->id_to_dim, counter_id.handle); if(!dims) return ROCPROFILER_STATUS_ERROR_COUNTER_NOT_FOUND; for(const auto& metric_dim : *dims) @@ -169,8 +177,9 @@ rocprofiler_iterate_counter_dimensions(rocprofiler_counter_id_t id, rocprofiler_available_dimensions_cb_t info_cb, void* user_data) { - const auto* dims = - rocprofiler::common::get_val(rocprofiler::counters::get_dimension_cache(), id.handle); + auto dim_ptr = rocprofiler::counters::get_dimension_cache(); + + const auto* dims = rocprofiler::common::get_val(dim_ptr->id_to_dim, id.handle); if(!dims) return ROCPROFILER_STATUS_ERROR_COUNTER_NOT_FOUND; // This is likely faster than a map lookup given the limited number of dims. @@ -178,7 +187,7 @@ rocprofiler_iterate_counter_dimensions(rocprofiler_counter_id_t id, for(const auto& internal_dim : *dims) { auto& dim = user_dims.emplace_back(); - dim.name = internal_dim.name().c_str(); + dim.name = get_static_string(internal_dim.name()); dim.instance_size = internal_dim.size(); dim.id = static_cast(internal_dim.type()); } @@ -203,4 +212,57 @@ rocprofiler_load_counter_definition(const char* yaml, size_t size, rocprofiler_c def.loaded = false; return rocprofiler::counters::setCustomCounterDefinition(def); } + +rocprofiler_status_t +rocprofiler_create_counter(const char* name, + size_t name_len, + const char* expr, + size_t expr_len, + const char* description, + size_t description_len, + rocprofiler_agent_id_t agent, + rocprofiler_counter_id_t* counter_id) +{ + const auto* agent_ptr = rocprofiler::agent::get_agent(agent); + if(!agent_ptr) return ROCPROFILER_STATUS_ERROR_AGENT_NOT_FOUND; + + rocprofiler::counters::Metric new_metric( + "", + std::string(name, name_len), + "", + "", + std::string((description ? description : ""), description_len), + std::string(expr, expr_len), + "", + -1); + + // Validate the metric. Checks for duplicate names and invalid expressions. + if(auto status = rocprofiler::counters::check_ast_generation(agent_ptr->name, new_metric); + status != ROCPROFILER_STATUS_SUCCESS) + { + return status; + } + + auto add_metric = + rocprofiler::counters::loadMetrics(true, std::make_pair(agent_ptr->name, new_metric)); + + if(add_metric->arch_to_metric.at(agent_ptr->name).back().name() != new_metric.name()) + { + ROCP_ERROR << fmt::format("Custom metric {} was not added", new_metric.name()); + return ROCPROFILER_STATUS_ERROR_INVALID_ARGUMENT; + } + + counter_id->handle = add_metric->arch_to_metric.at(agent_ptr->name).back().id(); + // Regenerate ASTs and Dimension Cache + try + { + rocprofiler::counters::get_ast_map(true); + rocprofiler::counters::get_dimension_cache(true); + } catch(std::exception& e) + { + ROCP_FATAL << "Could not regenerate ASTs and Dimension Cache " << e.what(); + } + + return ROCPROFILER_STATUS_SUCCESS; +} } diff --git a/source/lib/rocprofiler-sdk/counters/controller.cpp b/source/lib/rocprofiler-sdk/counters/controller.cpp index 546107839d..30445cfb93 100644 --- a/source/lib/rocprofiler-sdk/counters/controller.cpp +++ b/source/lib/rocprofiler-sdk/counters/controller.cpp @@ -30,6 +30,7 @@ #include "lib/rocprofiler-sdk/buffer.hpp" #include "lib/rocprofiler-sdk/context/context.hpp" #include "lib/rocprofiler-sdk/counters/ioctl.hpp" +#include "lib/rocprofiler-sdk/counters/metrics.hpp" namespace rocprofiler { @@ -38,7 +39,7 @@ namespace counters CounterController::CounterController() { // Pre-read metrics map file to catch faliures during initial setup. - rocprofiler::counters::getMetricIdMap(); + rocprofiler::counters::loadMetrics(); } // Adds a counter collection profile to our global cache. diff --git a/source/lib/rocprofiler-sdk/counters/core.cpp b/source/lib/rocprofiler-sdk/counters/core.cpp index 6f3d082d93..5bb00cebb1 100644 --- a/source/lib/rocprofiler-sdk/counters/core.cpp +++ b/source/lib/rocprofiler-sdk/counters/core.cpp @@ -55,7 +55,9 @@ counter_callback_info::setup_profile_config(std::shared_ptr& pro auto agent_name = std::string(config.agent->name); for(const auto& metric : config.metrics) { - auto req_counters = get_required_hardware_counters(get_ast_map(), agent_name, metric); + const auto asts = get_ast_map(); + auto req_counters = + get_required_hardware_counters(asts->arch_to_counter_asts, agent_name, metric); if(!req_counters) { @@ -67,7 +69,7 @@ counter_callback_info::setup_profile_config(std::shared_ptr& pro // constants like MAX_WAVE_SIZE for(const auto& req_metric : *req_counters) { - if(req_metric.special().empty()) + if(req_metric.constant().empty()) { config.reqired_hw_counters.insert(req_metric); } @@ -77,8 +79,8 @@ counter_callback_info::setup_profile_config(std::shared_ptr& pro } } - const auto& asts = get_ast_map(); - const auto* agent_map = rocprofiler::common::get_val(asts, agent_name); + const auto* agent_map = + rocprofiler::common::get_val(asts->arch_to_counter_asts, agent_name); if(!agent_map) { ROCP_ERROR << fmt::format("Coult not build AST for {}", agent_name); diff --git a/source/lib/rocprofiler-sdk/counters/device_counting.cpp b/source/lib/rocprofiler-sdk/counters/device_counting.cpp index 93db2f494a..3b36657868 100644 --- a/source/lib/rocprofiler-sdk/counters/device_counting.cpp +++ b/source/lib/rocprofiler-sdk/counters/device_counting.cpp @@ -280,7 +280,7 @@ read_agent_ctx(const context::context* ctx, // If we have not initiualized HSA yet, nothing to read, return; if(hsa_inited().load() == false) { - return ROCPROFILER_STATUS_ERROR; + return ROCPROFILER_STATUS_ERROR_HSA_NOT_LOADED; } // Set the state to LOCKED to prevent other calls to start/stop/read. @@ -385,7 +385,7 @@ start_agent_ctx(const context::context* ctx) if(hsa_inited().load() == false) { - return ROCPROFILER_STATUS_SUCCESS; + return ROCPROFILER_STATUS_ERROR_HSA_NOT_LOADED; } // Set the state to LOCKED to prevent other calls to start/stop/read. @@ -518,7 +518,7 @@ stop_agent_ctx(const context::context* ctx) if(hsa_inited().load() == false) { - return ROCPROFILER_STATUS_SUCCESS; + return ROCPROFILER_STATUS_ERROR_HSA_NOT_LOADED; } auto expected = rocprofiler::context::device_counting_service::state::ENABLED; diff --git a/source/lib/rocprofiler-sdk/counters/dimensions.cpp b/source/lib/rocprofiler-sdk/counters/dimensions.cpp index 36d7e51b35..7de9ed28a3 100644 --- a/source/lib/rocprofiler-sdk/counters/dimensions.cpp +++ b/source/lib/rocprofiler-sdk/counters/dimensions.cpp @@ -44,7 +44,7 @@ namespace counters std::vector getBlockDimensions(std::string_view agent, const Metric& metric) { - if(!metric.special().empty()) + if(!metric.constant().empty()) { // Special non-hardware counters without dimension data return std::vector{{dimension_map().at(ROCPROFILER_DIMENSION_INSTANCE), @@ -95,34 +95,55 @@ getBlockDimensions(std::string_view agent, const Metric& metric) return ret; } -const std::unordered_map>& -get_dimension_cache() +namespace { - static auto*& cache = - common::static_object>>:: - construct([]() -> std::unordered_map> { - std::unordered_map> dims; +metric_dims +generate_dimensions() +{ + std::unordered_map> dims; - const auto& asts = counters::get_ast_map(); - for(const auto& [gfx, metrics] : asts) - { - for(const auto& [metric, ast] : metrics) - { - auto ast_copy = ast; - try - { - dims.emplace(ast.out_id().handle, ast_copy.set_dimensions()); - } catch(std::runtime_error& e) - { - ROCP_ERROR << metric << " has improper dimensions" - << " " << e.what(); - throw; - } - } - } - return dims; - }()); - return *cache; + const auto asts = counters::get_ast_map(); + for(const auto& [gfx, metrics] : asts->arch_to_counter_asts) + { + for(const auto& [metric, ast] : metrics) + { + auto ast_copy = ast; + try + { + dims.emplace(ast.out_id().handle, ast_copy.set_dimensions()); + } catch(std::runtime_error& e) + { + ROCP_FATAL << metric << " has improper dimensions" + << " " << e.what(); + } + } + } + return {.id_to_dim = dims}; +} +} // namespace + +std::shared_ptr +get_dimension_cache(bool reload) +{ + using DimSync = common::Synchronized>; + static DimSync*& dim_data = common::static_object::construct( + [&]() { return std::make_shared(generate_dimensions()); }()); + + if(!dim_data) return nullptr; + + if(!reload) + { + return dim_data->rlock([](const auto& data) { + CHECK(data); + return data; + }); + } + + return dim_data->wlock([&](auto& data) { + data = std::make_shared(generate_dimensions()); + CHECK(data); + return data; + }); } } // namespace counters diff --git a/source/lib/rocprofiler-sdk/counters/dimensions.hpp b/source/lib/rocprofiler-sdk/counters/dimensions.hpp index c53b208e39..9932eee6b1 100644 --- a/source/lib/rocprofiler-sdk/counters/dimensions.hpp +++ b/source/lib/rocprofiler-sdk/counters/dimensions.hpp @@ -58,12 +58,17 @@ private: rocprofiler_profile_counter_instance_types type_; }; +struct metric_dims +{ + const std::unordered_map> id_to_dim; +}; + // get all dimensions for an agent, block_name std::vector getBlockDimensions(std::string_view agent, const counters::Metric&); -const std::unordered_map>& -get_dimension_cache(); +std::shared_ptr +get_dimension_cache(bool reload = false); } // namespace counters } // namespace rocprofiler diff --git a/source/lib/rocprofiler-sdk/counters/evaluate_ast.cpp b/source/lib/rocprofiler-sdk/counters/evaluate_ast.cpp index 096e9f1dc4..acd7cdeb88 100644 --- a/source/lib/rocprofiler-sdk/counters/evaluate_ast.cpp +++ b/source/lib/rocprofiler-sdk/counters/evaluate_ast.cpp @@ -239,67 +239,140 @@ perform_selection(std::map& -get_ast_map() +const ASTs +load_asts() { - static std::unordered_map ast_map = []() { - std::unordered_map data; - const auto& metric_map = *CHECK_NOTNULL(counters::getMetricMap()); - for(const auto& [gfx, metrics] : metric_map) + std::unordered_map data; + + auto mets = counters::loadMetrics(true); + const auto& metric_map = mets->arch_to_metric; + for(const auto& [gfx, metrics] : metric_map) + { + // TODO: Remove global XML from derived counters... + if(gfx == "global") continue; + + std::unordered_map by_name; + for(const auto& metric : metrics) { - // TODO: Remove global XML from derived counters... - if(gfx == "global") continue; - - std::unordered_map by_name; - for(const auto& metric : metrics) - { - by_name.emplace(metric.name(), metric); - } - - auto& eval_map = data.emplace(gfx, EvaluateASTMap{}).first->second; - for(auto& [_, metric] : by_name) - { - RawAST* ast = nullptr; - auto* buf = - yy_scan_string(metric.expression().empty() ? metric.name().c_str() - : metric.expression().c_str()); - yyparse(&ast); - if(!ast) - { - ROCP_ERROR << fmt::format("Unable to parse metric {}", metric); - throw std::runtime_error(fmt::format("Unable to parse metric {}", metric)); - } - try - { - auto& evaluate_ast_node = - eval_map - .emplace(metric.name(), - EvaluateAST({.handle = metric.id()}, by_name, *ast, gfx)) - .first->second; - evaluate_ast_node.validate_raw_ast( - by_name); // TODO: refactor and consolidate internal post-construction - // logic as a Finish() method - } catch(std::exception& e) - { - ROCP_ERROR << e.what(); - throw std::runtime_error( - fmt::format("AST was not generated for {}:{}", gfx, metric.name())); - } - yy_delete_buffer(buf); - delete ast; - } - - for(auto& [name, ast] : eval_map) - { - ast.expand_derived(eval_map); - } + by_name.emplace(metric.name(), metric); } + auto& eval_map = data.emplace(gfx, EvaluateASTMap{}).first->second; + for(auto& [_, metric] : by_name) + { + RawAST* ast = nullptr; + auto* buf = yy_scan_string(metric.expression().empty() ? metric.name().c_str() + : metric.expression().c_str()); + yyparse(&ast); + if(!ast) + { + ROCP_ERROR << fmt::format("Unable to parse metric {}", metric); + throw std::runtime_error(fmt::format("Unable to parse metric {}", metric)); + } + try + { + auto& evaluate_ast_node = + eval_map + .emplace(metric.name(), + EvaluateAST({.handle = metric.id()}, by_name, *ast, gfx)) + .first->second; + evaluate_ast_node.validate_raw_ast( + by_name); // TODO: refactor and consolidate internal post-construction + // logic as a Finish() method + } catch(std::exception& e) + { + ROCP_ERROR << e.what(); + throw std::runtime_error( + fmt::format("AST was not generated for {}:{}", gfx, metric.name())); + } + yy_delete_buffer(buf); + delete ast; + } + + for(auto& [name, ast] : eval_map) + { + ast.expand_derived(eval_map); + } + } + + return {.arch_to_counter_asts = data}; +} + +} // namespace + +rocprofiler_status_t +check_ast_generation(std::string_view arch, Metric metric) +{ + auto metrics = counters::loadMetrics(); + const auto* metric_list = + rocprofiler::common::get_val(metrics->arch_to_metric, std::string(arch)); + if(!metric_list) return ROCPROFILER_STATUS_ERROR_AGENT_NOT_FOUND; + + RawAST* ast = nullptr; + auto* buf = yy_scan_string(metric.expression().empty() ? metric.name().c_str() + : metric.expression().c_str()); + + auto delete_ast = [&]() { + yy_delete_buffer(buf); + delete ast; + }; + + yyparse(&ast); + if(!ast) + { + if(buf) yy_delete_buffer(buf); + ROCP_ERROR << fmt::format("Unable to parse metric {}", metric); + return ROCPROFILER_STATUS_ERROR_AST_GENERATION_FAILED; + } + + std::unordered_map by_name; + for(const auto& existing_metric : *metric_list) + { + by_name.emplace(existing_metric.name(), existing_metric); + } + + if(!by_name.emplace(metric.name(), metric).second) + { + delete_ast(); + return ROCPROFILER_STATUS_ERROR_INVALID_ARGUMENT; + } + + try + { + auto evaluate_ast_node = + EvaluateAST({.handle = metric.id()}, by_name, *ast, std::string(arch)); + evaluate_ast_node.validate_raw_ast(by_name); + } catch(std::exception& e) + { + ROCP_ERROR << fmt::format("Unable to generate AST for {} error: {}", metric, e.what()); + delete_ast(); + return ROCPROFILER_STATUS_ERROR_AST_GENERATION_FAILED; + } + + delete_ast(); + return ROCPROFILER_STATUS_SUCCESS; +} + +std::shared_ptr +get_ast_map(bool reload) +{ + using ASTSync = common::Synchronized>; + static ASTSync*& ast_data = common::static_object::construct( + [&]() { return std::make_shared(load_asts()); }()); + + if(!reload) + { + return ast_data->rlock([](const auto& data) { + CHECK(data); + return data; + }); + } + + return ast_data->wlock([&](auto& data) { + data = std::make_shared(load_asts()); + CHECK(data); return data; - }(); - return ast_map; + }); } std::optional> diff --git a/source/lib/rocprofiler-sdk/counters/evaluate_ast.hpp b/source/lib/rocprofiler-sdk/counters/evaluate_ast.hpp index 918f0947d5..7eed89bf48 100644 --- a/source/lib/rocprofiler-sdk/counters/evaluate_ast.hpp +++ b/source/lib/rocprofiler-sdk/counters/evaluate_ast.hpp @@ -183,12 +183,20 @@ private: using EvaluateASTMap = std::unordered_map; +struct ASTs +{ + const std::unordered_map arch_to_counter_asts; +}; + +rocprofiler_status_t +check_ast_generation(std::string_view arch, Metric metric); + /** * Construct the ASTs for all counters appearing in basic/derived counter * definition files. */ -const std::unordered_map& -get_ast_map(); +std::shared_ptr +get_ast_map(bool reload = false); /** * Get the required basic/hardware counters needed to evaluate a diff --git a/source/lib/rocprofiler-sdk/counters/metrics.cpp b/source/lib/rocprofiler-sdk/counters/metrics.cpp index f39ce64d1e..cd533245a1 100644 --- a/source/lib/rocprofiler-sdk/counters/metrics.cpp +++ b/source/lib/rocprofiler-sdk/counters/metrics.cpp @@ -25,6 +25,7 @@ #include #include "lib/common/filesystem.hpp" +#include "lib/common/logging.hpp" #include "lib/common/static_object.hpp" #include "lib/common/synchronized.hpp" #include "lib/common/utility.hpp" @@ -45,6 +46,8 @@ #include // for dladdr #include #include +#include +#include namespace rocprofiler { @@ -59,13 +62,6 @@ getCustomCounterDefinition() return def; } -uint64_t& -current_id() -{ - static uint64_t id = 0; - return id; -} - /** * Constant/speical metrics are treated as psudo-metrics in that they * are given their own metric id. MAX_WAVE_SIZE for example is not collected @@ -75,11 +71,10 @@ current_id() * used in derived counters (exact support properties that can be used can * be viewed in evaluate_ast.cpp:get_agent_property()). */ -const std::vector& -get_constants() +std::vector +get_constants(uint64_t starting_id) { - static std::vector constants; - if(!constants.empty()) return constants; + std::vector constants; // Ensure topology is read rocprofiler::agent::get_agents(); for(const auto& prop : rocprofiler::agent::get_agent_available_properties()) @@ -91,8 +86,8 @@ get_constants() fmt::format("Constant value {} from agent properties", prop), "", "yes", - current_id()); - current_id()++; + starting_id); + starting_id++; } return constants; } @@ -109,9 +104,12 @@ get_constants() * ... * description: General counter desctiption */ -MetricMap -loadYAML(const std::string& filename, bool load_constants = false, bool load_derived = false) +counter_metrics_t +loadYAML(const std::string& filename, std::optional add_metric) { + // Stores metrics that are added via the API + static MetricMap added_metrics; + MetricMap ret; auto override = getCustomCounterDefinition().wlock([&](auto& data) { data.loaded = true; @@ -132,7 +130,8 @@ loadYAML(const std::string& filename, bool load_constants = false, bool load_der counter_data << override.data; } - auto yaml = YAML::Load(counter_data.str()); + auto yaml = YAML::Load(counter_data.str()); + uint64_t current_id = 0; for(auto it = yaml.begin(); it != yaml.end(); ++it) { @@ -157,37 +156,87 @@ loadYAML(const std::string& filename, bool load_constants = false, bool load_der while(std::getline(ss, arch_name, '/')) { auto& metricVec = ret.emplace(arch_name, std::vector()).first->second; - if(metricVec.empty() && load_constants) + if(metricVec.empty()) { - metricVec.insert( - metricVec.end(), get_constants().begin(), get_constants().end()); + const auto constants = get_constants(current_id); + metricVec.insert(metricVec.end(), constants.begin(), constants.end()); + current_id += constants.size(); } - if((def["expression"] && load_derived) || (!load_derived && !def["expression"])) - { - std::string description; - if(def["description"]) - description = def["description"].as(); - else if(counter_def["description"]) - description = counter_def["description"].as(); - metricVec.emplace_back( - arch_name, - counter_name, - (def["block"] ? def["block"].as() : ""), - (def["event"] ? def["event"].as() : ""), - description, - (def["expression"] ? def["expression"].as() : ""), - "", - current_id()); - current_id()++; - ROCP_TRACE << fmt::format("Inserted info {}: {}", arch_name, metricVec.back()); - } + std::string description; + if(def["description"]) + description = def["description"].as(); + else if(counter_def["description"]) + description = counter_def["description"].as(); + metricVec.emplace_back( + arch_name, + counter_name, + (def["block"] ? def["block"].as() : ""), + (def["event"] ? def["event"].as() : ""), + description, + (def["expression"] ? def["expression"].as() : ""), + "", + current_id); + current_id++; + ROCP_TRACE << fmt::format("Inserted info {}: {}", arch_name, metricVec.back()); } } } - ROCP_FATAL_IF(current_id() > 65536) + + // Add custom counters after adding the above counters, ensures that the mapping is + // deterministic when generated. + for(const auto& [arch, metrics] : added_metrics) + { + auto& metricVec = ret.emplace(arch, std::vector()).first->second; + metricVec.insert(metricVec.end(), metrics.begin(), metrics.end()); + current_id += metrics.size(); + } + + if(add_metric) + { + Metric with_id = Metric(add_metric->first, + add_metric->second.name(), + add_metric->second.block(), + add_metric->second.event(), + add_metric->second.description(), + add_metric->second.expression(), + "", + current_id); + added_metrics.emplace(add_metric->first, std::vector{}) + .first->second.push_back(with_id); + ret.emplace(add_metric->first, std::vector{}).first->second.push_back(with_id); + } + + ROCP_FATAL_IF(current_id > 65536) << "Counter count exceeds 16 bits, which may break counter id output"; - return ret; + + return {.arch_to_metric = ret, + .id_to_metric = + [&]() { + MetricIdMap map; + for(const auto& [agent_name, metrics] : ret) + { + for(const auto& m : metrics) + { + map.emplace(m.id(), m); + } + } + return map; + }(), + .arch_to_id = + [&]() { + ArchToId map; + for(const auto& [agent_name, metrics] : ret) + { + std::unordered_set ids; + for(const auto& m : metrics) + { + ids.insert(m.id()); + } + map.emplace(agent_name, std::move(ids)); + } + return map; + }()}; } std::string @@ -229,47 +278,51 @@ setCustomCounterDefinition(const CustomCounterDefinition& def) }); } -MetricMap -getDerivedHardwareMetrics() +std::shared_ptr +loadMetrics(bool reload, const std::optional add_metric) { - auto counters_path = findViaEnvironment("counter_defs.yaml"); - ROCP_FATAL_IF(!common::filesystem::exists(counters_path)) - << "metric xml file '" << counters_path << "' does not exist"; - return loadYAML(counters_path, false, true); -} + using sync_metric = common::Synchronized>; -MetricMap -getBaseHardwareMetrics() -{ - auto counters_path = findViaEnvironment("counter_defs.yaml"); - ROCP_FATAL_IF(!common::filesystem::exists(counters_path)) - << "metric xml file '" << counters_path << "' does not exist"; - return loadYAML(counters_path, true, false); -} + if(!reload && add_metric) + { + ROCP_FATAL << "Adding a metric without reloading metric list, this should not happen and " + "will result in custom metrics not being added"; + } -const MetricIdMap* -getMetricIdMap() -{ - static MetricIdMap*& id_map = common::static_object::construct([]() { - MetricIdMap map; - for(const auto& [_, val] : *CHECK_NOTNULL(getMetricMap())) - { - for(const auto& metric : val) - { - map.emplace(metric.id(), metric); - } - } - return map; - }()); - return id_map; + auto reload_func = [&]() { + auto counters_path = findViaEnvironment("counter_defs.yaml"); + ROCP_FATAL_IF(!common::filesystem::exists(counters_path)) + << "metric xml file '" << counters_path << "' does not exist"; + return std::make_shared(loadYAML(counters_path, add_metric)); + }; + + static sync_metric*& id_map = + common::static_object::construct([&]() { return reload_func(); }()); + + if(!id_map) return nullptr; + + if(!reload) + { + return id_map->rlock([](const auto& data) { + CHECK(data); + return data; + }); + } + + return id_map->wlock([&](auto& data) { + data = reload_func(); + CHECK(data); + return data; + }); } std::unordered_map getPerfCountersIdMap() { std::unordered_map map; + auto mets = loadMetrics(); - for(const auto& [agent, list] : *CHECK_NOTNULL(getMetricMap())) + for(const auto& [agent, list] : mets->arch_to_metric) { if(agent.find("gfx9") == std::string::npos) continue; for(const auto& metric : list) @@ -282,29 +335,11 @@ getPerfCountersIdMap() return map; } -const MetricMap* -getMetricMap() -{ - static MetricMap*& map = common::static_object::construct([]() { - MetricMap ret = getBaseHardwareMetrics(); - for(auto& [key, val] : getDerivedHardwareMetrics()) - { - auto [iter, inserted] = ret.emplace(key, val); - if(!inserted) - { - iter->second.insert(iter->second.end(), val.begin(), val.end()); - } - } - return ret; - }()); - return map; -} - std::vector getMetricsForAgent(const std::string& agent) { - const auto& map = *CHECK_NOTNULL(getMetricMap()); - if(const auto* metric_ptr = rocprofiler::common::get_val(map, agent)) + auto mets = loadMetrics(); + if(const auto* metric_ptr = rocprofiler::common::get_val(mets->arch_to_metric, agent)) { return *metric_ptr; } @@ -315,24 +350,8 @@ getMetricsForAgent(const std::string& agent) bool checkValidMetric(const std::string& agent, const Metric& metric) { - static auto*& agent_to_id = - common::static_object>>:: - construct([]() -> std::unordered_map> { - std::unordered_map> ret; - const auto& map = *CHECK_NOTNULL(getMetricMap()); - for(const auto& [agent_name, metrics] : map) - { - auto& id_set = - ret.emplace(agent_name, std::unordered_set{}).first->second; - for(const auto& m : metrics) - { - id_set.insert(m.id()); - } - } - return ret; - }()); - - const auto* agent_map = common::get_val(*agent_to_id, agent); + auto metrics = loadMetrics(); + const auto* agent_map = common::get_val(metrics->arch_to_id, agent); return agent_map != nullptr && agent_map->count(metric.id()) > 0; } @@ -351,7 +370,7 @@ operator==(Metric const& lhs, Metric const& rhs) x.event_, x.description_, x.expression_, - x.special_, + x.constant_, x.id_, x.empty_, x.flags_); diff --git a/source/lib/rocprofiler-sdk/counters/metrics.hpp b/source/lib/rocprofiler-sdk/counters/metrics.hpp index 606cf59811..c7870ad9f2 100644 --- a/source/lib/rocprofiler-sdk/counters/metrics.hpp +++ b/source/lib/rocprofiler-sdk/counters/metrics.hpp @@ -22,11 +22,14 @@ #pragma once +#include "rocprofiler-sdk/fwd.h" + #include +#include #include #include +#include #include -#include "rocprofiler-sdk/fwd.h" #include #include @@ -47,14 +50,14 @@ public: std::string event, std::string dsc, std::string expr, - std::string special, + std::string constant, uint64_t id) : name_(std::move(name)) , block_(std::move(block)) , event_(std::move(event)) , description_(std::move(dsc)) , expression_(std::move(expr)) - , special_(std::move(special)) + , constant_(std::move(constant)) , id_(id) {} @@ -63,7 +66,7 @@ public: const std::string& event() const { return event_; } const std::string& description() const { return description_; } const std::string& expression() const { return expression_; } - const std::string& special() const { return special_; } + const std::string& constant() const { return constant_; } uint64_t id() const { return id_; } uint32_t flags() const { return flags_; } bool empty() const { return empty_; } @@ -79,7 +82,7 @@ private: std::string event_ = {}; std::string description_ = {}; std::string expression_ = {}; - std::string special_ = {}; + std::string constant_ = {}; int64_t id_ = -1; bool empty_ = false; uint32_t flags_ = 0; @@ -94,24 +97,18 @@ struct CustomCounterDefinition using MetricMap = std::unordered_map>; using MetricIdMap = std::unordered_map; +using ArchToId = std::unordered_map>; +using ArchMetric = std::pair; -/** - * Get base hardware counters for all GFXs Map - */ -MetricMap -getBaseHardwareMetrics(); +struct counter_metrics_t +{ + const MetricMap arch_to_metric; + const MetricIdMap id_to_metric; + const ArchToId arch_to_id; +}; -/** - * Get derived hardware metrics for all GFXs Map - */ -MetricMap -getDerivedHardwareMetrics(); - -/** - * Combined map containing both base and derived counters - */ -const MetricMap* -getMetricMap(); +std::shared_ptr +loadMetrics(bool reload = false, std::optional add_metric = std::nullopt); /** * Get the metrics that apply to a specific agent. Supplied parameter @@ -120,12 +117,6 @@ getMetricMap(); std::vector getMetricsForAgent(const std::string&); -/** - * Get a map of metric::id() -> metric - */ -const MetricIdMap* -getMetricIdMap(); - /** * Get the metric event ids for perfcounters options in thread trace * applicable only for GFX9 agents and SQ block counters diff --git a/source/lib/rocprofiler-sdk/counters/parser/tests/parser_test.cpp b/source/lib/rocprofiler-sdk/counters/parser/tests/parser_test.cpp index 5789daa1ba..aeda258286 100644 --- a/source/lib/rocprofiler-sdk/counters/parser/tests/parser_test.cpp +++ b/source/lib/rocprofiler-sdk/counters/parser/tests/parser_test.cpp @@ -274,15 +274,17 @@ TEST(parser, selection) } } -TEST(parser, parse_derived_counters) +TEST(parser, parse_counters) { // Checks that ASTs are properly formed from derived counters defined in XML // Does not check accuracy, only parseability - auto derived_counters = rocprofiler::counters::getDerivedHardwareMetrics(); - for(auto& [gfx, counter_list] : derived_counters) + auto derived_counters = rocprofiler::counters::loadMetrics(); + for(const auto& [gfx, counter_list] : derived_counters->arch_to_metric) { for(const auto& v : counter_list) { + if(!v.constant().empty() || v.expression().empty()) continue; + RawAST* ast = nullptr; auto* buf = yy_scan_string(v.expression().c_str()); yyparse(&ast); diff --git a/source/lib/rocprofiler-sdk/counters/tests/CMakeLists.txt b/source/lib/rocprofiler-sdk/counters/tests/CMakeLists.txt index 1bc693b771..fefca834b8 100644 --- a/source/lib/rocprofiler-sdk/counters/tests/CMakeLists.txt +++ b/source/lib/rocprofiler-sdk/counters/tests/CMakeLists.txt @@ -56,8 +56,8 @@ target_link_libraries( rocprofiler-sdk::rocprofiler-sdk-hsa-runtime) set(ROCPROFILER_LIB_COUNTER_TEST_SOURCES - metrics_test.cpp evaluate_ast_test.cpp dimension.cpp init_order.cpp core.cpp - code_object_loader.cpp device_counting.cpp) + metrics_test.cpp evaluate_ast_test.cpp dimension.cpp core.cpp code_object_loader.cpp + device_counting.cpp) set(ROCPROFILER_LIB_COUNTER_TEST_HEADERS code_object_loader.hpp device_counting.hpp) add_executable(counter-test) diff --git a/source/lib/rocprofiler-sdk/counters/tests/core.cpp b/source/lib/rocprofiler-sdk/counters/tests/core.cpp index 618add850e..d909757ae5 100644 --- a/source/lib/rocprofiler-sdk/counters/tests/core.cpp +++ b/source/lib/rocprofiler-sdk/counters/tests/core.cpp @@ -76,17 +76,18 @@ auto findDeviceMetrics(const hsa::AgentCache& agent, const std::unordered_set& metrics) { std::vector ret; - auto all_counters = counters::getMetricMap(); + auto mets = counters::loadMetrics(); + const auto& all_counters = mets->arch_to_metric; ROCP_INFO << "Looking up counters for " << std::string(agent.name()); - auto gfx_metrics = common::get_val(*all_counters, std::string(agent.name())); + const auto* gfx_metrics = common::get_val(all_counters, std::string(agent.name())); if(!gfx_metrics) { ROCP_ERROR << "No counters found for " << std::string(agent.name()); return ret; } - for(auto& counter : *gfx_metrics) + for(const auto& counter : *gfx_metrics) { if(metrics.count(counter.name()) > 0 || metrics.empty()) { @@ -246,12 +247,13 @@ TEST(core, check_packet_generation) * Check that required hardware counters match */ ASSERT_TRUE(profile->agent); - auto name_str = std::string(profile->agent->name); - auto req_counters = - counters::get_required_hardware_counters(counters::get_ast_map(), name_str, metric); + auto name_str = std::string(profile->agent->name); + const auto asts = counters::get_ast_map(); + auto req_counters = counters::get_required_hardware_counters( + asts->arch_to_counter_asts, name_str, metric); for(const auto& req_metric : *req_counters) { - if(req_metric.special().empty()) + if(req_metric.constant().empty()) { EXPECT_GT(profile->reqired_hw_counters.count(req_metric), 0) << "Could not find metric - " << req_metric.name(); @@ -811,3 +813,47 @@ TEST_YAML_LOAD: 2); } } + +TEST(core, create_counter) +{ + std::vector to_add = { + Metric("", "SQ_WAVES_REDUCE_T", "", "", "", "reduce(SQ_WAVES,sum)", "", 10000), + Metric("", "SQ_WAVES_AVR_T", "", "", "", "reduce(SQ_WAVES,avr)", "", 10001), + Metric("", "SQ_WAVES_INVALID", "", "", "", "reduce(ABC,avr)", "", 10002), + }; + + ASSERT_EQ(hsa_init(), HSA_STATUS_SUCCESS); + test_init(); + + registration::init_logging(); + registration::set_init_status(-1); + context::push_client(1); + auto agents = hsa::get_queue_controller()->get_supported_agents(); + for(const auto& [_, agent] : agents) + { + for(auto& metric : to_add) + { + rocprofiler_counter_id_t id; + auto status = rocprofiler_create_counter(metric.name().c_str(), + metric.name().size(), + metric.expression().c_str(), + metric.expression().size(), + metric.description().c_str(), + metric.description().size(), + agent.get_rocp_agent()->id, + &id); + auto metrics = findDeviceMetrics(agent, {metric.name()}); + if(metric.name() == "SQ_WAVES_INVALID") + { + EXPECT_EQ(status, ROCPROFILER_STATUS_ERROR_AST_GENERATION_FAILED); + EXPECT_TRUE(metrics.empty()); + } + else + { + EXPECT_EQ(metrics.size(), 1); + EXPECT_EQ(metric.name(), metrics[0].name()); + EXPECT_EQ(metric.expression(), metrics[0].expression()); + } + } + } +} diff --git a/source/lib/rocprofiler-sdk/counters/tests/device_counting.cpp b/source/lib/rocprofiler-sdk/counters/tests/device_counting.cpp index ea9f86c21c..588ca4e277 100644 --- a/source/lib/rocprofiler-sdk/counters/tests/device_counting.cpp +++ b/source/lib/rocprofiler-sdk/counters/tests/device_counting.cpp @@ -75,10 +75,10 @@ auto findDeviceMetrics(const hsa::AgentCache& agent, const std::unordered_set& metrics) { std::vector ret; - const auto* all_counters = counters::getMetricMap(); - + auto mets = counters::loadMetrics(); + const auto& all_counters = mets->arch_to_metric; ROCP_INFO << "Looking up counters for " << std::string(agent.name()); - const auto* gfx_metrics = common::get_val(*all_counters, std::string(agent.name())); + const auto* gfx_metrics = common::get_val(all_counters, std::string(agent.name())); if(!gfx_metrics) { ROCP_INFO << "No counters found for " << std::string(agent.name()); @@ -518,8 +518,8 @@ protected: ROCP_INFO << fmt::format("Running test on agent {:x}", gpu_agent.get_hsa_agent().handle); - - const auto* agent_map = rocprofiler::common::get_val(counters::get_ast_map(), + auto ast_map = counters::get_ast_map(); + const auto* agent_map = rocprofiler::common::get_val(ast_map->arch_to_counter_asts, std::string(gpu_agent.name())); CHECK(agent_map); const auto* original_ast = rocprofiler::common::get_val(*agent_map, metric_to_test); diff --git a/source/lib/rocprofiler-sdk/counters/tests/dimension.cpp b/source/lib/rocprofiler-sdk/counters/tests/dimension.cpp index 1da80f6421..dd14d1e4cb 100644 --- a/source/lib/rocprofiler-sdk/counters/tests/dimension.cpp +++ b/source/lib/rocprofiler-sdk/counters/tests/dimension.cpp @@ -158,17 +158,18 @@ auto findDeviceMetrics(const hsa::AgentCache& agent, const std::unordered_set& metrics) { std::vector ret; - auto all_counters = counters::getMetricMap(); + auto mets = counters::loadMetrics(); + const auto& all_counters = mets->arch_to_metric; ROCP_INFO << "Looking up counters for " << std::string(agent.name()); - auto gfx_metrics = common::get_val(*all_counters, std::string(agent.name())); + const auto* gfx_metrics = common::get_val(all_counters, std::string(agent.name())); if(!gfx_metrics) { ROCP_ERROR << "No counters found for " << std::string(agent.name()); return ret; } - for(auto& counter : *gfx_metrics) + for(const auto& counter : *gfx_metrics) { if(metrics.count(counter.name()) > 0 || metrics.empty()) { @@ -212,8 +213,8 @@ TEST(dimension, block_dim_test) */ std::unordered_map rocp_dims; - ROCP_INFO << metric.name() << " " << metric.special(); - if(!metric.special().empty()) + ROCP_INFO << metric.name() << " " << metric.constant(); + if(!metric.constant().empty()) { rocp_dims[counters::rocprofiler_profile_counter_instance_types:: ROCPROFILER_DIMENSION_INSTANCE] = 1; @@ -261,8 +262,8 @@ TEST(dimension, block_dim_test) /** * Check this value exists in the dimension cache */ - const auto* dim_cache = - rocprofiler::common::get_val(counters::get_dimension_cache(), metric.id()); + auto dim_ptr = counters::get_dimension_cache(); + const auto* dim_cache = rocprofiler::common::get_val(dim_ptr->id_to_dim, metric.id()); ASSERT_TRUE(dim_cache); EXPECT_EQ(fmt::format("{}", fmt::join(dims, "|")), fmt::format("{}", fmt::join(*dim_cache, "|"))); diff --git a/source/lib/rocprofiler-sdk/counters/tests/evaluate_ast_test.cpp b/source/lib/rocprofiler-sdk/counters/tests/evaluate_ast_test.cpp index 7b002ca526..e2f38b4f4a 100644 --- a/source/lib/rocprofiler-sdk/counters/tests/evaluate_ast_test.cpp +++ b/source/lib/rocprofiler-sdk/counters/tests/evaluate_ast_test.cpp @@ -312,7 +312,7 @@ TEST(evaluate_ast, counter_constants) for(const auto& c : *eval_counters) { EXPECT_NE(expected.find(c.name()), expected.end()); - EXPECT_TRUE(!c.special().empty()); + EXPECT_TRUE(!c.constant().empty()); } // Check that special counters are being decoded properly by the AST @@ -451,7 +451,7 @@ TEST(evaluate_ast, evaluate_simple_counters) for(const auto& [val, metric] : metrics) { RawAST* ast = nullptr; - auto buf = yy_scan_string(metric.expression().empty() ? metric.name().c_str() + auto* buf = yy_scan_string(metric.expression().empty() ? metric.name().c_str() : metric.expression().c_str()); yyparse(&ast); ASSERT_TRUE(ast) << metric.expression() << " " << metric.name(); @@ -470,7 +470,7 @@ TEST(evaluate_ast, evaluate_simple_counters) ASSERT_TRUE(eval_counters); ASSERT_EQ(eval_counters->size(), 1); EXPECT_EQ(eval_counters->begin()->name(), name); - EXPECT_TRUE(eval_counters->begin()->special().empty()); + EXPECT_TRUE(eval_counters->begin()->constant().empty()); std::unordered_map> decode = { {metrics[name].id(), expected}}; std::vector>> cache; diff --git a/source/lib/rocprofiler-sdk/counters/tests/init_order.cpp b/source/lib/rocprofiler-sdk/counters/tests/init_order.cpp deleted file mode 100644 index dee96cb12d..0000000000 --- a/source/lib/rocprofiler-sdk/counters/tests/init_order.cpp +++ /dev/null @@ -1,178 +0,0 @@ -// MIT License -// -// Copyright (c) 2023-2025 Advanced Micro Devices, Inc. All rights reserved. -// -// Permission is hereby granted, free of charge, to any person obtaining a copy -// of this software and associated documentation files (the "Software"), to deal -// in the Software without restriction, including without limitation the rights -// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -// copies of the Software, and to permit persons to whom the Software is -// furnished to do so, subject to the following conditions: -// -// The above copyright notice and this permission notice shall be included in all -// copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -// SOFTWARE. - -#include "lib/common/static_object.hpp" -#include "lib/common/utility.hpp" -#include "lib/rocprofiler-sdk/context/context.hpp" -#include "lib/rocprofiler-sdk/counters/metrics.hpp" -#include "lib/rocprofiler-sdk/registration.hpp" - -#include -#include -#include - -#include -#include - -#include -#include - -using namespace rocprofiler::counters; - -#define ROCPROFILER_CALL(result, msg) \ - { \ - rocprofiler_status_t CHECKSTATUS = result; \ - if(CHECKSTATUS != ROCPROFILER_STATUS_SUCCESS) \ - { \ - std::string status_msg = rocprofiler_get_status_string(CHECKSTATUS); \ - std::cerr << "[" #result "][" << __FILE__ << ":" << __LINE__ << "] " << msg \ - << " failed with error code " << CHECKSTATUS << ": " << status_msg \ - << std::endl; \ - std::stringstream errmsg{}; \ - errmsg << "[" #result "][" << __FILE__ << ":" << __LINE__ << "] " << msg " failure (" \ - << status_msg << ")"; \ - throw std::runtime_error(errmsg.str()); \ - } \ - } - -struct metric_map_order -{ - metric_map_order() = default; - ~metric_map_order() { check_copy(); } - - metric_map_order(const metric_map_order&) = delete; - metric_map_order& operator=(const metric_map_order&) = delete; - - metric_map_order(metric_map_order&&) noexcept = delete; - metric_map_order& operator=(metric_map_order&&) noexcept = delete; - - void check_copy() - { - ASSERT_TRUE(!copy_.empty()); - - const auto* metricIdMap = rocprofiler::counters::getMetricIdMap(); - int fini_status = 0; - ROCPROFILER_CALL(rocprofiler_is_finalized(&fini_status), "get finalization state"); - - if(fini_status > 0) - { - // this should only be true in the destructor of the static metric_map_order instance - ASSERT_TRUE(metricIdMap != nullptr) << "rocprofiler finalization state: " << fini_status - << ", metricIdMap: " << metricIdMap; - - // this should ensure the metric id map is destroyed - rocprofiler::common::destroy_static_objects(); - metricIdMap = rocprofiler::counters::getMetricIdMap(); - - ASSERT_TRUE(metricIdMap == nullptr) << "rocprofiler finalization state: " << fini_status - << ", metricIdMap: " << metricIdMap; - } - else - { - for(const auto& [id, actual] : copy_) - { - // Assert because this is getting triggered on shutdown and - // we want to fail the test if the values in both maps are not equal. - const auto* val = rocprofiler::common::get_val(*metricIdMap, id); - ASSERT_TRUE(val != nullptr) << "metricIdMap: " << metricIdMap; - ASSERT_TRUE(*val == actual) << "metricIdMap: " << metricIdMap; - } - } - } - -private: - MetricIdMap copy_ = *CHECK_NOTNULL(rocprofiler::counters::getMetricIdMap()); -}; - -namespace -{ -metric_map_order& -get_metric_map() -{ - static metric_map_order order = {}; - return order; -} - -void -buffered_callback(rocprofiler_context_id_t, - rocprofiler_buffer_id_t, - rocprofiler_record_header_t**, - size_t, - void*, - uint64_t) -{} - -void -dispatch_callback(rocprofiler_dispatch_counting_service_data_t, - rocprofiler_profile_config_id_t*, - rocprofiler_user_data_t*, - void*) -{} - -rocprofiler_context_id_t& -get_client_ctx() -{ - static rocprofiler_context_id_t ctx{0}; - return ctx; -} - -rocprofiler_buffer_id_t& -get_buffer() -{ - static rocprofiler_buffer_id_t buf = {}; - return buf; -} -} // namespace - -// Test that metrics map remains in scope at exit -TEST(counters_init_order, metric_map_order) -{ - rocprofiler::registration::init_logging(); - // do not call rocprofiler::registration::initialize()! - // doing so will add an atexit call which might invoke - // rocprofiler::common::destroy_static_objects() before - // the get_metric_map() instance is destroyed - - rocprofiler::registration::set_init_status(-1); - rocprofiler::context::push_client(1); - ROCPROFILER_CALL(rocprofiler_create_context(&get_client_ctx()), "context creation failed"); - ROCPROFILER_CALL(rocprofiler_create_buffer(get_client_ctx(), - 4096, - 2048, - ROCPROFILER_BUFFER_POLICY_LOSSLESS, - buffered_callback, - nullptr, - &get_buffer()), - "buffer creation failed"); - ROCPROFILER_CALL(rocprofiler_configure_buffered_dispatch_counting_service( - get_client_ctx(), get_buffer(), dispatch_callback, nullptr), - "Could not setup buffered service"); - rocprofiler::registration::set_init_status(1); - - auto& global_metric_map = get_metric_map(); - global_metric_map.check_copy(); - - auto local_metric_map = metric_map_order{}; - local_metric_map.check_copy(); - - rocprofiler::registration::finalize(); -} diff --git a/source/lib/rocprofiler-sdk/counters/tests/metrics_test.cpp b/source/lib/rocprofiler-sdk/counters/tests/metrics_test.cpp index 25194bf489..fe31afbf65 100644 --- a/source/lib/rocprofiler-sdk/counters/tests/metrics_test.cpp +++ b/source/lib/rocprofiler-sdk/counters/tests/metrics_test.cpp @@ -61,7 +61,21 @@ loadTestData(const std::unordered_map> ret; + for(const auto& [gfx, metrics] : loaded_metrics->arch_to_metric) + { + std::vector base_metrics; + std::copy_if(metrics.begin(), + metrics.end(), + std::back_inserter(base_metrics), + [](const auto& m) { return m.expression().empty(); }); + if(!base_metrics.empty()) ret.emplace(gfx, std::move(base_metrics)); + } + return ret; + }(); auto test_data = loadTestData(basic_gfx908); ASSERT_EQ(rocp_data.count("gfx908"), 1); @@ -97,7 +111,22 @@ TEST(metrics, base_load) TEST(metrics, derived_load) { - auto rocp_data = counters::getDerivedHardwareMetrics(); + auto loaded_metrics = counters::loadMetrics(); + auto rocp_data = [&]() { + // get only derrived metrics + std::unordered_map> ret; + for(const auto& [gfx, metrics] : loaded_metrics->arch_to_metric) + { + std::vector derived_metrics; + std::copy_if(metrics.begin(), + metrics.end(), + std::back_inserter(derived_metrics), + [](const auto& m) { return !m.expression().empty(); }); + if(!derived_metrics.empty()) ret.emplace(gfx, std::move(derived_metrics)); + } + return ret; + }(); + auto test_data = loadTestData(derived_gfx908); ASSERT_EQ(rocp_data.count("gfx908"), 1); ASSERT_EQ(test_data.count("gfx908"), 1); @@ -128,8 +157,10 @@ TEST(metrics, derived_load) TEST(metrics, check_agent_valid) { - const auto& rocp_data = *counters::getMetricMap(); - auto common_metrics = [&]() -> std::set { + auto mets = counters::loadMetrics(); + const auto& rocp_data = mets->arch_to_metric; + + auto common_metrics = [&]() -> std::set { std::set ret; for(const auto& [gfx, counters] : rocp_data) { @@ -171,7 +202,7 @@ TEST(metrics, check_agent_valid) if(other_gfx == gfx) continue; for(const auto& metric : other_counters) { - if(common_metrics.count(metric.id()) || !metric.special().empty()) continue; + if(common_metrics.count(metric.id()) > 0 || !metric.constant().empty()) continue; EXPECT_EQ(counters::checkValidMetric(gfx, metric), false) << fmt::format("GFX {} has Metric {} but shouldn't", gfx, metric); } @@ -181,8 +212,9 @@ TEST(metrics, check_agent_valid) TEST(metrics, check_public_api_query) { - const auto* id_map = counters::getMetricIdMap(); - for(const auto& [id, metric] : *id_map) + auto metrics_map = rocprofiler::counters::loadMetrics(); + const auto& id_map = metrics_map->id_to_metric; + for(const auto& [id, metric] : id_map) { rocprofiler_counter_info_v0_t version; @@ -190,10 +222,10 @@ TEST(metrics, check_public_api_query) rocprofiler_query_counter_info( {.handle = id}, ROCPROFILER_COUNTER_INFO_VERSION_0, static_cast(&version)), ROCPROFILER_STATUS_SUCCESS); - EXPECT_EQ(version.name, metric.name().c_str()); - EXPECT_EQ(version.block, metric.block().c_str()); - EXPECT_EQ(version.expression, metric.expression().c_str()); + EXPECT_EQ(std::string(version.name), metric.name()); + EXPECT_EQ(std::string(version.block), metric.block()); + EXPECT_EQ(std::string(version.expression), metric.expression()); EXPECT_EQ(version.is_derived, !metric.expression().empty()); - EXPECT_EQ(version.description, metric.description().c_str()); + EXPECT_EQ(std::string(version.description), metric.description()); } } diff --git a/source/lib/rocprofiler-sdk/profile_config.cpp b/source/lib/rocprofiler-sdk/profile_config.cpp index d04d5345ab..2462efee97 100644 --- a/source/lib/rocprofiler-sdk/profile_config.cpp +++ b/source/lib/rocprofiler-sdk/profile_config.cpp @@ -56,7 +56,9 @@ rocprofiler_create_profile_config(rocprofiler_agent_id_t agent_id, std::shared_ptr config = std::make_shared(); - const auto& id_map = *CHECK_NOTNULL(rocprofiler::counters::getMetricIdMap()); + auto metrics_map = rocprofiler::counters::loadMetrics(); + const auto& id_map = metrics_map->id_to_metric; + for(size_t i = 0; i < counters_count; i++) { auto& counter_id = counters_list[i];