From cb3fc070c7cc9f1edfa0b4d2a6248e778e3cc9f1 Mon Sep 17 00:00:00 2001 From: Benjamin Welton Date: Wed, 1 May 2024 13:34:54 -0700 Subject: [PATCH] [1/N] Agent Counter Collection Implementation (#832) Added public API call to setup agent counter collection on a context. Refactored the return types internally for dispatch counter collection to use rocprofiler_status_t (allow for more verbose failures to be surfaced via the API) Subsequent commits will fill out the sampling functionality for agent counter collection. Co-authored-by: Benjamin Welton --- .../include/rocprofiler-sdk/agent_profile.h | 26 +++------- source/include/rocprofiler-sdk/fwd.h | 4 +- source/include/rocprofiler-sdk/rocprofiler.h | 2 +- source/lib/rocprofiler-sdk/CMakeLists.txt | 1 + source/lib/rocprofiler-sdk/agent_profile.cpp | 35 +++++++++++++ .../lib/rocprofiler-sdk/context/context.hpp | 28 +++++++--- .../rocprofiler-sdk/counters/controller.cpp | 40 +++++++++++++-- .../rocprofiler-sdk/counters/controller.hpp | 17 ++++--- source/lib/rocprofiler-sdk/counters/core.cpp | 12 ++++- source/lib/rocprofiler-sdk/counters/core.hpp | 9 +++- .../rocprofiler-sdk/counters/tests/core.cpp | 51 +++++++++++++++++++ .../lib/rocprofiler-sdk/dispatch_profile.cpp | 12 +---- .../rocprofiler-sdk/hsa/queue_controller.cpp | 2 +- source/lib/rocprofiler-sdk/rocprofiler.cpp | 6 +++ 14 files changed, 192 insertions(+), 53 deletions(-) create mode 100644 source/lib/rocprofiler-sdk/agent_profile.cpp diff --git a/source/include/rocprofiler-sdk/agent_profile.h b/source/include/rocprofiler-sdk/agent_profile.h index 09a636ce81..167eb43ce3 100644 --- a/source/include/rocprofiler-sdk/agent_profile.h +++ b/source/include/rocprofiler-sdk/agent_profile.h @@ -34,30 +34,18 @@ ROCPROFILER_EXTERN_C_INIT * @{ */ -/** - * @brief ROCProfiler Agent Profile Counting Data. - * - * Counters, including identifiers to get counter information and Counters values - */ -typedef struct -{ - /** - */ - rocprofiler_record_counter_t* counters; - uint64_t counters_count; -} rocprofiler_agent_profile_counting_data_t; - /** * @brief Configure Profile Counting Service for agent. * - * @param [in] buffer_id - * @param [in] profile_config_id + * @param [in] context_id context id + * @param [in] buffer_id id of the buffer to use for the counting service + * @param [in] config_id Profile config detailing the counters to collect for this kernel * @return ::rocprofiler_status_t */ rocprofiler_status_t ROCPROFILER_API -rocprofiler_configure_agent_profile_counting_service( - rocprofiler_buffer_id_t buffer_id, - rocprofiler_profile_config_id_t profile_config_id); +rocprofiler_configure_agent_profile_counting_service(rocprofiler_context_id_t context_id, + rocprofiler_buffer_id_t buffer_id, + rocprofiler_profile_config_id_t config_id); /** * @brief Sample Profile Counting Service for agent. @@ -66,7 +54,7 @@ rocprofiler_configure_agent_profile_counting_service( * @return ::rocprofiler_status_t */ rocprofiler_status_t ROCPROFILER_API -rocprofiler_sample_agent_profile_counting_service(rocprofiler_agent_profile_counting_data_t* data); +rocprofiler_sample_agent_profile_counting_service(rocprofiler_context_id_t context_id); /** @} */ diff --git a/source/include/rocprofiler-sdk/fwd.h b/source/include/rocprofiler-sdk/fwd.h index 87137877c4..3d2e0bf29b 100644 --- a/source/include/rocprofiler-sdk/fwd.h +++ b/source/include/rocprofiler-sdk/fwd.h @@ -92,7 +92,9 @@ typedef enum // NOLINT(performance-enum-size) ROCPROFILER_STATUS_ERROR_INCOMPATIBLE_KERNEL, ///< A service depends on a newer version of KFD ///< (amdgpu kernel driver). Check logs for ///< service that report incompatibility - + ROCPROFILER_STATUS_ERROR_PROFILE_NOT_FOUND, ///< Could not find the counter profile + ROCPROFILER_STATUS_ERROR_AGENT_DISPATCH_CONFLICT, ///< Cannot enable both agent and dispatch + ///< counting in the same context. ROCPROFILER_STATUS_LAST, } rocprofiler_status_t; diff --git a/source/include/rocprofiler-sdk/rocprofiler.h b/source/include/rocprofiler-sdk/rocprofiler.h index dd27304c9f..4c1d06e0b2 100644 --- a/source/include/rocprofiler-sdk/rocprofiler.h +++ b/source/include/rocprofiler-sdk/rocprofiler.h @@ -65,7 +65,7 @@ ROCPROFILER_EXTERN_C_FINI /** @} */ #include "rocprofiler-sdk/agent.h" -// #include "rocprofiler-sdk/agent_profile.h" +#include "rocprofiler-sdk/agent_profile.h" #include "rocprofiler-sdk/buffer.h" #include "rocprofiler-sdk/buffer_tracing.h" #include "rocprofiler-sdk/callback_tracing.h" diff --git a/source/lib/rocprofiler-sdk/CMakeLists.txt b/source/lib/rocprofiler-sdk/CMakeLists.txt index ffb104320a..e63cbbcb92 100644 --- a/source/lib/rocprofiler-sdk/CMakeLists.txt +++ b/source/lib/rocprofiler-sdk/CMakeLists.txt @@ -10,6 +10,7 @@ set(ROCPROFILER_LIB_SOURCES allocator.cpp buffer.cpp buffer_tracing.cpp + agent_profile.cpp callback_tracing.cpp context.cpp counters.cpp diff --git a/source/lib/rocprofiler-sdk/agent_profile.cpp b/source/lib/rocprofiler-sdk/agent_profile.cpp new file mode 100644 index 0000000000..7c484d6dc5 --- /dev/null +++ b/source/lib/rocprofiler-sdk/agent_profile.cpp @@ -0,0 +1,35 @@ +// MIT License +// +// Copyright (c) 2023 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 + +#include "lib/rocprofiler-sdk/counters/core.hpp" + +extern "C" { +rocprofiler_status_t ROCPROFILER_API +rocprofiler_configure_agent_profile_counting_service(rocprofiler_context_id_t context_id, + rocprofiler_buffer_id_t buffer_id, + rocprofiler_profile_config_id_t config_id) +{ + return rocprofiler::counters::configure_agent_collection(context_id, buffer_id, config_id); +} +} \ No newline at end of file diff --git a/source/lib/rocprofiler-sdk/context/context.hpp b/source/lib/rocprofiler-sdk/context/context.hpp index 81c2e813c6..2ebe9218b5 100644 --- a/source/lib/rocprofiler-sdk/context/context.hpp +++ b/source/lib/rocprofiler-sdk/context/context.hpp @@ -84,17 +84,29 @@ struct dispatch_counter_collection_service common::Synchronized enabled{false}; }; +struct agent_counter_collection_service +{ + std::shared_ptr profile; + rocprofiler_buffer_id_t buffer; + // A flag to state wether or not the counter set is currently enabled. This is primarily + // to protect against multithreaded calls to enable a context (and enabling already enabled + // counters). + std::atomic enabled{false}; +}; + struct context { // size is used to ensure that we never read past the end of the version - size_t size = 0; - uint64_t context_idx = 0; // context id - uint32_t client_idx = 0; // tool id - correlation_tracing_service correlation_tracer = {}; - std::unique_ptr callback_tracer = {}; - std::unique_ptr buffered_tracer = {}; - std::unique_ptr counter_collection = {}; - std::shared_ptr thread_trace = {}; + size_t size = 0; + uint64_t context_idx = 0; // context id + uint32_t client_idx = 0; // tool id + correlation_tracing_service correlation_tracer = {}; + std::unique_ptr callback_tracer = {}; + std::unique_ptr buffered_tracer = {}; + // Only one of counter collection/agent counter collection can exists in the ctx. + std::unique_ptr counter_collection = {}; + std::unique_ptr agent_counter_collection = {}; + std::shared_ptr thread_trace = {}; }; // set the client index needs to be called before allocate_context() diff --git a/source/lib/rocprofiler-sdk/counters/controller.cpp b/source/lib/rocprofiler-sdk/counters/controller.cpp index a162b1eca2..64447e3bff 100644 --- a/source/lib/rocprofiler-sdk/counters/controller.cpp +++ b/source/lib/rocprofiler-sdk/counters/controller.cpp @@ -27,6 +27,7 @@ #include #include +#include "lib/rocprofiler-sdk/buffer.hpp" #include "lib/rocprofiler-sdk/context/context.hpp" namespace rocprofiler @@ -62,11 +63,42 @@ CounterController::destroy_profile(uint64_t id) _configs.wlock([&](auto& data) { data.erase(id); }); } +rocprofiler_status_t +CounterController::configure_agent_collection(rocprofiler_context_id_t context_id, + rocprofiler_buffer_id_t buffer, + rocprofiler_profile_config_id_t config_id) +{ + auto* ctx_p = rocprofiler::context::get_mutable_registered_context(context_id); + if(!ctx_p) return ROCPROFILER_STATUS_ERROR_CONTEXT_INVALID; + + auto& ctx = *ctx_p; + + if(ctx.counter_collection) return ROCPROFILER_STATUS_ERROR_AGENT_DISPATCH_CONFLICT; + if(!rocprofiler::buffer::get_buffer(buffer.handle)) + { + return ROCPROFILER_STATUS_ERROR_BUFFER_NOT_FOUND; + } + auto cfg = get_profile_cfg(config_id); + + if(!cfg) return ROCPROFILER_STATUS_ERROR_PROFILE_NOT_FOUND; + + if(!ctx.agent_counter_collection) + { + ctx.agent_counter_collection = + std::make_unique(); + } + + ctx.agent_counter_collection->profile = cfg; + ctx.agent_counter_collection->buffer = buffer; + + return ROCPROFILER_STATUS_SUCCESS; +} + // Setup the counter collection service. counter_callback_info is created here // to contain the counters that need to be collected (specified in profile_id) and // the AQL packet generator for injecting packets. Note: the service is created // in the stop state. -bool +rocprofiler_status_t CounterController::configure_dispatch( rocprofiler_context_id_t context_id, rocprofiler_buffer_id_t buffer, @@ -76,10 +108,12 @@ CounterController::configure_dispatch( void* record_callback_args) { auto* ctx_p = rocprofiler::context::get_mutable_registered_context(context_id); - if(!ctx_p) return false; + if(!ctx_p) return ROCPROFILER_STATUS_ERROR_CONTEXT_INVALID; auto& ctx = *ctx_p; + if(ctx.agent_counter_collection) return ROCPROFILER_STATUS_ERROR_AGENT_DISPATCH_CONFLICT; + if(!ctx.counter_collection) { ctx.counter_collection = @@ -100,7 +134,7 @@ CounterController::configure_dispatch( cb.record_callback = record_callback; cb.record_callback_args = record_callback_args; - return true; + return ROCPROFILER_STATUS_SUCCESS; } std::shared_ptr diff --git a/source/lib/rocprofiler-sdk/counters/controller.hpp b/source/lib/rocprofiler-sdk/counters/controller.hpp index e8b496b4ce..2a29f1b1b0 100644 --- a/source/lib/rocprofiler-sdk/counters/controller.hpp +++ b/source/lib/rocprofiler-sdk/counters/controller.hpp @@ -78,14 +78,19 @@ public: // to contain the counters that need to be collected (specified in profile_id) and // the AQL packet generator for injecting packets. Note: the service is created // in the stop state. - static bool configure_dispatch(rocprofiler_context_id_t context_id, - rocprofiler_buffer_id_t buffer, - rocprofiler_profile_counting_dispatch_callback_t callback, - void* callback_args, - rocprofiler_profile_counting_record_callback_t record_callback, - void* record_callback_args); + static rocprofiler_status_t configure_dispatch( + rocprofiler_context_id_t context_id, + rocprofiler_buffer_id_t buffer, + rocprofiler_profile_counting_dispatch_callback_t callback, + void* callback_args, + rocprofiler_profile_counting_record_callback_t record_callback, + void* record_callback_args); std::shared_ptr get_profile_cfg(rocprofiler_profile_config_id_t id); + rocprofiler_status_t configure_agent_collection(rocprofiler_context_id_t context_id, + rocprofiler_buffer_id_t buffer, + rocprofiler_profile_config_id_t config_id); + private: rocprofiler::common::Synchronized>> _configs; diff --git a/source/lib/rocprofiler-sdk/counters/core.cpp b/source/lib/rocprofiler-sdk/counters/core.cpp index 452db26b93..9fca5a5bde 100644 --- a/source/lib/rocprofiler-sdk/counters/core.cpp +++ b/source/lib/rocprofiler-sdk/counters/core.cpp @@ -206,7 +206,15 @@ stop_context(const context::context* ctx) if(controller) controller->disable_serialization(); } -bool +rocprofiler_status_t +configure_agent_collection(rocprofiler_context_id_t context_id, + rocprofiler_buffer_id_t buffer_id, + rocprofiler_profile_config_id_t config_id) +{ + return get_controller().configure_agent_collection(context_id, buffer_id, config_id); +} + +rocprofiler_status_t configure_buffered_dispatch(rocprofiler_context_id_t context_id, rocprofiler_buffer_id_t buffer, rocprofiler_profile_counting_dispatch_callback_t callback, @@ -217,7 +225,7 @@ configure_buffered_dispatch(rocprofiler_context_id_t con context_id, buffer, callback, callback_args, nullptr, nullptr); } -bool +rocprofiler_status_t configure_callback_dispatch(rocprofiler_context_id_t context_id, rocprofiler_profile_counting_dispatch_callback_t callback, void* callback_data_args, diff --git a/source/lib/rocprofiler-sdk/counters/core.hpp b/source/lib/rocprofiler-sdk/counters/core.hpp index d86903efad..9e23badb76 100644 --- a/source/lib/rocprofiler-sdk/counters/core.hpp +++ b/source/lib/rocprofiler-sdk/counters/core.hpp @@ -80,19 +80,24 @@ create_counter_profile(std::shared_ptr&& void destroy_counter_profile(uint64_t id); -bool +rocprofiler_status_t configure_buffered_dispatch(rocprofiler_context_id_t context_id, rocprofiler_buffer_id_t buffer, rocprofiler_profile_counting_dispatch_callback_t callback, void* callback_args); -bool +rocprofiler_status_t configure_callback_dispatch(rocprofiler_context_id_t context_id, rocprofiler_profile_counting_dispatch_callback_t callback, void* callback_data_args, rocprofiler_profile_counting_record_callback_t record_callback, void* record_callback_args); +rocprofiler_status_t +configure_agent_collection(rocprofiler_context_id_t context_id, + rocprofiler_buffer_id_t buffer_id, + rocprofiler_profile_config_id_t config_id); + void start_context(const context::context*); diff --git a/source/lib/rocprofiler-sdk/counters/tests/core.cpp b/source/lib/rocprofiler-sdk/counters/tests/core.cpp index 694c79a578..04b5a8ad51 100644 --- a/source/lib/rocprofiler-sdk/counters/tests/core.cpp +++ b/source/lib/rocprofiler-sdk/counters/tests/core.cpp @@ -758,3 +758,54 @@ TEST(core, public_api_iterate_agents) EXPECT_TRUE(from_api.empty()); } } + +TEST(core, init_agent_collection) +{ + ASSERT_EQ(hsa_init(), HSA_STATUS_SUCCESS); + registration::init_logging(); + registration::set_init_status(-1); + context::push_client(1); + ROCPROFILER_CALL(rocprofiler_create_context(&get_client_ctx()), "context creation failed"); + auto agents = hsa::get_queue_controller()->get_supported_agents(); + + rocprofiler_buffer_id_t opt_buff_id = {.handle = 0}; + ROCPROFILER_CALL(rocprofiler_create_buffer(get_client_ctx(), + 500 * sizeof(size_t), + 500 * sizeof(size_t), + ROCPROFILER_BUFFER_POLICY_LOSSLESS, + null_buffered_callback, + nullptr, + &opt_buff_id), + "Could not create buffer"); + for(const auto& [_, agent] : agents) + { + auto metrics = findDeviceMetrics(agent, {}); + ASSERT_FALSE(metrics.empty()); + ASSERT_TRUE(agent.get_rocp_agent()); + for(auto& metric : metrics) + { + expected_dispatch expected = {}; + rocprofiler_counter_id_t id = {.handle = metric.id()}; + ROCPROFILER_CALL( + rocprofiler_create_profile_config(agent.get_rocp_agent()->id, &id, 1, &expected.id), + "Unable to create profile"); + + ROCPROFILER_CALL(rocprofiler_configure_agent_profile_counting_service( + get_client_ctx(), opt_buff_id, expected.id), + "Could not create agent collection"); + { + auto cfg = counters::get_profile_config(expected.id); + auto* ctx = rocprofiler::context::get_mutable_registered_context(get_client_ctx()); + ASSERT_TRUE(ctx); + ASSERT_TRUE(ctx->agent_counter_collection); + EXPECT_EQ(ctx->agent_counter_collection->profile, cfg); + EXPECT_EQ(ctx->agent_counter_collection->buffer.handle, opt_buff_id.handle); + } + ROCPROFILER_CALL(rocprofiler_destroy_profile_config(expected.id), + "Could not delete profile id"); + } + } + rocprofiler_destroy_buffer(opt_buff_id); + registration::set_init_status(1); + context::pop_client(1); +} \ No newline at end of file diff --git a/source/lib/rocprofiler-sdk/dispatch_profile.cpp b/source/lib/rocprofiler-sdk/dispatch_profile.cpp index b556b49d54..986858ec04 100644 --- a/source/lib/rocprofiler-sdk/dispatch_profile.cpp +++ b/source/lib/rocprofiler-sdk/dispatch_profile.cpp @@ -22,11 +22,7 @@ #include -#include "lib/rocprofiler-sdk/aql/helpers.hpp" #include "lib/rocprofiler-sdk/counters/core.hpp" -#include "lib/rocprofiler-sdk/counters/evaluate_ast.hpp" -#include "lib/rocprofiler-sdk/counters/metrics.hpp" -#include "lib/rocprofiler-sdk/hsa/agent_cache.hpp" extern "C" { /** @@ -49,9 +45,7 @@ rocprofiler_configure_buffered_dispatch_profile_counting_service( void* callback_data_args) { return rocprofiler::counters::configure_buffered_dispatch( - context_id, buffer_id, callback, callback_data_args) - ? ROCPROFILER_STATUS_SUCCESS - : ROCPROFILER_STATUS_ERROR; + context_id, buffer_id, callback, callback_data_args); } /** @@ -78,8 +72,6 @@ rocprofiler_configure_callback_dispatch_profile_counting_service( dispatch_callback, dispatch_callback_args, record_callback, - record_callback_args) - ? ROCPROFILER_STATUS_SUCCESS - : ROCPROFILER_STATUS_ERROR; + record_callback_args); } } diff --git a/source/lib/rocprofiler-sdk/hsa/queue_controller.cpp b/source/lib/rocprofiler-sdk/hsa/queue_controller.cpp index 282e3fa37e..dd5f09f73e 100644 --- a/source/lib/rocprofiler-sdk/hsa/queue_controller.cpp +++ b/source/lib/rocprofiler-sdk/hsa/queue_controller.cpp @@ -258,7 +258,7 @@ QueueController::init(CoreApiTable& core_table, AmdExtTable& ext_table) auto enable_intercepter = false; for(const auto& itr : context::get_registered_contexts()) { - constexpr auto expected_context_size = 184UL; + constexpr auto expected_context_size = 192UL; static_assert( sizeof(context::context) == expected_context_size + sizeof(std::shared_ptr), diff --git a/source/lib/rocprofiler-sdk/rocprofiler.cpp b/source/lib/rocprofiler-sdk/rocprofiler.cpp index a55ecd9525..72d040da4b 100644 --- a/source/lib/rocprofiler-sdk/rocprofiler.cpp +++ b/source/lib/rocprofiler-sdk/rocprofiler.cpp @@ -89,6 +89,12 @@ ROCPROFILER_STATUS_STRING( "AQL Profiler was not able to find event coordinates for defined counters") ROCPROFILER_STATUS_STRING(ROCPROFILER_STATUS_ERROR_INCOMPATIBLE_KERNEL, "A service depends on a newer version of KFD (amdgpu kernel driver)") +ROCPROFILER_STATUS_STRING(ROCPROFILER_STATUS_ERROR_PROFILE_NOT_FOUND, + "Could not find counter profile") +ROCPROFILER_STATUS_STRING(ROCPROFILER_STATUS_ERROR_AGENT_DISPATCH_CONFLICT, + "Cannot have both an agent counter collection and a dispatch counter " + "in the same context") + template const char* get_status_name(rocprofiler_status_t status, std::index_sequence)