From 04e243aadce3cc153588cdeb1482e634b3a31354 Mon Sep 17 00:00:00 2001 From: "Jonathan R. Madsen" Date: Tue, 30 Jan 2024 14:52:17 -0600 Subject: [PATCH] Kernel Tracing Fix (#439) * Update lib/rocprofiler-sdk/hsa/queue.cpp - switch using the kernel_pkt.kernel_dispatch.completion_signal instead of interrupt signal for getting the dispatch time * Update tests/kernel-tracing/validate.py - add verification of total runtime collected in test_timestamps - the sum of the runtime of all the kernels in reproducible-runtime should be ~1 sec +/- 10% * Remove include/rocprofiler-sdk/rocprofiler_plugin.h * Update CI workflow - update actions/cache@v3 -> v4 - actions/cache/save@v3 -> v4 - thollander/actions-comment-pull-request@v2 -> v2.4.3 * Update pytest.ini - change default options to one that is more verbose * Update tests/kernel-tracing/CMakeLists.txt - skip test_total_runtime when Address or Thread Sanitizer enabled - overhead skews the results * Update tests/kernel-tracing/validate.py - separate test_total_runtime test [ROCm/rocprofiler-sdk commit: aaff4976d2bc6078169a6d0984572335a0b7cb14] --- .../workflows/continuous_integration.yml | 6 +- .../include/rocprofiler-sdk/CMakeLists.txt | 1 - .../rocprofiler-sdk/rocprofiler_plugin.h | 136 ------------------ .../source/lib/rocprofiler-sdk/hsa/queue.cpp | 10 +- .../tests/async-copy-tracing/pytest.ini | 2 +- .../tests/kernel-tracing/CMakeLists.txt | 12 +- .../tests/kernel-tracing/pytest.ini | 2 +- .../tests/kernel-tracing/validate.py | 14 ++ .../rocprofv3/counter-collection/pytest.ini | 2 +- .../rocprofv3/tracing-plus-cc/pytest.ini | 2 +- 10 files changed, 35 insertions(+), 152 deletions(-) delete mode 100644 projects/rocprofiler-sdk/source/include/rocprofiler-sdk/rocprofiler_plugin.h diff --git a/projects/rocprofiler-sdk/.github/workflows/continuous_integration.yml b/projects/rocprofiler-sdk/.github/workflows/continuous_integration.yml index 948a5eb1ab..21eaae0ccb 100644 --- a/projects/rocprofiler-sdk/.github/workflows/continuous_integration.yml +++ b/projects/rocprofiler-sdk/.github/workflows/continuous_integration.yml @@ -226,7 +226,7 @@ jobs: - name: Load Existing XML Code Coverage if: github.event_name == 'pull_request' id: load-coverage - uses: actions/cache@v3 + uses: actions/cache@v4 with: key: ${{ github.event.pull_request.base.sha }}-codecov path: .codecov/** @@ -308,7 +308,7 @@ jobs: - name: Save XML Code Coverage id: save-coverage - uses: actions/cache/save@v3 + uses: actions/cache/save@v4 with: key: ${{ github.sha }}-codecov path: | @@ -355,7 +355,7 @@ jobs: - name: Write Code Coverage Comment if: github.event_name == 'pull_request' timeout-minutes: 5 - uses: thollander/actions-comment-pull-request@v2 + uses: thollander/actions-comment-pull-request@v2.4.3 with: comment_tag: codecov-report filePath: .codecov/report.md diff --git a/projects/rocprofiler-sdk/source/include/rocprofiler-sdk/CMakeLists.txt b/projects/rocprofiler-sdk/source/include/rocprofiler-sdk/CMakeLists.txt index a5414b7974..7a65469918 100644 --- a/projects/rocprofiler-sdk/source/include/rocprofiler-sdk/CMakeLists.txt +++ b/projects/rocprofiler-sdk/source/include/rocprofiler-sdk/CMakeLists.txt @@ -9,7 +9,6 @@ configure_file(${CMAKE_CURRENT_LIST_DIR}/version.h.in set(ROCPROFILER_HEADER_FILES # core headers rocprofiler.h - rocprofiler_plugin.h # secondary headers agent.h agent_profile.h diff --git a/projects/rocprofiler-sdk/source/include/rocprofiler-sdk/rocprofiler_plugin.h b/projects/rocprofiler-sdk/source/include/rocprofiler-sdk/rocprofiler_plugin.h deleted file mode 100644 index 86f5fd3459..0000000000 --- a/projects/rocprofiler-sdk/source/include/rocprofiler-sdk/rocprofiler_plugin.h +++ /dev/null @@ -1,136 +0,0 @@ -// 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. - -/** @section rocprofiler_plugin_api ROCProfiler Plugin API - * - * The ROCProfiler Plugin API is used by the ROCProfiler Tool to output all - * profiling information. Different implementations of the ROCProfiler Plugin - * API can be developed that output the data in different formats. The - * ROCProfiler Tool can be configured to load a specific library that supports - * the user desired format. - * - * The API is not thread safe. It is the responsibility of the ROCProfiler Tool - * to ensure the operations are synchronized and not called concurrently. There - * is no requirement for the ROCProfiler Tool to report trace data in any - * specific order. If the format supported by plugin requires specific - * ordering, it is the responsibility of the plugin implementation to perform - * any necessary sorting. - */ - -/** - * @file - * ROCProfiler Tool Plugin API interface. - */ - -#pragma once - -#include "rocprofiler-sdk/rocprofiler.h" - -#include - -ROCPROFILER_EXTERN_C_INIT - -/** - * @defgroup ROCPROFILER_PLUGINS ROCProfiler Plugin API Specification - * @{ - */ - -/** - * @defgroup INITIALIZATION_GROUP Initialization and Finalization - * @brief The ROCProfiler Plugin API must be initialized before using any of the - * operations to report trace data, and finalized after the last trace data has - * been reported. - * @ingroup ROCPROFILER_PLUGINS - * - * @{ - */ - -/** - * @brief Initialize plugin. Must be called before any other operation. - * - * @param[in] rocprofiler_major_version The major version of the ROCProfiler API - * being used by the ROCProfiler Tool. An error is reported if this does not - * match the major version of the ROCProfiler API used to build the plugin - * library. This ensures compatibility of the trace data format. - * @param[in] rocprofiler_minor_version The minor version of the ROCProfiler API - * being used by the ROCProfiler Tool. An error is reported if the - * @p rocprofiler_major_version matches and this is greater than the minor - * version of the ROCProfiler API used to build the plugin library. This ensures - * compatibility of the trace data format. - * @param[in] data Pointer to the data passed to the ROCProfiler Plugin by the tool - * @return Returns 0 on success and -1 on error. - */ -ROCPROFILER_EXPORT int -rocprofiler_plugin_initialize(uint32_t rocprofiler_major_version, - uint32_t rocprofiler_minor_version, - void* data); - -/** - * @brief Finalize plugin. - * This must be called after ::rocprofiler_plugin_initialize and after all - * profiling data has been reported by - * rocprofiler_plugin_write_kernel_records - */ -ROCPROFILER_EXPORT void -rocprofiler_plugin_finalize(); - -/** @} */ - -/** - * @defgroup profiling_record_write_functions Profiling data reporting - * @brief Operations to output profiling data. - * @ingroup ROCPROFILER_PLUGINS - * - * @{ - */ - -// TODO(aelwazir): Recheck wording of the description - -/** - * Report Buffer Records. - * - * @param[in] context_id context ID - * @param[in] buffer_id Buffer ID - * @param[in] headers Array of ::rocprofiler_record_header_t - * @param[in] num_headers Number of ::rocprofiler_record_header_t entries in array - * @return Returns 0 on success and -1 on error. - */ -ROCPROFILER_EXPORT int -rocprofiler_plugin_write_buffer_records(rocprofiler_context_id_t context_id, - rocprofiler_buffer_id_t buffer_id, - rocprofiler_record_header_t** headers, - size_t num_headers); - -/** - * @brief Report Synchronous Record. - * - * @param[in] record Synchronous Tracer record. - * @return Returns 0 on success and -1 on error. - */ - -ROCPROFILER_EXPORT int -rocprofiler_plugin_write_record(rocprofiler_record_header_t record); - -/** @} */ -/** @} */ - -ROCPROFILER_EXTERN_C_FINI diff --git a/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/hsa/queue.cpp b/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/hsa/queue.cpp index 581cbfc625..a8a436fe1b 100644 --- a/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/hsa/queue.cpp +++ b/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/hsa/queue.cpp @@ -89,11 +89,11 @@ AsyncSignalHandler(hsa_signal_value_t /*signal_v*/, void* data) if(!ctxs.empty()) { // only do the following work if there are contexts that require this info - const auto* _rocp_agent = queue_info_session.rocp_agent; - auto _hsa_agent = queue_info_session.hsa_agent; - auto _queue_id = queue_info_session.queue_id; - auto _signal = queue_info_session.interrupt_signal; - auto _kern_id = queue_info_session.kernel_id; + const auto* _rocp_agent = queue_info_session.rocp_agent; + auto _hsa_agent = queue_info_session.hsa_agent; + auto _queue_id = queue_info_session.queue_id; + auto _signal = queue_info_session.kernel_pkt.kernel_dispatch.completion_signal; + auto _kern_id = queue_info_session.kernel_id; const auto& _extern_corr_ids = queue_info_session.extern_corr_ids; auto dispatch_time = hsa_amd_profiling_dispatch_time_t{}; diff --git a/projects/rocprofiler-sdk/tests/async-copy-tracing/pytest.ini b/projects/rocprofiler-sdk/tests/async-copy-tracing/pytest.ini index 899bc38408..eb3f82f5cd 100644 --- a/projects/rocprofiler-sdk/tests/async-copy-tracing/pytest.ini +++ b/projects/rocprofiler-sdk/tests/async-copy-tracing/pytest.ini @@ -1,4 +1,4 @@ [pytest] -addopts = --durations=20 -ras -vv +addopts = --durations=20 -rA -s -vv testpaths = validate.py diff --git a/projects/rocprofiler-sdk/tests/kernel-tracing/CMakeLists.txt b/projects/rocprofiler-sdk/tests/kernel-tracing/CMakeLists.txt index 052b9bde07..6cfc0c1876 100644 --- a/projects/rocprofiler-sdk/tests/kernel-tracing/CMakeLists.txt +++ b/projects/rocprofiler-sdk/tests/kernel-tracing/CMakeLists.txt @@ -10,6 +10,11 @@ project( find_package(rocprofiler-sdk REQUIRED) +set(PYTEST_ARGS) +if(ROCPROFILER_MEMCHECK MATCHES "(Address|Thread)Sanitizer") + set(PYTEST_ARGS -k "not test_total_runtime") +endif() + if(ROCPROFILER_MEMCHECK_PRELOAD_ENV) set(PRELOAD_ENV "${ROCPROFILER_MEMCHECK_PRELOAD_ENV}:$") @@ -36,9 +41,10 @@ foreach(FILENAME validate.py pytest.ini conftest.py) ${CMAKE_CURRENT_BINARY_DIR}/${FILENAME} COPYONLY) endforeach() -add_test(NAME test-kernel-tracing-validate - COMMAND ${Python3_EXECUTABLE} ${CMAKE_CURRENT_BINARY_DIR}/validate.py --input - ${CMAKE_CURRENT_BINARY_DIR}/kernel-tracing-test.json) +add_test( + NAME test-kernel-tracing-validate + COMMAND ${Python3_EXECUTABLE} ${CMAKE_CURRENT_BINARY_DIR}/validate.py ${PYTEST_ARGS} + --input ${CMAKE_CURRENT_BINARY_DIR}/kernel-tracing-test.json) set_tests_properties( test-kernel-tracing-validate diff --git a/projects/rocprofiler-sdk/tests/kernel-tracing/pytest.ini b/projects/rocprofiler-sdk/tests/kernel-tracing/pytest.ini index 899bc38408..eb3f82f5cd 100644 --- a/projects/rocprofiler-sdk/tests/kernel-tracing/pytest.ini +++ b/projects/rocprofiler-sdk/tests/kernel-tracing/pytest.ini @@ -1,4 +1,4 @@ [pytest] -addopts = --durations=20 -ras -vv +addopts = --durations=20 -rA -s -vv testpaths = validate.py diff --git a/projects/rocprofiler-sdk/tests/kernel-tracing/validate.py b/projects/rocprofiler-sdk/tests/kernel-tracing/validate.py index 01c68d056a..08c67553a3 100644 --- a/projects/rocprofiler-sdk/tests/kernel-tracing/validate.py +++ b/projects/rocprofiler-sdk/tests/kernel-tracing/validate.py @@ -74,6 +74,20 @@ def test_timestamps(input_data): assert api_end <= itr["end_timestamp"] +def test_total_runtime(input_data): + sdk_data = input_data["rocprofiler-sdk-json-tool"] + + runtime_data = [] + for itr in sdk_data["buffer_records"]["kernel_dispatches"]: + elapsed = itr["end_timestamp"] - itr["start_timestamp"] + runtime_data.append(elapsed) # in nanoseconds + + expected_runtime = 1.0e3 # one second in milliseconds + + assert (sum(runtime_data) * 1.0e-6) >= (0.9 * expected_runtime) + assert (sum(runtime_data) * 1.0e-6) <= (1.1 * expected_runtime) + + def test_internal_correlation_ids(input_data): data = input_data sdk_data = data["rocprofiler-sdk-json-tool"] diff --git a/projects/rocprofiler-sdk/tests/rocprofv3/counter-collection/pytest.ini b/projects/rocprofiler-sdk/tests/rocprofv3/counter-collection/pytest.ini index 75dce2e9c2..e7bc653ac4 100644 --- a/projects/rocprofiler-sdk/tests/rocprofv3/counter-collection/pytest.ini +++ b/projects/rocprofiler-sdk/tests/rocprofv3/counter-collection/pytest.ini @@ -1,5 +1,5 @@ [pytest] -addopts = --durations=20 -ras -vv +addopts = --durations=20 -rA -s -vv testpaths = input1/validate.py input2/validate.py diff --git a/projects/rocprofiler-sdk/tests/rocprofv3/tracing-plus-cc/pytest.ini b/projects/rocprofiler-sdk/tests/rocprofv3/tracing-plus-cc/pytest.ini index 899bc38408..eb3f82f5cd 100644 --- a/projects/rocprofiler-sdk/tests/rocprofv3/tracing-plus-cc/pytest.ini +++ b/projects/rocprofiler-sdk/tests/rocprofv3/tracing-plus-cc/pytest.ini @@ -1,4 +1,4 @@ [pytest] -addopts = --durations=20 -ras -vv +addopts = --durations=20 -rA -s -vv testpaths = validate.py