From 2d533ad91e02e41a30a640bafa57ee3684800a2b Mon Sep 17 00:00:00 2001 From: "Jonathan R. Madsen" Date: Wed, 27 Sep 2023 15:44:46 -0500 Subject: [PATCH] Fix set_tests_properties on some unittests (#90) * Fix set_tests_properties on some unittests - misspelled variable in two places * Update samples/api_buffered_tracing/client.cpp - output to file by default * Update samples/api_callback_tracing/client.cpp - output to file by default * Update lib/rocprofiler/registration.cpp - improve guards around initialize and finalize * Update lib/rocprofiler/tests/registration.cpp - test rocprofiler_iterate_callback_tracing_kind_names - validate number of kind names and number of HSA operation names * Update CI workflow and run-ci.py - change --coverage flag to support all/unittests/samples - samples mode excludes lib/common - samples mode appends -L samples - unittests mode appends -L unittests * Update samples/api_buffered_tracing/client.cpp - header include location fix --- .github/workflows/continuous_integration.yml | 10 ++--- samples/api_buffered_tracing/client.cpp | 41 +++++++++++++++--- samples/api_callback_tracing/client.cpp | 41 +++++++++++++++--- .../rocprofiler/counters/tests/CMakeLists.txt | 2 +- source/lib/rocprofiler/registration.cpp | 13 ++---- source/lib/rocprofiler/tests/registration.cpp | 42 +++++++++++++------ source/lib/tests/buffering/CMakeLists.txt | 2 +- source/scripts/run-ci.py | 26 +++++++++++- 8 files changed, 133 insertions(+), 44 deletions(-) diff --git a/.github/workflows/continuous_integration.yml b/.github/workflows/continuous_integration.yml index 3ea67a8d6c..0d3fcd55f9 100644 --- a/.github/workflows/continuous_integration.yml +++ b/.github/workflows/continuous_integration.yml @@ -178,7 +178,7 @@ jobs: --build-jobs 8 --site mi200 --gpu-targets ${{ env.GPU_LIST }} - --coverage + --coverage all -- -DCMAKE_BUILD_TYPE=${{ matrix.build-type }} -DPython3_EXECUTABLE=$(which python3) @@ -193,12 +193,10 @@ jobs: --build-jobs 8 --site mi200 --gpu-targets ${{ env.GPU_LIST }} - --coverage + --coverage unittests -- -DCMAKE_BUILD_TYPE=${{ matrix.build-type }} -DPython3_EXECUTABLE=$(which python3) - -- - -L unittests - name: Configure, Build, and Test (Samples Code Coverage) timeout-minutes: 30 @@ -210,12 +208,10 @@ jobs: --build-jobs 8 --site mi200 --gpu-targets ${{ env.GPU_LIST }} - --coverage + --coverage samples -- -DCMAKE_BUILD_TYPE=${{ matrix.build-type }} -DPython3_EXECUTABLE=$(which python3) - -- - -L samples sanitizers: strategy: diff --git a/samples/api_buffered_tracing/client.cpp b/samples/api_buffered_tracing/client.cpp index ba707289af..3e5420f452 100644 --- a/samples/api_buffered_tracing/client.cpp +++ b/samples/api_buffered_tracing/client.cpp @@ -46,6 +46,8 @@ #include #include #include +#include +#include #include #include #include @@ -87,17 +89,44 @@ print_call_stack(const call_stack_t& _call_stack) { namespace fs = ::std::filesystem; + auto ofname = std::string{"api_buffered_trace.log"}; + if(auto* eofname = getenv("ROCPROFILER_SAMPLE_OUTPUT_FILE")) ofname = eofname; + + std::ostream* ofs = nullptr; + auto cleanup = std::function{}; + + if(ofname == "stdout") + ofs = &std::cout; + else if(ofname == "stderr") + ofs = &std::cerr; + else + { + ofs = new std::ofstream{ofname}; + if(ofs && *ofs) + cleanup = [](std::ostream*& _os) { delete _os; }; + else + { + std::cerr << "Error outputting to " << ofname << ". Redirecting to stderr...\n"; + ofname = "stderr"; + ofs = &std::cerr; + } + } + + std::cout << "Outputting collected data to " << ofname << "...\n" << std::flush; + size_t n = 0; for(const auto& itr : _call_stack) { - std::clog << std::setw(2) << ++n << "/" << std::setw(2) << _call_stack.size() << " "; - std::clog << "[" << fs::path{itr.file}.filename() << ":" << itr.line << "] " - << std::setw(20) << std::left << itr.function; - if(!itr.context.empty()) std::clog << " :: " << itr.context; - std::clog << "\n"; + *ofs << std::setw(2) << ++n << "/" << std::setw(2) << _call_stack.size() << " "; + *ofs << "[" << fs::path{itr.file}.filename() << ":" << itr.line << "] " << std::setw(20) + << std::left << itr.function; + if(!itr.context.empty()) *ofs << " :: " << itr.context; + *ofs << "\n"; } - std::clog << std::flush; + *ofs << std::flush; + + if(cleanup) cleanup(ofs); } void diff --git a/samples/api_callback_tracing/client.cpp b/samples/api_callback_tracing/client.cpp index b8dcc064c6..957c9ecd21 100644 --- a/samples/api_callback_tracing/client.cpp +++ b/samples/api_callback_tracing/client.cpp @@ -42,6 +42,8 @@ #include #include #include +#include +#include #include #include #include @@ -81,17 +83,44 @@ print_call_stack(const call_stack_t& _call_stack) { namespace fs = ::std::filesystem; + auto ofname = std::string{"api_callback_trace.log"}; + if(auto* eofname = getenv("ROCPROFILER_SAMPLE_OUTPUT_FILE")) ofname = eofname; + + std::ostream* ofs = nullptr; + auto cleanup = std::function{}; + + if(ofname == "stdout") + ofs = &std::cout; + else if(ofname == "stderr") + ofs = &std::cerr; + else + { + ofs = new std::ofstream{ofname}; + if(ofs && *ofs) + cleanup = [](std::ostream*& _os) { delete _os; }; + else + { + std::cerr << "Error outputting to " << ofname << ". Redirecting to stderr...\n"; + ofname = "stderr"; + ofs = &std::cerr; + } + } + + std::cout << "Outputting collected data to " << ofname << "...\n" << std::flush; + size_t n = 0; for(const auto& itr : _call_stack) { - std::clog << std::setw(2) << ++n << "/" << std::setw(2) << _call_stack.size() << " "; - std::clog << "[" << fs::path{itr.file}.filename() << ":" << itr.line << "] " - << std::setw(20) << std::left << itr.function; - if(!itr.context.empty()) std::clog << " :: " << itr.context; - std::clog << "\n"; + *ofs << std::setw(2) << ++n << "/" << std::setw(2) << _call_stack.size() << " "; + *ofs << "[" << fs::path{itr.file}.filename() << ":" << itr.line << "] " << std::setw(20) + << std::left << itr.function; + if(!itr.context.empty()) *ofs << " :: " << itr.context; + *ofs << "\n"; } - std::clog << std::flush; + *ofs << std::flush; + + if(cleanup) cleanup(ofs); } void diff --git a/source/lib/rocprofiler/counters/tests/CMakeLists.txt b/source/lib/rocprofiler/counters/tests/CMakeLists.txt index 6a6f87a24c..5536d48c4e 100644 --- a/source/lib/rocprofiler/counters/tests/CMakeLists.txt +++ b/source/lib/rocprofiler/counters/tests/CMakeLists.txt @@ -18,7 +18,7 @@ target_link_libraries( gtest_add_tests( TARGET counter-test SOURCES ${ROCPROFILER_LIB_COUNTER_TEST_SOURCES} - TEST_LIST counter-test_TESTS + TEST_LIST counter-tests_TESTS WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}) set_tests_properties(${counter-tests_TESTS} PROPERTIES TIMEOUT 45 LABELS "unittests") diff --git a/source/lib/rocprofiler/registration.cpp b/source/lib/rocprofiler/registration.cpp index e8762b9e0a..0bff1efb67 100644 --- a/source/lib/rocprofiler/registration.cpp +++ b/source/lib/rocprofiler/registration.cpp @@ -437,9 +437,9 @@ set_fini_status(int v) void initialize() { - static auto _once = std::once_flag{}; - static auto _ready = std::atomic{false}; + if(get_init_status() != 0) return; + static auto _once = std::once_flag{}; std::call_once(_once, []() { // initialization is in process set_init_status(-1); @@ -450,19 +450,14 @@ initialize() internal_threading::initialize(); // initialization is no longer available set_init_status(1); - _ready.store(true, std::memory_order_release); }); - - if(!_ready.load(std::memory_order_acquire)) - { - while(!_ready.load(std::memory_order_acquire)) - std::this_thread::yield(); - } } void finalize() { + if(get_fini_status() != 0) return; + static auto _once = std::once_flag{}; std::call_once(_once, []() { set_fini_status(-1); diff --git a/source/lib/rocprofiler/tests/registration.cpp b/source/lib/rocprofiler/tests/registration.cpp index 271c6b28ef..c8065ab3d7 100644 --- a/source/lib/rocprofiler/tests/registration.cpp +++ b/source/lib/rocprofiler/tests/registration.cpp @@ -76,20 +76,33 @@ tool_tracing_callback(rocprofiler_callback_tracing_record_t record, void* client auto* cb_data = static_cast(client_data); - static auto name_map = [&record]() { - rocprofiler_callback_tracing_operation_name_cb_t cb = - [](rocprofiler_service_callback_tracing_kind_t kind, - uint32_t operation, - const char* operation_name, - void* data) { - auto mdata = *static_cast(data); - (*static_cast(data))[kind][operation] = operation_name; + static auto name_map = []() { + auto outer_cb = [](rocprofiler_service_callback_tracing_kind_t kind_v, + const char* /*kind_name*/, + void* data_v) { + auto inner_cb = [](rocprofiler_service_callback_tracing_kind_t kind, + uint32_t operation, + const char* operation_name, + void* data) { + auto& mdata = *static_cast(data); + mdata[kind][operation] = operation_name; return 0; }; + auto& mdata_v = *static_cast(data_v); + mdata_v.emplace(kind_v, std::unordered_map{}); + + rocprofiler_iterate_callback_tracing_kind_operation_names(kind_v, inner_cb, data_v); + return 0; + }; + auto tmp = name_map_t{}; - rocprofiler_iterate_callback_tracing_kind_operation_names( - record.kind, cb, static_cast(&tmp)); + rocprofiler_iterate_callback_tracing_kind_names(outer_cb, static_cast(&tmp)); + + EXPECT_EQ(tmp.size(), ROCPROFILER_SERVICE_CALLBACK_TRACING_LAST - 1); + EXPECT_EQ(tmp.at(ROCPROFILER_SERVICE_CALLBACK_TRACING_HSA_API).size(), + ROCPROFILER_HSA_API_ID_LAST); + return tmp; }(); @@ -139,8 +152,13 @@ tool_tracing_callback(rocprofiler_callback_tracing_record_t record, void* client ROCPROFILER_CALL(rocprofiler_iterate_callback_tracing_operation_args( record, info_data_cb, static_cast(&info_data_v)), "Failure iterating trace operation args"); - EXPECT_GT(info_data_v.num_args, 0) - << name_map[record.kind][record.operation] << info_data_v.arg_ss.str(); + if(record.kind == ROCPROFILER_SERVICE_CALLBACK_TRACING_HSA_API && + !(record.operation == ROCPROFILER_HSA_API_ID_hsa_init || + record.operation == ROCPROFILER_HSA_API_ID_hsa_shut_down)) + { + EXPECT_GT(info_data_v.num_args, 0) + << name_map[record.kind][record.operation] << info_data_v.arg_ss.str(); + } } void diff --git a/source/lib/tests/buffering/CMakeLists.txt b/source/lib/tests/buffering/CMakeLists.txt index 5101e5cb70..0d685882d6 100644 --- a/source/lib/tests/buffering/CMakeLists.txt +++ b/source/lib/tests/buffering/CMakeLists.txt @@ -17,7 +17,7 @@ target_link_libraries( gtest_add_tests( TARGET buffering-test SOURCES ${buffering_sources} - TEST_LIST buffering-test_TESTS + TEST_LIST buffering-tests_TESTS WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}) set_tests_properties(${buffering-tests_TESTS} PROPERTIES TIMEOUT 45 LABELS "unittests") diff --git a/source/scripts/run-ci.py b/source/scripts/run-ci.py index 9897925854..838d29ed13 100755 --- a/source/scripts/run-ci.py +++ b/source/scripts/run-ci.py @@ -85,6 +85,19 @@ def generate_custom(args, cmake_args, ctest_args): ] ) + codecov_exclude = [ + "/usr/.*", + "/opt/.*", + "external/.*", + "samples/.*", + "tests/.*", + ".*/details/.*", + ] + if args.coverage == "samples": + codecov_exclude += [".*/lib/common/.*"] + + COVERAGE_EXCLUDE = ";".join(codecov_exclude) + return f""" set(CTEST_PROJECT_NAME "{_PROJECT_NAME}") set(CTEST_NIGHTLY_START_TIME "05:00:00 UTC") @@ -105,7 +118,7 @@ def generate_custom(args, cmake_args, ctest_args): set(CTEST_CUSTOM_MAXIMUM_NUMBER_OF_ERRORS "100") set(CTEST_CUSTOM_MAXIMUM_NUMBER_OF_WARNINGS "100") set(CTEST_CUSTOM_MAXIMUM_PASSED_TEST_OUTPUT_SIZE "51200") - set(CTEST_CUSTOM_COVERAGE_EXCLUDE "/usr/.*;/opt/.*;.*external/.*;.*samples/.*;.*tests/.*;.*/details/.*") + set(CTEST_CUSTOM_COVERAGE_EXCLUDE "{COVERAGE_EXCLUDE}") set(CTEST_MEMORYCHECK_TYPE "{MEMCHECK_TYPE}") set(CTEST_MEMORYCHECK_SUPPRESSIONS_FILE "{MEMCHECK_SUPPRESSION_FILE}") @@ -224,7 +237,12 @@ def parse_cdash_args(args): "-q", "--quiet", help="Disable printing logs", action="store_true" ) parser.add_argument( - "-c", "--coverage", help="Enable code coverage", action="store_true" + "-c", + "--coverage", + help="Enable code coverage", + choices=("all", "unittests", "samples"), + type=str, + default=None, ) parser.add_argument( "-j", @@ -355,6 +373,10 @@ def parse_args(args=None): if cdash_args.coverage: cmake_args += ["-DROCPROFILER_BUILD_CODECOV=ON"] + if cdash_args.coverage == "samples": + ctest_args += ["-L", "samples"] + elif cdash_args.coverage == "unittests": + ctest_args += ["-L", "unittests"] if cdash_args.linter == "clang-tidy": cmake_args += ["-DROCPROFILER_ENABLE_CLANG_TIDY=ON"]