From 022d7abc29c6e5467eaab025c7f29653b19cd47d Mon Sep 17 00:00:00 2001 From: Benjamin Welton Date: Thu, 30 Nov 2023 14:58:54 -0800 Subject: [PATCH] Documentation Update For Counters (#246) * Documentation Update * Minor fixes * source formatting (clang-format v11) (#265) Co-authored-by: bwelton --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: bwelton --- samples/counter_collection/client.cpp | 22 +++++++- .../print_functional_counters.cpp | 16 ++++-- source/include/rocprofiler-sdk/counters.h | 55 ++++++++++++------- .../rocprofiler-sdk/dispatch_profile.h | 9 ++- .../include/rocprofiler-sdk/profile_config.h | 11 +++- 5 files changed, 85 insertions(+), 28 deletions(-) diff --git a/samples/counter_collection/client.cpp b/samples/counter_collection/client.cpp index 5a01c48b30..60fc71ee25 100644 --- a/samples/counter_collection/client.cpp +++ b/samples/counter_collection/client.cpp @@ -98,6 +98,11 @@ get_output_stream() return stream.get(); } +/** + * Buffer callback called when the buffer is full. rocprofiler_record_header_t + * can contain counter records as well as other records (such as tracing). These + * records need to be filtered based on the category type. + */ void buffered_callback(rocprofiler_context_id_t, rocprofiler_buffer_id_t, @@ -110,11 +115,13 @@ buffered_callback(rocprofiler_context_id_t, enter_count++; if(enter_count % 100 != 0) return; std::stringstream ss; + // Iterate through the returned records for(size_t i = 0; i < num_headers; ++i) { auto* header = headers[i]; if(header->category == ROCPROFILER_BUFFER_CATEGORY_COUNTERS && header->kind == 0) { + // Print the returned counter data. auto* record = static_cast(header->payload); ss << "(Id: " << record->id << " Value [D]: " << record->counter_value << " Corr_Id: " << record->corr_id.internal << "),"; @@ -124,6 +131,12 @@ buffered_callback(rocprofiler_context_id_t, *get_output_stream() << "[" << __FUNCTION__ << "] " << ss.str() << "\n"; } +/** + * Callback from rocprofiler when an kernel dispatch is enqueued into the HSA queue. + * rocprofiler_profile_config_id_t* is a return to specify what counters to collect + * for this dispatch (dispatch_packet). This example function creates a profile + * to collect the counter SQ_WAVES for all kernel dispatch packets. + */ void dispatch_callback(rocprofiler_queue_id_t /*queue_id*/, const rocprofiler_agent_t* agent, @@ -159,8 +172,12 @@ dispatch_callback(rocprofiler_queue_id_t /*queue_id*/, auto wlock = std::unique_lock{m_mutex}; if(search_cache()) return; - std::set counters_to_collect = {"SQ_WAVES"}; + // Counters we want to collect (here its SQ_WAVES) + std::set counters_to_collect = {"SQ_WAVES"}; + // GPU Counter IDs std::vector gpu_counters; + + // Iterate through the agents and get the counters available on that agent ROCPROFILER_CALL( rocprofiler_iterate_agent_supported_counters( *agent, @@ -177,6 +194,7 @@ dispatch_callback(rocprofiler_queue_id_t /*queue_id*/, "Could not fetch supported counters"); std::vector collect_counters; + // Look for the counters contained in counters_to_collect in gpu_counters for(auto& counter : gpu_counters) { const char* name; @@ -190,12 +208,14 @@ dispatch_callback(rocprofiler_queue_id_t /*queue_id*/, } } + // Create a colleciton profile for the counters rocprofiler_profile_config_id_t profile; ROCPROFILER_CALL(rocprofiler_create_profile_config( *agent, collect_counters.data(), collect_counters.size(), &profile), "Could not construct profile cfg"); profile_cache.emplace(agent->id.handle, profile); + // Return the profile to collect those counters for this dispatch *config = profile; } diff --git a/samples/counter_collection/print_functional_counters.cpp b/samples/counter_collection/print_functional_counters.cpp index 11fa16501c..2f3da33553 100644 --- a/samples/counter_collection/print_functional_counters.cpp +++ b/samples/counter_collection/print_functional_counters.cpp @@ -90,17 +90,16 @@ buffered_callback(rocprofiler_context_id_t, auto* header = headers[i]; if(header->category == ROCPROFILER_BUFFER_CATEGORY_COUNTERS && header->kind == 0) { + // Record the counters we have in the buffer and the number of instances of + // the counter we have seen. rocprofiler_counter_id_t counter; auto* record = static_cast(header->payload); rocprofiler_query_record_counter_id(record->id, &counter); - if(counter.handle == 517) - { - std::clog << "HERE"; - } seen_counters.emplace(counter.handle, 0).first->second++; } } + // Store these counts for post execution comparison for(const auto& [counter_id, instances] : seen_counters) { cap.captured.emplace(counter_id, 0).first->second += instances; @@ -118,6 +117,12 @@ dispatch_callback(rocprofiler_queue_id_t /*queue_id*/, auto& cap = *get_capture(); auto wlock = std::unique_lock{cap.m_mutex}; + /** + * Fetch all counters that are available for this agent if we haven't already. + * Each of these counters will be collected 1 by 1 for each dispatch until we + * have tried all counters. This requires the program to have at least counters + * number of kernel launches to test all counters. + */ if(cap.expected.empty()) { std::vector counters_needed; @@ -157,6 +162,7 @@ dispatch_callback(rocprofiler_queue_id_t /*queue_id*/, rocprofiler_profile_config_id_t profile; + // Select the next counter to collect. ROCPROFILER_CALL( rocprofiler_create_profile_config(*agent, &(cap.remaining.back()), 1, &profile), "Could not construct profile cfg"); @@ -208,6 +214,8 @@ tool_fini(void*) auto& cap = *get_capture(); auto wlock = std::unique_lock{cap.m_mutex}; + // Print out errors in counters that were not collected or had differences in instance + // count information. if(cap.captured.size() != cap.expected.size()) { std::clog << "[ERROR] Expected " << cap.expected.size() << " counters collected but got " diff --git a/source/include/rocprofiler-sdk/counters.h b/source/include/rocprofiler-sdk/counters.h index 65b5bbcbed..80bd77041a 100644 --- a/source/include/rocprofiler-sdk/counters.h +++ b/source/include/rocprofiler-sdk/counters.h @@ -35,25 +35,27 @@ ROCPROFILER_EXTERN_C_INIT */ /** - * @brief Query counter id information from record_id + * @brief Query counter id information from record_id. * * @param [in] id record id from rocprofiler_record_counter_t * @param [out] counter_id counter id associated with the record * @return ::rocprofiler_status_t + * @retval ROCPROFILER_STATUS_SUCCESS if id decoded */ rocprofiler_status_t ROCPROFILER_API rocprofiler_query_record_counter_id(rocprofiler_counter_instance_id_t id, rocprofiler_counter_id_t* counter_id) ROCPROFILER_NONNULL(2); /** - * @brief Query dimension position from record_id + * @brief Query dimension position from record_id. If the dimension does not exist + * in the counter, the return will be 0. * - * @param [in] id record id from rocprofiler_record_counter_t - * @param [in] dim dimension for which positional info is requested - * @param [out] pos value of the dimension in id. + * @param [in] id record id from @ref rocprofiler_record_counter_t + * @param [in] dim dimension for which positional info is requested (currently only + * 0 is allowed, i.e. flat array without dimension). + * @param [out] pos value of the dimension in id * @return ::rocprofiler_status_t - * - * TODO(aelwazir): rocprofiler_status_t types that can be returned + * @retval ROCPROFILER_STATUS_SUCCESS if dimension decoded */ rocprofiler_status_t ROCPROFILER_API rocprofiler_query_record_dimension_position(rocprofiler_counter_instance_id_t id, @@ -61,15 +63,16 @@ rocprofiler_query_record_dimension_position(rocprofiler_counter_instance_id_t i size_t* pos) ROCPROFILER_NONNULL(3); /** - * @brief Return information about the dimension for a specified counter + * @brief Return information about the dimension for a specified counter. This call + * is primary for future use not related to this alpha since the only dimension + * supported is 0 (flat array without dimension). * * @param [in] id counter id to query dimension info for. - * @param [in] dim dimension + * @param [in] dim dimension (currently only 0 is allowed) * @param [out] info info on the dimension (name, instance_size) * @return ::rocprofiler_status_t - * - * TODO(aelwazir): rocprofiler_status_t types that can be returned - * + * @retval ROCPROFILER_STATUS_SUCCESS if dimension exists + * @retval ROCPROFILER_STATUS_ERROR if the dimension does not */ rocprofiler_status_t ROCPROFILER_API rocprofiler_query_record_dimension_info(rocprofiler_counter_id_t id, @@ -78,16 +81,15 @@ rocprofiler_query_record_dimension_info(rocprofiler_counter_id_t id, ROCPROFILER_NONNULL(3); /** - * @brief Query Counter name as a literal string. + * @brief Query Counter name. Name is a pointer controlled by rocprofiler and + * should not be free'd or modified. * - * @param [in] counter_id @see rocprofiler_iterate_agent_supported_counters to get the available - * counter IDs + * @param [in] counter_id counter for which to get its name. * @param [out] name returns a pointer to the name of the counter * @param [out] size returns the size of the name returned * @return ::rocprofiler_status_t - * - * TODO(aelwazir): rocprofiler_status_t types that can be returned - * + * @retval ROCPROFILER_STATUS_SUCCESS if counter found + * @retval ROCPROFILER_STATUS_ERROR_COUNTER_NOT_FOUND if counter not found */ rocprofiler_status_t ROCPROFILER_API rocprofiler_query_counter_name(rocprofiler_counter_id_t counter_id, const char** name, size_t* size) @@ -104,14 +106,25 @@ rocprofiler_query_counter_name(rocprofiler_counter_id_t counter_id, const char** * @param [in] agent rocprofiler agent * @param [in] counter_id counter id (obtained from iterate_agent_supported_counters) * @param [out] instance_count number of instances the counter has - * @return rocprofiler_status_t + * @return ::rocprofiler_status_t + * @retval ROCPROFILER_STATUS_SUCCESS if counter found + * @retval ROCPROFILER_STATUS_ERROR_COUNTER_NOT_FOUND if counter not found */ rocprofiler_status_t ROCPROFILER_API rocprofiler_query_counter_instance_count(rocprofiler_agent_t agent, rocprofiler_counter_id_t counter_id, size_t* instance_count) ROCPROFILER_NONNULL(3); -// TODO(aelwazir): Ben to add a brief +/** + * @brief Callback that gives a list of counters available on an agent. The + * counters variable is owned by rocprofiler and should not be free'd. + * + * @param [in] counters An array of counters that are avialable on the agent + * @ref rocprofiler_iterate_agent_supported_counters was called on. + * @param [in] num_counters Number of counters contained in counters + * @param [in] user_data User data supplied by + * @ref rocprofiler_iterate_agent_supported_counters + */ typedef rocprofiler_status_t (*rocprofiler_available_counters_cb_t)( rocprofiler_counter_id_t* counters, size_t num_counters, @@ -124,6 +137,8 @@ typedef rocprofiler_status_t (*rocprofiler_available_counters_cb_t)( * @param [in] cb callback to caller to get counters * @param [in] user_data data to pass into the callback * @return ::rocprofiler_status_t + * @retval ROCPROFILER_STATUS_SUCCESS if counters found for agent + * @retval ROCPROFILER_STATUS_ERROR if no counters found for agent */ rocprofiler_status_t ROCPROFILER_API rocprofiler_iterate_agent_supported_counters(rocprofiler_agent_t agent, diff --git a/source/include/rocprofiler-sdk/dispatch_profile.h b/source/include/rocprofiler-sdk/dispatch_profile.h index 4f575fde0a..dd7cce0554 100644 --- a/source/include/rocprofiler-sdk/dispatch_profile.h +++ b/source/include/rocprofiler-sdk/dispatch_profile.h @@ -64,9 +64,14 @@ typedef void (*rocprofiler_profile_counting_dispatch_callback_t)( * Collects the counters in dispatch packets and stores them * in a buffer with @p buffer_id. The buffer may contain packets from more than * one dispatch (denoted by correlation id). Will trigger the - * callback based on the parameters setup in @p buffer_id. + * callback based on the parameters setup in buffer_id_t. * - * // TODO(aelwazir): Should this be per agent? + * Interface is up for comment as to whether restrictions + * on agent should be made here (limiting the CB based on agent) + * or if the restriction should be performed by the tool in + * @ref rocprofiler_profile_counting_dispatch_callback_t (i.e. + * tool code checking the agent param to see if they want to profile + * it). * * @param [in] context_id context id * @param [in] buffer_id id of the buffer to use for the counting service diff --git a/source/include/rocprofiler-sdk/profile_config.h b/source/include/rocprofiler-sdk/profile_config.h index 5cbeccee45..c265d3c93a 100644 --- a/source/include/rocprofiler-sdk/profile_config.h +++ b/source/include/rocprofiler-sdk/profile_config.h @@ -36,13 +36,20 @@ ROCPROFILER_EXTERN_C_INIT */ /** - * @brief Create Profile Configuration. + * @brief Create Profile Configuration. A profile is bound to an agent but can + * be used across many contexts. The profile has a fixed set of counters + * that are collected (and specified by counter_list). The available + * counters for an agent can be queried using + * @ref rocprofiler_iterate_agent_supported_counters. * * @param [in] agent Agent identifier * @param [in] counters_list List of GPU counters * @param [in] counters_count Size of counters list * @param [out] config_id Identifier for GPU counters group * @return ::rocprofiler_status_t + * @retval ROCPROFILER_STATUS_SUCCESS if profile created + * @retval ROCPROFILER_STATUS_ERROR if profile could not be created + * */ rocprofiler_status_t ROCPROFILER_API rocprofiler_create_profile_config(rocprofiler_agent_t agent, @@ -56,6 +63,8 @@ rocprofiler_create_profile_config(rocprofiler_agent_t agent, * * @param [in] config_id * @return ::rocprofiler_status_t + * @retval ROCPROFILER_STATUS_SUCCESS if profile destroyed + * @retval ROCPROFILER_STATUS_ERROR if profile could not be destroyed */ rocprofiler_status_t ROCPROFILER_API rocprofiler_destroy_profile_config(rocprofiler_profile_config_id_t config_id);