From 42922ec85135643e00c54fc336e3690c7928ee08 Mon Sep 17 00:00:00 2001 From: Sohaib Nadeem Date: Wed, 5 Mar 2025 16:05:18 -0500 Subject: [PATCH] Fix hardware counter summary files not being generated after profiling (#124) - Register a cleanup function in tim::manager instance to write out data in counter storages - The counter_storage::write() calls in tool_fini happen after the storage is destroyed which is too late for the write to happen. - Adjust traits for counter_data_tracker - Add MIN, MAX, VAR, STDDEV columns - Remove DEPTH, UNITS, %SELF columns - Update "add_validation_test" to test for the existence of output file(s). - Added step to test perfetto output for `transpose-rocprofiler-sampling` and `transpose-rocprofiler-binary-rewrite` --------- Co-authored-by: David Galiffi --- .cmake-format.yaml | 1 + .../rocprof-sys/library/rocprofiler-sdk.cpp | 16 ---------------- .../library/rocprofiler-sdk/counters.cpp | 11 +++++++++-- .../library/rocprofiler-sdk/counters.hpp | 13 ++++++++++++- tests/rocprof-sys-rocm-tests.cmake | 19 +++++++++++++++---- tests/rocprof-sys-testing.cmake | 10 +++++++++- 6 files changed, 46 insertions(+), 24 deletions(-) diff --git a/.cmake-format.yaml b/.cmake-format.yaml index 5eab3b50b6..8ece7156ed 100644 --- a/.cmake-format.yaml +++ b/.cmake-format.yaml @@ -230,6 +230,7 @@ parse: PERFETTO_METRIC: '*' TIMEMORY_FILE: '*' TIMEMORY_METRIC: '*' + EXIST_FILES: '*' override_spec: {} vartags: [] proptags: [] diff --git a/source/lib/rocprof-sys/library/rocprofiler-sdk.cpp b/source/lib/rocprof-sys/library/rocprofiler-sdk.cpp index e7cd5da60f..b94d4ff84c 100644 --- a/source/lib/rocprof-sys/library/rocprofiler-sdk.cpp +++ b/source/lib/rocprof-sys/library/rocprofiler-sdk.cpp @@ -1179,22 +1179,6 @@ tool_fini(void* callback_data) if(get_counter_storage()) { - auto _storages = std::vector{}; - for(const auto& citr : *get_counter_storage()) - { - for(const auto& itr : citr.second) - _storages.emplace_back(&itr.second); - } - - std::sort(_storages.begin(), _storages.end(), - [](const counter_storage* lhs, const counter_storage* rhs) { - return *lhs < *rhs; - }); - - for(const auto* itr : _storages) - itr->write(); - _storages.clear(); - get_counter_storage()->clear(); delete get_counter_storage(); get_counter_storage() = nullptr; diff --git a/source/lib/rocprof-sys/library/rocprofiler-sdk/counters.cpp b/source/lib/rocprof-sys/library/rocprofiler-sdk/counters.cpp index 94977f9597..fe1142e36f 100644 --- a/source/lib/rocprof-sys/library/rocprofiler-sdk/counters.cpp +++ b/source/lib/rocprof-sys/library/rocprofiler-sdk/counters.cpp @@ -104,6 +104,12 @@ counter_storage::counter_storage(const client_data* _tool_data, uint64_t _devid, storage_name = JOIN('-', "rocprof", "device", device_id, _metric_name); storage = std::make_unique(tim::standalone_storage{}, index, storage_name); + tim::manager::instance()->add_cleanup( + storage_name + "cleanup", [storage_ptr = storage.get(), metric_name = metric_name, + metric_description = metric_description]() { + if(storage_ptr) + counter_storage::write(storage_ptr, metric_name, metric_description); + }); { constexpr auto _unit = ::perfetto::CounterTrack::Unit::UNIT_COUNT; track_name = JOIN(" ", "GPU", _metric_name, JOIN("", '[', device_id, ']')); @@ -124,7 +130,8 @@ counter_storage::operator()(const counter_event& _event, timing_interval _timing } void -counter_storage::write() const +counter_storage::write(counter_storage_type* storage, std::string metric_name, + std::string metric_description) { if(!trait::runtime_enabled::get()) { @@ -134,7 +141,7 @@ counter_storage::write() const return; } - operation::set_storage{}(storage.get()); + operation::set_storage{}(storage); counter_data_tracker::label() = metric_name; counter_data_tracker::description() = metric_description; storage->write(); diff --git a/source/lib/rocprof-sys/library/rocprofiler-sdk/counters.hpp b/source/lib/rocprof-sys/library/rocprofiler-sdk/counters.hpp index a4f145666a..e3af71b130 100644 --- a/source/lib/rocprof-sys/library/rocprofiler-sdk/counters.hpp +++ b/source/lib/rocprof-sys/library/rocprofiler-sdk/counters.hpp @@ -108,7 +108,8 @@ struct counter_storage void operator()(const counter_event& _event, timing_interval _timing, scope::config _scope = scope::get_default()) const; - void write() const; + static void write(counter_storage_type* storage, std::string metric_name, + std::string metric_description); }; } // namespace rocprofiler_sdk } // namespace rocprofsys @@ -166,3 +167,13 @@ struct get_storage<::rocprofsys::rocprofiler_sdk::counter_data_tracker> }; } // namespace operation } // namespace tim + +// Add columns for MIN, MAX, VAR, STDDEV +TIMEMORY_STATISTICS_TYPE(rocprofsys::rocprofiler_sdk::counter_data_tracker, double) +// Hide DEPTH, UNITS, and %SELF columns since they are not relevant +ROCPROFSYS_DEFINE_CONCRETE_TRAIT(report_depth, rocprofiler_sdk::counter_data_tracker, + false_type) +ROCPROFSYS_DEFINE_CONCRETE_TRAIT(report_units, rocprofiler_sdk::counter_data_tracker, + false_type) +ROCPROFSYS_DEFINE_CONCRETE_TRAIT(report_self, rocprofiler_sdk::counter_data_tracker, + false_type) diff --git a/tests/rocprof-sys-rocm-tests.cmake b/tests/rocprof-sys-rocm-tests.cmake index 50d4ef9584..68d07acdb9 100644 --- a/tests/rocprof-sys-rocm-tests.cmake +++ b/tests/rocprof-sys-rocm-tests.cmake @@ -62,10 +62,6 @@ rocprofiler_systems_add_test( REWRITE_FAIL_REGEX "0 instrumented loops in procedure transpose") if(ROCPROFSYS_USE_ROCM) - set(_ROCP_PASS_REGEX - "rocprof-device-0-GRBM_COUNT.txt(.*)rocprof-device-0-SQ_INSTS_VALU.txt(.*)rocprof-device-0-SQ_WAVES.txt(.*)rocprof-device-0-TA_TA_BUSY.txt(.*)" - ) - rocprofiler_systems_add_test( SKIP_BASELINE SKIP_RUNTIME NAME transpose-rocprofiler @@ -80,4 +76,19 @@ if(ROCPROFSYS_USE_ROCM) REWRITE_RUN_PASS_REGEX "${_ROCP_PASS_REGEX}" SAMPLING_PASS_REGEX "${_ROCP_PASS_REGEX}") + rocprofiler_systems_add_validation_test( + NAME transpose-rocprofiler-sampling + PERFETTO_FILE "perfetto-trace.proto" + ARGS --counter-names "TA_TA_BUSY" "SQ_WAVES" "GRBM_COUNT" "SQ_INSTS_VALU" -p + EXIST_FILES rocprof-device-0-GRBM_COUNT.txt rocprof-device-0-TA_TA_BUSY.txt + rocprof-device-0-SQ_INSTS_VALU.txt rocprof-device-0-SQ_WAVES.txt + LABELS "rocprofiler") + + rocprofiler_systems_add_validation_test( + NAME transpose-rocprofiler-binary-rewrite + PERFETTO_FILE "perfetto-trace.proto" + ARGS --counter-names "TA_TA_BUSY" "SQ_WAVES" "GRBM_COUNT" "SQ_INSTS_VALU" -p + EXIST_FILES rocprof-device-0-GRBM_COUNT.txt rocprof-device-0-TA_TA_BUSY.txt + rocprof-device-0-SQ_INSTS_VALU.txt rocprof-device-0-SQ_WAVES.txt + LABELS "rocprofiler") endif() diff --git a/tests/rocprof-sys-testing.cmake b/tests/rocprof-sys-testing.cmake index fafafec9dc..13741737e5 100644 --- a/tests/rocprof-sys-testing.cmake +++ b/tests/rocprof-sys-testing.cmake @@ -929,7 +929,7 @@ function(ROCPROFILER_SYSTEMS_ADD_VALIDATION_TEST) TEST "" "NAME;TIMEOUT;TIMEMORY_METRIC;TIMEMORY_FILE;PERFETTO_METRIC;PERFETTO_FILE" - "ENVIRONMENT;LABELS;PROPERTIES;PASS_REGEX;FAIL_REGEX;SKIP_REGEX;DEPENDS;ARGS" + "ENVIRONMENT;LABELS;PROPERTIES;PASS_REGEX;FAIL_REGEX;SKIP_REGEX;DEPENDS;EXIST_FILES;ARGS" ${ARGN}) if(NOT TEST "${TEST_NAME}") @@ -963,6 +963,14 @@ function(ROCPROFILER_SYSTEMS_ADD_VALIDATION_TEST) ) endif() + foreach(_FILE ${TEST_EXIST_FILES}) + add_test( + NAME validate-${TEST_NAME}-${_FILE}-exists + COMMAND test -e + ${PROJECT_BINARY_DIR}/rocprof-sys-tests-output/${TEST_NAME}/${_FILE} + WORKING_DIRECTORY ${PROJECT_BINARY_DIR}) + endforeach() + if(TEST_TIMEMORY_FILE) add_test( NAME validate-${TEST_NAME}-timemory