From 6fe6bcc8328e09634cc7b9e444dbfde46e732b73 Mon Sep 17 00:00:00 2001 From: "Jonathan R. Madsen" Date: Tue, 2 Apr 2024 03:05:11 -0500 Subject: [PATCH] Stop all client contexts prior to finalization (#721) * Stop all client contexts prior to finalization * Update lib/common/container/static_vector.hpp - improve emplace_back for non-{move,copy}-assignable object * Update samples/intercept_table/client.cpp - improve robustness against static object destruction * Update lib/rocprofiler-sdk/context/context.cpp - change storage of registered context array - stable_vector of optional contexts - common::static_object wrapper around stable_vector * Update samples/intercept_table/client.cpp - use variable template for underlying function pointer [ROCm/rocprofiler-sdk commit: 939e23e9d11c48c8bae8bcc8b68026ae913a4da6] --- .../samples/intercept_table/client.cpp | 41 +++++++---- .../lib/common/container/static_vector.hpp | 3 +- .../lib/rocprofiler-sdk/context/context.cpp | 69 ++++++++++++------- .../lib/rocprofiler-sdk/context/context.hpp | 15 ++-- .../lib/rocprofiler-sdk/registration.cpp | 1 + .../tests/external_correlation.cpp | 12 ++-- .../rocprofiler-sdk/tests/intercept_table.cpp | 13 ++-- .../rocprofiler-sdk/tests/registration.cpp | 12 ++-- .../lib/rocprofiler-sdk/tests/roctx.cpp | 12 ++-- 9 files changed, 115 insertions(+), 63 deletions(-) diff --git a/projects/rocprofiler-sdk/samples/intercept_table/client.cpp b/projects/rocprofiler-sdk/samples/intercept_table/client.cpp index 1094430791..eb2dfc32b9 100644 --- a/projects/rocprofiler-sdk/samples/intercept_table/client.cpp +++ b/projects/rocprofiler-sdk/samples/intercept_table/client.cpp @@ -76,8 +76,8 @@ using callback_kind_operation_names_t = std::map>; using wrap_count_t = std::pair; -rocprofiler_client_id_t* client_id = nullptr; -std::map client_wrap_data = {}; +rocprofiler_client_id_t* client_id = nullptr; +auto* client_wrap_data = new std::map{}; void print_call_stack(const call_stack_t& _call_stack) @@ -113,7 +113,7 @@ print_call_stack(const call_stack_t& _call_stack) { *ofs << std::left << std::setw(2) << ++n << "/" << std::setw(2) << _call_stack.size() << " [" << common::fs::path{itr.file}.filename() << ":" << itr.line << "] " - << std::setw(20) << itr.function; + << std::setw(41) << itr.function; if(!itr.context.empty()) *ofs << " :: " << itr.context; *ofs << "\n"; } @@ -131,7 +131,7 @@ tool_fini(void* tool_data) auto* _call_stack = static_cast(tool_data); size_t wrapped_count = 0; - for(const auto& itr : client_wrap_data) + for(const auto& itr : *client_wrap_data) { auto src_loc = itr.second.first; src_loc.context += "call_count=" + std::to_string(itr.second.second); @@ -144,6 +144,7 @@ tool_fini(void* tool_data) print_call_stack(*_call_stack); delete _call_stack; + delete client_wrap_data; if(wrapped_count == 0) { @@ -151,22 +152,32 @@ tool_fini(void* tool_data) } } +template +RetT (*underlying_function)(Args...) = nullptr; + +template +RetT +get_wrapper_function(Args... args) +{ + if(client_wrap_data->at(Idx).second == 0) + std::clog << "First invocation of wrapped function: '" + << client_wrap_data->at(Idx).first.function << "'...\n" + << std::flush; + + client_wrap_data->at(Idx).second += 1; + if(underlying_function) + return underlying_function(args...); + if constexpr(!std::is_void::value) return RetT{}; +} + template auto generate_wrapper(const char* name, RetT (*func)(Args...)) { - using functor_type = RetT (*)(Args...); + client_wrap_data->emplace(Idx, wrap_count_t{source_location{name, __FILE__, __LINE__, ""}, 0}); - client_wrap_data.emplace(Idx, wrap_count_t{source_location{name, __FILE__, __LINE__, ""}, 0}); - - static functor_type underlying_func = func; - static functor_type wrapped_func = [](Args... args) -> RetT { - client_wrap_data.at(Idx).second += 1; - if(underlying_func) return underlying_func(args...); - if constexpr(!std::is_void::value) return RetT{}; - }; - - return wrapped_func; + underlying_function = func; + return &get_wrapper_function; } #define GENERATE_WRAPPER(TABLE, FUNC) \ diff --git a/projects/rocprofiler-sdk/source/lib/common/container/static_vector.hpp b/projects/rocprofiler-sdk/source/lib/common/container/static_vector.hpp index ee8aa30800..78bae501be 100644 --- a/projects/rocprofiler-sdk/source/lib/common/container/static_vector.hpp +++ b/projects/rocprofiler-sdk/source/lib/common/container/static_vector.hpp @@ -30,6 +30,7 @@ #include #include #include +#include namespace rocprofiler { @@ -208,7 +209,7 @@ static_vector::emplace_back(Args&&... _v) else m_data[_idx] = Tp{std::forward(_v)...}; } - else + else if constexpr(std::is_move_assignable::value || std::is_copy_assignable::value) { m_data[_idx] = {}; } diff --git a/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/context/context.cpp b/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/context/context.cpp index 8a7dad7500..842a4ae013 100644 --- a/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/context/context.cpp +++ b/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/context/context.cpp @@ -50,9 +50,8 @@ namespace context { namespace { -using reserve_size_t = common::container::reserve_size; -using unique_context_vec_t = - common::container::stable_vector, 8>; +using reserve_size_t = common::container::reserve_size; +using stable_context_vec_t = common::container::stable_vector, 8>; using active_context_vec_t = common::container::stable_vector, 8>; constexpr auto invalid_client_idx = std::numeric_limits::max(); @@ -83,10 +82,11 @@ get_client_index() return _v; } -unique_context_vec_t& +stable_context_vec_t*& get_registered_contexts_impl() { - static auto _v = unique_context_vec_t{reserve_size_t{unique_context_vec_t::chunk_size}}; + static auto*& _v = common::static_object::construct( + reserve_size_t{stable_context_vec_t::chunk_size}); return _v; } @@ -100,14 +100,7 @@ get_num_active_contexts() active_context_vec_t& get_active_contexts_impl() { - static auto* _v = new active_context_vec_t{reserve_size_t{active_context_vec_t::chunk_size}}; - static auto _once = std::once_flag{}; - std::call_once(_once, std::atexit, []() { - for(auto& itr : *_v) - { - itr.store(nullptr); - } - }); + static auto* _v = new active_context_vec_t{reserve_size_t{active_context_vec_t::chunk_size}}; return *_v; } @@ -243,13 +236,16 @@ context_array_t& get_registered_contexts(context_array_t& data, context_filter_t filter) { data.clear(); - auto num_ctx = get_registered_contexts_impl().size(); + + if(!get_registered_contexts_impl()) return data; + + auto num_ctx = get_registered_contexts_impl()->size(); if(num_ctx <= 0) return data; data.reserve(num_ctx); - for(auto& itr : get_registered_contexts_impl()) + for(auto& itr : *get_registered_contexts_impl()) { - const auto* ctx = itr.get(); + const auto* ctx = &itr.value(); if(ctx) { if(!filter || (filter && filter(ctx))) data.emplace_back(ctx); @@ -332,15 +328,15 @@ allocate_context() auto _lk = std::unique_lock{get_contexts_mutex()}; // initial context identifier number - auto _idx = get_registered_contexts_impl().size() + get_contexts_offset(); + auto _idx = get_registered_contexts_impl()->size() + get_contexts_offset(); // make space in registered - get_registered_contexts_impl().emplace_back(nullptr); + get_registered_contexts_impl()->emplace_back(); // create an entry in the registered - auto& _cfg_v = get_registered_contexts_impl().back(); - _cfg_v = allocator::make_unique_static(); - auto* _cfg = _cfg_v.get(); + auto& _cfg_v = get_registered_contexts_impl()->back(); + _cfg_v.emplace(); + auto* _cfg = &_cfg_v.value(); // ... if(!_cfg) return std::nullopt; @@ -361,8 +357,8 @@ get_mutable_registered_context(rocprofiler_context_id_t id) { if(id.handle < get_contexts_offset()) return nullptr; auto _idx = id.handle - get_contexts_offset(); - if(_idx >= get_registered_contexts_impl().size()) return nullptr; - return get_registered_contexts_impl().at(_idx).get(); + if(_idx >= get_registered_contexts_impl()->size()) return nullptr; + return &get_registered_contexts_impl()->at(_idx).value(); } const context* @@ -382,7 +378,9 @@ start_context(rocprofiler_context_id_t context_id) { if(context_id.handle < get_contexts_offset()) return ROCPROFILER_STATUS_ERROR_CONTEXT_NOT_FOUND; - if((context_id.handle - get_contexts_offset()) >= get_registered_contexts_impl().size()) + if(!get_registered_contexts_impl()) return ROCPROFILER_STATUS_ERROR_CONTEXT_NOT_FOUND; + + if((context_id.handle - get_contexts_offset()) >= get_registered_contexts_impl()->size()) return ROCPROFILER_STATUS_ERROR_CONTEXT_NOT_FOUND; const auto* cfg = get_registered_context(context_id); @@ -406,7 +404,7 @@ start_context(rocprofiler_context_id_t context_id) } } - uint64_t rocp_tot_contexts = get_registered_contexts_impl().size(); + uint64_t rocp_tot_contexts = get_registered_contexts_impl()->size(); auto idx = rocp_tot_contexts; auto& active_contexts = get_active_contexts_impl(); { @@ -482,6 +480,25 @@ stop_context(rocprofiler_context_id_t idx) return ROCPROFILER_STATUS_ERROR_CONTEXT_NOT_FOUND; // compare exchange failed } +rocprofiler_status_t +stop_client_contexts(rocprofiler_client_id_t client_id) +{ + if(!get_registered_contexts_impl()) return ROCPROFILER_STATUS_ERROR; + + auto ret = ROCPROFILER_STATUS_SUCCESS; + for(auto& itr : *get_registered_contexts_impl()) + { + if(itr->client_idx == client_id.handle) + { + auto status = stop_context(rocprofiler_context_id_t{itr->context_idx}); + if(status != ROCPROFILER_STATUS_SUCCESS && + status != ROCPROFILER_STATUS_ERROR_CONTEXT_NOT_FOUND) + ret = status; + } + } + return ret; +} + void deactivate_client_contexts(rocprofiler_client_id_t client_id) { @@ -498,7 +515,7 @@ deactivate_client_contexts(rocprofiler_client_id_t client_id) void deregister_client_contexts(rocprofiler_client_id_t client_id) { - for(auto& itr : get_registered_contexts_impl()) + for(auto& itr : *get_registered_contexts_impl()) { if(itr->client_idx == client_id.handle && buffer::get_buffers()) { diff --git a/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/context/context.hpp b/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/context/context.hpp index ce54d7b29a..ead31fb51f 100644 --- a/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/context/context.hpp +++ b/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/context/context.hpp @@ -182,8 +182,9 @@ validate_context(const context* cfg); rocprofiler_status_t start_context(rocprofiler_context_id_t id); -/// \brief disable the contexturation. -rocprofiler_status_t stop_context(rocprofiler_context_id_t); +/// \brief remove context from active array +rocprofiler_status_t +stop_context(rocprofiler_context_id_t id); using context_array_t = common::container::small_vector; @@ -216,10 +217,16 @@ get_active_contexts(context_array_t& data, context_filter_t filter = default_con context_array_t get_active_contexts(context_filter_t filter = default_context_filter); -void deactivate_client_contexts(rocprofiler_client_id_t); +/// \brief disable the contexturation. +rocprofiler_status_t +stop_client_contexts(rocprofiler_client_id_t id); + +void +deactivate_client_contexts(rocprofiler_client_id_t id); // should only be called if the client failed to initialize -void deregister_client_contexts(rocprofiler_client_id_t); +void +deregister_client_contexts(rocprofiler_client_id_t id); inline bool default_context_filter(const context* val) diff --git a/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/registration.cpp b/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/registration.cpp index 8be91c119e..dc165d03ea 100644 --- a/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/registration.cpp +++ b/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/registration.cpp @@ -471,6 +471,7 @@ invoke_client_finalizer(rocprofiler_client_id_t client_id) if(itr && itr->internal_client_id.handle == client_id.handle && itr->mutable_client_id.handle == client_id.handle) { + context::stop_client_contexts(itr->internal_client_id); if(itr->configure_result && itr->configure_result->finalize) { // set to nullptr so finalize only gets called once diff --git a/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/tests/external_correlation.cpp b/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/tests/external_correlation.cpp index cd6977532d..f081a004ea 100644 --- a/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/tests/external_correlation.cpp +++ b/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/tests/external_correlation.cpp @@ -276,8 +276,10 @@ TEST(rocprofiler_lib, callback_external_correlation) static fini_func_t tool_fini = [](void* client_data) -> void { auto* cb_data = static_cast(client_data); - ROCPROFILER_CALL(rocprofiler_stop_context(cb_data->client_ctx), - "rocprofiler context stop failed"); + int status = 0; + ROCPROFILER_CALL(rocprofiler_context_is_active(cb_data->client_ctx, &status), + "rocprofiler_context_is_active failed"); + EXPECT_EQ(status, 0); static_cast(client_data)->client_workflow_count++; }; @@ -432,8 +434,10 @@ TEST(rocprofiler_lib, buffered_external_correlation) static fini_func_t tool_fini = [](void* client_data) -> void { auto* cb_data = static_cast(client_data); - ROCPROFILER_CALL(rocprofiler_stop_context(cb_data->client_ctx), - "rocprofiler context stop failed"); + int status = 0; + ROCPROFILER_CALL(rocprofiler_context_is_active(cb_data->client_ctx, &status), + "rocprofiler_context_is_active failed"); + EXPECT_EQ(status, 0); static_cast(client_data)->client_workflow_count++; }; diff --git a/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/tests/intercept_table.cpp b/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/tests/intercept_table.cpp index 18f8422c7c..2ab135f387 100644 --- a/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/tests/intercept_table.cpp +++ b/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/tests/intercept_table.cpp @@ -218,8 +218,10 @@ TEST(rocprofiler_lib, intercept_table_and_callback_tracing) static fini_func_t tool_fini = [](void* client_data) -> void { auto* cb_data = static_cast(client_data); - ROCPROFILER_CALL(rocprofiler_stop_context(cb_data->client_hsa_ctx), - "rocprofiler context stop failed"); + int status = 0; + ROCPROFILER_CALL(rocprofiler_context_is_active(cb_data->client_hsa_ctx, &status), + "rocprofiler_context_is_active failed"); + EXPECT_EQ(status, 0); static_cast(client_data)->client_workflow_count++; }; @@ -375,9 +377,10 @@ TEST(rocprofiler_lib, intercept_table_and_callback_tracing_disable_context) static fini_func_t tool_fini = [](void* client_data) -> void { auto* cb_data = static_cast(client_data); - ROCPROFILER_CALL_EXPECT(rocprofiler_stop_context(cb_data->client_hsa_ctx), - "rocprofiler context stop", - ROCPROFILER_STATUS_ERROR_CONTEXT_NOT_FOUND); + int status = 0; + ROCPROFILER_CALL(rocprofiler_context_is_active(cb_data->client_hsa_ctx, &status), + "rocprofiler_context_is_active failed"); + EXPECT_EQ(status, 0); ROCPROFILER_CALL_EXPECT(rocprofiler_start_context(cb_data->client_hsa_ctx), "rocprofiler context start", diff --git a/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/tests/registration.cpp b/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/tests/registration.cpp index bceff11ade..6c8c4751b0 100644 --- a/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/tests/registration.cpp +++ b/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/tests/registration.cpp @@ -278,8 +278,10 @@ TEST(rocprofiler_lib, callback_registration_lambda_with_result) static fini_func_t tool_fini = [](void* client_data) -> void { auto* cb_data = static_cast(client_data); - ROCPROFILER_CALL(rocprofiler_stop_context(cb_data->client_ctx), - "rocprofiler context stop failed"); + int status = 0; + ROCPROFILER_CALL(rocprofiler_context_is_active(cb_data->client_ctx, &status), + "rocprofiler_context_is_active failed"); + EXPECT_EQ(status, 0); static_cast(client_data)->client_workflow_count++; }; @@ -429,8 +431,10 @@ TEST(rocprofiler_lib, buffer_registration_lambda_with_result) auto* cb_data = static_cast(client_data); ROCPROFILER_CALL(rocprofiler_flush_buffer(cb_data->client_buffer), "rocprofiler context stop failed"); - ROCPROFILER_CALL(rocprofiler_stop_context(cb_data->client_ctx), - "rocprofiler context stop failed"); + int status = 0; + ROCPROFILER_CALL(rocprofiler_context_is_active(cb_data->client_ctx, &status), + "rocprofiler_context_is_active failed"); + EXPECT_EQ(status, 0); static_cast(client_data)->client_workflow_count++; }; diff --git a/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/tests/roctx.cpp b/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/tests/roctx.cpp index a84b510d3e..27e540a203 100644 --- a/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/tests/roctx.cpp +++ b/projects/rocprofiler-sdk/source/lib/rocprofiler-sdk/tests/roctx.cpp @@ -360,8 +360,10 @@ TEST(rocprofiler_lib, roctx_callback_tracing) static fini_func_t tool_fini = [](void* client_data) -> void { auto* cb_data = static_cast(client_data); - ROCPROFILER_CALL(rocprofiler_stop_context(cb_data->client_ctx), - "rocprofiler context stop failed"); + int status = 0; + ROCPROFILER_CALL(rocprofiler_context_is_active(cb_data->client_ctx, &status), + "rocprofiler_context_is_active failed"); + EXPECT_EQ(status, 0); static_cast(client_data)->client_workflow_count++; }; @@ -489,8 +491,10 @@ TEST(rocprofiler_lib, roctx_buffered_tracing) auto* cb_data = static_cast(client_data); ROCPROFILER_CALL(rocprofiler_flush_buffer(cb_data->client_buffer), "rocprofiler context stop failed"); - ROCPROFILER_CALL(rocprofiler_stop_context(cb_data->client_ctx), - "rocprofiler context stop failed"); + int status = 0; + ROCPROFILER_CALL(rocprofiler_context_is_active(cb_data->client_ctx, &status), + "rocprofiler_context_is_active failed"); + EXPECT_EQ(status, 0); static_cast(client_data)->client_workflow_count++; };