Documentation Update For Counters (#246)

* Documentation Update * Minor fixes * source formatting (clang-format v11) (#265) Co-authored-by: bwelton <bwelton@users.noreply.github.com> --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: bwelton <bwelton@users.noreply.github.com>
2023-11-30 14:58:54 -08:00
@@ -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<rocprofiler_record_counter_t*>(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<std::string>                 counters_to_collect = {"SQ_WAVES"};
+    // Counters we want to collect (here its SQ_WAVES)
+    std::set<std::string> counters_to_collect = {"SQ_WAVES"};
+    // GPU Counter IDs
    std::vector<rocprofiler_counter_id_t> 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<rocprofiler_counter_id_t> 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;
 }

@@ -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<rocprofiler_record_counter_t*>(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<rocprofiler_counter_id_t> 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 "
@@ -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,
@@ -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
@@ -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);