From 66ed1e46b26fa72c00261aa72f6d0900d0219bd2 Mon Sep 17 00:00:00 2001 From: Valentin Seitz <valentin.seitz@bsc.es> Date: Wed, 24 Jul 2024 12:20:15 +0200 Subject: [PATCH 1/5] Refactored and made the delegator a bit easier --- src/delegator.cpp | 151 ++++++++++++++++++++++------------------------ src/delegator.hpp | 15 +++-- 2 files changed, 80 insertions(+), 86 deletions(-) diff --git a/src/delegator.cpp b/src/delegator.cpp index 60f68f5..547a317 100644 --- a/src/delegator.cpp +++ b/src/delegator.cpp @@ -1,6 +1,7 @@ #include <delegator.hpp> #include <iostream> #include <memory> +#include <optional> #include <sstream> #include <strategies.hpp> #include <string> @@ -19,8 +20,18 @@ #include "backends/extrae/extrae_type_stack.hpp" #endif -void Delegator::InitDetectionBackend(const std::string &backend) { - pn_annotation_strategy_ = std::make_unique<DetectionStrategy>(); +std::optional<NestingMode> getNestingMode(const std::string &nesting_mode) { + // detect nesting mode specified by the user + if (stringsAreCaseInsensitiveEqual(nesting_mode, "?")) { + return NestingMode::Detection; + } else if (stringsAreCaseInsensitiveEqual(nesting_mode, "ProperlyNested")) { + return NestingMode::ProperlyNested; + } else if (stringsAreCaseInsensitiveEqual(nesting_mode, + "NotProperlyNested")) { + return NestingMode::NotProperlyNested; + } else { + return std::nullopt; + } } void Delegator::InitNonProperlyNestedBackends(const std::string &backend) { @@ -37,12 +48,13 @@ void Delegator::InitNonProperlyNestedBackends(const std::string &backend) { else { npn_annotation_strategy_ = std::make_unique<NPNDefault>(); - std::cout << "Could not find a matching NotProperlyNested backend with name" - << backend << " now selecting the Default" << std::endl; + std::cout + << "Could not find a matching NotProperlyNested backend with name " + << backend << " now selecting the Default" << std::endl; } } -void Delegator::InitProperlyNestedBackends(const std::string &backend) { +void Delegator::TryInitProperlyNestedBackend(const std::string &backend) { if (false) { // a bit ugly, but worky } @@ -58,28 +70,28 @@ void Delegator::InitProperlyNestedBackends(const std::string &backend) { #endif } -void Delegator::Init(std::string_view nesting_mode, +void Delegator::Init(std::string_view nesting_mode_in, std::string_view backend_in) { mpi_helper_ = std::make_unique<MPIHelper>(); + std::string backend = std::string(backend_in); if (mpi_helper_->IsRankNumber(0)) { - std::cout << "NESMIK init called with: " << nesting_mode - << " and Backend: " << backend << std::endl; + std::cout << "neSmiK init called with program nesting: " << nesting_mode_in + << " and backend: " << backend << std::endl; } - // detect nesting mode specified by the user - if (nesting_mode.compare("?") == 0) { - nesting_mode_ = NestingMode::Detection; - } else if (nesting_mode.compare("ProperlyNested") == 0) { - nesting_mode_ = NestingMode::ProperlyNested; - } else if (nesting_mode.compare("NotProperlyNested") == 0) { - nesting_mode_ = NestingMode::NotProperlyNested; + // now determine nesting mode + auto nesting_mode = getNestingMode(std::string(nesting_mode_in)); + if (!nesting_mode.has_value()) { + std::cout << "Something went wrong when selecting the nesting mode, " + "please make sure to spell it correctly" + << std::endl; + exit(1); } - if (mpi_helper_->IsRankNumber(0)) { // Some debug output - switch (nesting_mode_) { + switch (nesting_mode.value()) { case NestingMode::Detection: std::cout << "nesting mode set to: " << "Detection (?)" << std::endl; @@ -92,11 +104,6 @@ void Delegator::Init(std::string_view nesting_mode, std::cout << "nesting mode set to: " << "ProperlyNested" << std::endl; break; - case NestingMode::ProperlyNestedButOtherBackend: - std::cout << "Something went wrong when selecting the nesting mode" - << std::endl; - exit(1); - break; } } @@ -117,41 +124,48 @@ void Delegator::Init(std::string_view nesting_mode, } } - // next choose backend - switch (nesting_mode_) { - case NestingMode::ProperlyNestedButOtherBackend: - break; + // next do backend selection: + + switch (nesting_mode.value()) { case NestingMode::Detection: - InitDetectionBackend(backend); + pn_annotation_strategy_ = std::make_unique<DetectionStrategy>(); + backend_selection_ = BackendSelection::ProperlyNested; break; case NestingMode::ProperlyNested: - InitProperlyNestedBackends(backend); - // we preferably use a Properly nested backend, + TryInitProperlyNestedBackend(backend); + // we preferably use a Properly nested backend, // but if this is not possible, we will select the not properly nested // one + if (pn_annotation_strategy_ != nullptr) { + backend_selection_ = BackendSelection::ProperlyNested; + break; // we break here if we already inited the backend + } [[fallthrough]]; case NestingMode::NotProperlyNested: - if (pn_annotation_strategy_ == nullptr && mpi_helper_->IsRankNumber(0)) { + // two cases lead us here: either we previously tried to init a properly + // nested backend, which failed. if this failed, we inform the user of + // this + if (nesting_mode.value() == NestingMode::ProperlyNested && + pn_annotation_strategy_ == nullptr && mpi_helper_->IsRankNumber(0)) { std::cout << "Could not find a matching ProperlyNested backend with name " << backend << std::endl; std::cout << "Now looking for a NotProperlyNested backend, as they " "support ProperlyNested ones" << std::endl; - nesting_mode_ = NestingMode::ProperlyNestedButOtherBackend; } - InitNonProperlyNestedBackends(backend); + if (npn_annotation_strategy_ != nullptr) { + backend_selection_ = BackendSelection::NotProperlyNested; + } break; } - switch (nesting_mode_) { - case NestingMode::NotProperlyNested: - case NestingMode::ProperlyNestedButOtherBackend: + switch (backend_selection_) { + case BackendSelection::NotProperlyNested: return npn_annotation_strategy_->Init(); break; - case NestingMode::Detection: - case NestingMode::ProperlyNested: + case BackendSelection::ProperlyNested: return pn_annotation_strategy_->Init(); break; } @@ -159,28 +173,22 @@ void Delegator::Init(std::string_view nesting_mode, // should be thread safe to call? void Delegator::RegionStart(std::string_view name) { - switch (nesting_mode_) { - case NestingMode::NotProperlyNested: - return npn_annotation_strategy_->RegionStart({name}); - case NestingMode::ProperlyNestedButOtherBackend: - name_stack_.push(std::string(name)); + name_stack_.push(std::string(name)); + switch (backend_selection_) { + case BackendSelection::NotProperlyNested: return npn_annotation_strategy_->RegionStart({name}); - case NestingMode::Detection: - case NestingMode::ProperlyNested: - // First push to stack - name_stack_.push(std::string(name)); + case BackendSelection::ProperlyNested: return pn_annotation_strategy_->RegionStart({name, name_stack_}); break; } } + void Delegator::RegionStop(std::string_view name) { - switch (nesting_mode_) { - case NestingMode::NotProperlyNested: - case NestingMode::ProperlyNestedButOtherBackend: + switch (backend_selection_) { + case BackendSelection::NotProperlyNested: return npn_annotation_strategy_->RegionStop({name}); break; - case NestingMode::Detection: - case NestingMode::ProperlyNested: + case BackendSelection::ProperlyNested: pn_annotation_strategy_->RegionStopLast({name, name_stack_}); if (!name_stack_.empty()) { name_stack_.pop(); @@ -190,41 +198,28 @@ void Delegator::RegionStop(std::string_view name) { }; void Delegator::RegionStopLast() { - switch (nesting_mode_) { - case NestingMode::NotProperlyNested: - std::cout << "Calling region_stop_last() is not supported in " - "NotProperlyNested Codes" - << std::endl; + const std::string stack_empty = "NESMIK: STACK EMPTPY"; + const std::string &last_region_name = + name_stack_.empty() ? stack_empty : name_stack_.top(); + switch (backend_selection_) { + case BackendSelection::ProperlyNested: + pn_annotation_strategy_->RegionStopLast({last_region_name, name_stack_}); break; - case NestingMode::Detection: - case NestingMode::ProperlyNested: - case NestingMode::ProperlyNestedButOtherBackend: - const std::string stack_empty = "NESMIK: STACK EMPTPY"; - const std::string &last_region_name = - name_stack_.empty() ? stack_empty : name_stack_.top(); - - if (nesting_mode_ == NestingMode::Detection || - nesting_mode_ == NestingMode::ProperlyNested) { - pn_annotation_strategy_->RegionStopLast( - {last_region_name, name_stack_}); - } else if (nesting_mode_ == NestingMode::ProperlyNestedButOtherBackend) { - npn_annotation_strategy_->RegionStop({last_region_name}); - } - if (!name_stack_.empty()) { - name_stack_.pop(); - } + case BackendSelection::NotProperlyNested: + npn_annotation_strategy_->RegionStop({last_region_name}); break; } + if (!name_stack_.empty()) { + name_stack_.pop(); + } } void Delegator::Finalize() { - switch (nesting_mode_) { - case NestingMode::NotProperlyNested: - case NestingMode::ProperlyNestedButOtherBackend: + switch (backend_selection_) { + case BackendSelection::NotProperlyNested: return npn_annotation_strategy_->Finalize(); break; - case NestingMode::Detection: - case NestingMode::ProperlyNested: + case BackendSelection::ProperlyNested: return pn_annotation_strategy_->Finalize(); break; } diff --git a/src/delegator.hpp b/src/delegator.hpp index 021ea81..deb18e6 100644 --- a/src/delegator.hpp +++ b/src/delegator.hpp @@ -8,13 +8,13 @@ #include <vector> enum class NestingMode { - Detection, // When selecting the very special Detection backend - ProperlyNested, // Application is properly nested - NotProperlyNested, // Application annotation is not properly nested - ProperlyNestedButOtherBackend // Application is properly nested, but were - // using a not properly nested backend + Detection, // When selecting the very special Detection backend + ProperlyNested, // Application is properly nested + NotProperlyNested // Application annotation is not properly nested }; +enum class BackendSelection { ProperlyNested, NotProperlyNested }; + class Delegator { private: inline static std::unique_ptr<ProperlyNestedAnnotationStrategy> @@ -24,7 +24,7 @@ class Delegator { inline static thread_local std::stack<std::string> name_stack_; - inline static NestingMode nesting_mode_ = NestingMode::Detection; + inline static BackendSelection backend_selection_; inline static std::unique_ptr<MPIHelper> mpi_helper_; @@ -36,8 +36,7 @@ class Delegator { static void Finalize(); private: - static void InitDetectionBackend(const std::string &backend); static void InitNonProperlyNestedBackends(const std::string &backend); - static void InitProperlyNestedBackends(const std::string &backend); + static void TryInitProperlyNestedBackend(const std::string &backend); }; #endif // NESMIK_DELEGATOR_HPP -- GitLab From 1c12c977f50d99cfd3230ac51a06b8a789cb0f93 Mon Sep 17 00:00:00 2001 From: Valentin Seitz <valentin.seitz@bsc.es> Date: Wed, 24 Jul 2024 14:20:28 +0200 Subject: [PATCH 2/5] Fix mixed calls --- src/delegator.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/delegator.cpp b/src/delegator.cpp index 547a317..b0f42b5 100644 --- a/src/delegator.cpp +++ b/src/delegator.cpp @@ -190,11 +190,11 @@ void Delegator::RegionStop(std::string_view name) { break; case BackendSelection::ProperlyNested: pn_annotation_strategy_->RegionStopLast({name, name_stack_}); - if (!name_stack_.empty()) { - name_stack_.pop(); - } break; } + if (!name_stack_.empty()) { + name_stack_.pop(); + } }; void Delegator::RegionStopLast() { -- GitLab From 417c696b60c41ad51a31ffe664c8ec3f6ababc0e Mon Sep 17 00:00:00 2001 From: Valentin Seitz <valentin.seitz@bsc.es> Date: Wed, 24 Jul 2024 14:43:16 +0200 Subject: [PATCH 3/5] remove return, which stopped stack from getting popped --- src/delegator.cpp | 13 ++++++++++--- tests/cpp/TestRegionStopLast.cpp | 2 +- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/delegator.cpp b/src/delegator.cpp index b0f42b5..4e56797 100644 --- a/src/delegator.cpp +++ b/src/delegator.cpp @@ -174,11 +174,13 @@ void Delegator::Init(std::string_view nesting_mode_in, // should be thread safe to call? void Delegator::RegionStart(std::string_view name) { name_stack_.push(std::string(name)); + switch (backend_selection_) { case BackendSelection::NotProperlyNested: - return npn_annotation_strategy_->RegionStart({name}); + npn_annotation_strategy_->RegionStart({name}); + break; case BackendSelection::ProperlyNested: - return pn_annotation_strategy_->RegionStart({name, name_stack_}); + pn_annotation_strategy_->RegionStart({name, name_stack_}); break; } } @@ -186,13 +188,17 @@ void Delegator::RegionStart(std::string_view name) { void Delegator::RegionStop(std::string_view name) { switch (backend_selection_) { case BackendSelection::NotProperlyNested: - return npn_annotation_strategy_->RegionStop({name}); + npn_annotation_strategy_->RegionStop({name}); break; case BackendSelection::ProperlyNested: pn_annotation_strategy_->RegionStopLast({name, name_stack_}); break; } if (!name_stack_.empty()) { + if (name.compare(name_stack_.top()) != 0) { + std::cout << "neSmiK region_close() expected " << name_stack_.top() + << "but got " << name << std::endl; + } name_stack_.pop(); } }; @@ -209,6 +215,7 @@ void Delegator::RegionStopLast() { npn_annotation_strategy_->RegionStop({last_region_name}); break; } + if (!name_stack_.empty()) { name_stack_.pop(); } diff --git a/tests/cpp/TestRegionStopLast.cpp b/tests/cpp/TestRegionStopLast.cpp index 85c3e49..6bc07cb 100644 --- a/tests/cpp/TestRegionStopLast.cpp +++ b/tests/cpp/TestRegionStopLast.cpp @@ -4,7 +4,7 @@ int main() { nesmik::init("ProperlyNested", "Default"); nesmik::region_start("Test0-1"); nesmik::region_start("Test1-1"); - nesmik::region_stop_last(); + nesmik::region_stop("Test1-1"); nesmik::region_start("Test1-2"); nesmik::region_stop_last(); nesmik::region_stop_last(); -- GitLab From 73be96b978144b01612c870df0c1f6889db18e00 Mon Sep 17 00:00:00 2001 From: VALENTIN SEITZ <valentin.seitz@bsc.es> Date: Wed, 24 Jul 2024 15:54:56 +0200 Subject: [PATCH 4/5] Apply 1 suggestion(s) to 1 file(s) Co-authored-by: JOAN VINYALS YLLA CATALA <joan.vinyals@bsc.es> --- src/delegator.cpp | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/delegator.cpp b/src/delegator.cpp index 4e56797..9a7861a 100644 --- a/src/delegator.cpp +++ b/src/delegator.cpp @@ -140,13 +140,10 @@ void Delegator::Init(std::string_view nesting_mode_in, backend_selection_ = BackendSelection::ProperlyNested; break; // we break here if we already inited the backend } - [[fallthrough]]; - case NestingMode::NotProperlyNested: - // two cases lead us here: either we previously tried to init a properly - // nested backend, which failed. if this failed, we inform the user of - // this - if (nesting_mode.value() == NestingMode::ProperlyNested && - pn_annotation_strategy_ == nullptr && mpi_helper_->IsRankNumber(0)) { + + // we inform the user that ProperlyNested initialization failed, and + // fallthrough + if (mpi_helper_->IsRankNumber(0)) { std::cout << "Could not find a matching ProperlyNested backend with name " << backend << std::endl; @@ -154,6 +151,11 @@ void Delegator::Init(std::string_view nesting_mode_in, "support ProperlyNested ones" << std::endl; } + + [[fallthrough]]; + case NestingMode::NotProperlyNested: + // two cases lead us here: either we previously tried to init a properly + // nested backend, which failed, or we are in not properly nested mode. InitNonProperlyNestedBackends(backend); if (npn_annotation_strategy_ != nullptr) { backend_selection_ = BackendSelection::NotProperlyNested; -- GitLab From c3436a395c715f3ccb0fab1134b6b80a6ec3b916 Mon Sep 17 00:00:00 2001 From: Valentin Seitz <valentin.seitz@bsc.es> Date: Wed, 24 Jul 2024 16:00:30 +0200 Subject: [PATCH 5/5] added clang-format --- src/delegator.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/delegator.cpp b/src/delegator.cpp index 9a7861a..51ff083 100644 --- a/src/delegator.cpp +++ b/src/delegator.cpp @@ -141,7 +141,7 @@ void Delegator::Init(std::string_view nesting_mode_in, break; // we break here if we already inited the backend } - // we inform the user that ProperlyNested initialization failed, and + // we inform the user that ProperlyNested initialization failed, and // fallthrough if (mpi_helper_->IsRankNumber(0)) { std::cout -- GitLab