From 1f5a5af040faa71499e5e38cc3f4c8a4f94ab43c Mon Sep 17 00:00:00 2001 From: Willem Deconinck Date: Tue, 23 Apr 2024 17:56:53 +0200 Subject: [PATCH] Use std::string_view in eckit_mpi --- src/eckit/mpi/Comm.cc | 82 ++++++++++++++++---------------- src/eckit/mpi/Comm.h | 31 ++++++------ src/eckit/mpi/Parallel.cc | 8 ++-- src/eckit/mpi/Parallel.h | 6 +-- src/eckit/mpi/ParallelGroup.cc | 7 +-- src/eckit/mpi/ParallelRequest.cc | 4 +- src/eckit/mpi/Serial.cc | 4 +- src/eckit/mpi/Serial.h | 4 +- 8 files changed, 75 insertions(+), 71 deletions(-) diff --git a/src/eckit/mpi/Comm.cc b/src/eckit/mpi/Comm.cc index eb88fa252..6071135fb 100644 --- a/src/eckit/mpi/Comm.cc +++ b/src/eckit/mpi/Comm.cc @@ -36,7 +36,7 @@ constexpr bool have_parallel() { class Environment { public: - static const char* getDefaultCommType() { + static std::string_view getDefaultCommType() { // Force a given communicator (only required if e.g. running serial applications with MPI) if (const char* forcedComm = ::getenv("ECKIT_MPI_FORCE")) { return forcedComm; @@ -80,10 +80,10 @@ class Environment { default_ = world; } - void setDefaut(const char* name) { + void setDefault(std::string_view name) { AutoLock lock(mutex_); - std::map::iterator itr = communicators.find(name); + auto itr = communicators.find(name); if (itr != communicators.end()) { default_ = itr->second; return; @@ -95,10 +95,10 @@ class Environment { for (itr = communicators.begin(); itr != communicators.end(); ++itr) { eckit::Log::error() << " " << (*itr).first << std::endl; } - throw eckit::SeriousBug(std::string("No communicator called ") + name, Here()); + throw eckit::SeriousBug("No communicator called " + std::string{name}, Here()); } - bool hasComm(const char* name) { + bool hasComm(std::string_view name) { AutoLock lock(mutex_); std::map::iterator itr = communicators.find(name); if (itr != communicators.end()) { @@ -120,17 +120,17 @@ class Environment { void finaliseAllComms() { AutoLock lock(mutex_); - std::map::iterator itr = communicators.begin(); + auto itr = communicators.begin(); for (; itr != communicators.end(); ++itr) { delete itr->second; } communicators.clear(); } - Comm& getComm(const char* name = nullptr) { + Comm& getComm(std::string_view name = {}) { AutoLock lock(mutex_); - if (!name && default_) { + if (name.empty() && default_) { return *default_; /* most common case first */ } @@ -138,12 +138,12 @@ class Environment { initDefault(); } - if (!name) { + if (name.empty()) { ASSERT(default_); /* sanity check init was successful */ return *default_; } - std::map::iterator itr = communicators.find(name); + auto itr = communicators.find(name); if (itr != communicators.end()) { return *itr->second; } @@ -153,30 +153,30 @@ class Environment { for (itr = communicators.begin(); itr != communicators.end(); ++itr) { eckit::Log::error() << " " << (*itr).first << std::endl; } - throw eckit::SeriousBug(std::string("No communicator called ") + name, Here()); + throw eckit::SeriousBug("No communicator called " + std::string{name}, Here()); } - void addComm(const char* name, int comm) { + void addComm(std::string_view name, int comm) { AutoLock lock(mutex_); if (hasComm(name)) { - throw SeriousBug("Communicator with name " + std::string(name) + " already exists", Here()); + throw SeriousBug("Communicator with name " + std::string{name} + " already exists", Here()); } Comm* pComm = CommFactory::build(name, getDefaultCommType(), comm); - communicators[name] = pComm; + communicators.emplace(name, pComm); } - void addComm(const char* name, Comm* comm) { + void addComm(std::string_view name, Comm* comm) { AutoLock lock(mutex_); if (hasComm(name)) { - throw SeriousBug("Communicator with name " + std::string(name) + " already exists", Here()); + throw SeriousBug("Communicator with name " + std::string{name} + " already exists", Here()); } - communicators[name] = comm; + communicators.emplace(name, comm); } - void deleteComm(const char* name) { + void deleteComm(std::string_view name) { AutoLock lock(mutex_); auto itr = communicators.find(name); @@ -188,7 +188,7 @@ class Environment { // refuse to delete the default communicator if (default_ == comm) { throw SeriousBug("Trying to delete the default Communicator with name " - + std::string(name), + + std::string{name}, Here()); } @@ -198,7 +198,7 @@ class Environment { communicators.erase(itr); } else { - throw SeriousBug("Communicator with name " + std::string(name) + " does not exist", Here()); + throw SeriousBug("Communicator with name " + std::string{name} + " does not exist", Here()); } } @@ -215,7 +215,7 @@ class Environment { Comm* default_; - std::map communicators; + std::map> communicators; eckit::Mutex mutex_; }; @@ -224,21 +224,21 @@ class Environment { class CommFactories { public: - void registFactory(const std::string& builder, CommFactory* f) { + void registFactory(std::string_view builder, CommFactory* f) { AutoLock lock(mutex_); ASSERT(factories.find(builder) == factories.end()); - factories[builder] = f; + factories.emplace(builder, f); } - void unregistFactory(const std::string& builder) { + void unregistFactory(std::string_view builder) { AutoLock lock(mutex_); - factories.erase(builder); + factories.erase(std::string{builder}); } - CommFactory& getFactory(const std::string& builder) { + CommFactory& getFactory(std::string_view builder) const { AutoLock lock(mutex_); - std::map::const_iterator j = factories.find(builder); + auto j = factories.find(builder); if (j != factories.end()) { return *(j->second); @@ -250,7 +250,7 @@ class CommFactories { eckit::Log::error() << " " << (*j).first << std::endl; } - throw eckit::SeriousBug(std::string("No CommFactory called ") + builder, Here()); + throw eckit::SeriousBug("No CommFactory called " + std::string{builder}, Here()); } static CommFactories& instance() { @@ -261,11 +261,11 @@ class CommFactories { private: CommFactories() {} - std::map factories; - eckit::Mutex mutex_; + std::map> factories; + mutable eckit::Mutex mutex_; }; -CommFactory::CommFactory(const std::string& builder) : +CommFactory::CommFactory(std::string_view builder) : builder_(builder) { CommFactories::instance().registFactory(builder, this); } @@ -274,43 +274,43 @@ CommFactory::~CommFactory() { CommFactories::instance().unregistFactory(builder_); } -Comm* CommFactory::build(const std::string& name, const std::string& builder) { +Comm* CommFactory::build(std::string_view name, std::string_view builder) { return CommFactories::instance().getFactory(builder).make(name); } -Comm* CommFactory::build(const std::string& name, const std::string& builder, int comm) { +Comm* CommFactory::build(std::string_view name, std::string_view builder, int comm) { return CommFactories::instance().getFactory(builder).make(name, comm); } //---------------------------------------------------------------------------------------------------------------------- -Comm::Comm(const std::string& name) : +Comm::Comm(std::string_view name) : name_(name) {} Comm::~Comm() {} //---------------------------------------------------------------------------------------------------------------------- -Comm& comm(const char* name) { +Comm& comm(std::string_view name) { return Environment::instance().getComm(name); } -void setCommDefault(const char* name) { - Environment::instance().setDefaut(name); +void setCommDefault(std::string_view name) { + Environment::instance().setDefault(name); } -void addComm(const char* name, int comm) { +void addComm(std::string_view name, int comm) { Environment::instance().addComm(name, comm); } -void addComm(const char* name, Comm* comm) { +void addComm(std::string_view name, Comm* comm) { Environment::instance().addComm(name, comm); } -void deleteComm(const char* name) { +void deleteComm(std::string_view name) { Environment::instance().deleteComm(name); } -bool hasComm(const char* name) { +bool hasComm(std::string_view name) { return Environment::instance().hasComm(name); } diff --git a/src/eckit/mpi/Comm.h b/src/eckit/mpi/Comm.h index 5150d00c3..4c21635fe 100644 --- a/src/eckit/mpi/Comm.h +++ b/src/eckit/mpi/Comm.h @@ -14,6 +14,7 @@ #include #include #include +#include #include #include "eckit/filesystem/PathName.h" @@ -35,25 +36,25 @@ class Environment; /// @returns the communicator registered with associated name, or default communicator when NULL is /// passed -Comm& comm(const char* name = nullptr); +Comm& comm(std::string_view name = {}); Comm& self(); /// Set a communicator as default -void setCommDefault(const char* name); +void setCommDefault(std::string_view name); /// Register a communicator comming from Fortran code -void addComm(const char* name, int comm); +void addComm(std::string_view name, int comm); /// Register an existing communicator -void addComm(const char* name, Comm* comm); +void addComm(std::string_view name, Comm* comm); /// Unregister and delete specific comm /// @pre Comm is registered in the environment -void deleteComm(const char* name); +void deleteComm(std::string_view name); /// Check if a communicator is registered -bool hasComm(const char* name); +bool hasComm(std::string_view name); std::vector listComms(); @@ -78,7 +79,7 @@ class Comm : private eckit::NonCopyable { friend class Environment; public: // class methods - static Comm& comm(const char* name = nullptr); + static Comm& comm(std::string_view name = {}); public: // methods std::string name() const { return name_; } @@ -477,7 +478,7 @@ class Comm : private eckit::NonCopyable { } protected: // methods - Comm(const std::string& name); + Comm(std::string_view name); virtual ~Comm(); @@ -492,21 +493,21 @@ class CommFactory { friend class eckit::mpi::Environment; std::string builder_; - virtual Comm* make(const std::string& name) = 0; - virtual Comm* make(const std::string& name, int) = 0; + virtual Comm* make(std::string_view name) = 0; + virtual Comm* make(std::string_view name, int) = 0; protected: - CommFactory(const std::string& builder); + CommFactory(std::string_view builder); virtual ~CommFactory(); - static Comm* build(const std::string& name, const std::string& builder); - static Comm* build(const std::string& name, const std::string& builder, int); + static Comm* build(std::string_view name, std::string_view builder); + static Comm* build(std::string_view name, std::string_view builder, int); }; template class CommBuilder : public CommFactory { - Comm* make(const std::string& name) override { return new T(name); } - Comm* make(const std::string& name, int comm) override { return new T(name, comm); } + Comm* make(std::string_view name) override { return new T(name); } + Comm* make(std::string_view name, int comm) override { return new T(name, comm); } public: CommBuilder(const std::string& builder) : diff --git a/src/eckit/mpi/Parallel.cc b/src/eckit/mpi/Parallel.cc index 2479e4422..ea348fae9 100644 --- a/src/eckit/mpi/Parallel.cc +++ b/src/eckit/mpi/Parallel.cc @@ -55,7 +55,7 @@ class MPIError : public eckit::Exception { //---------------------------------------------------------------------------------------------------------------------- -void MPICall(int code, const char* mpifunc, const eckit::CodeLocation& loc) { +void MPICall(int code, std::string_view mpifunc, const eckit::CodeLocation& loc) { if (code != MPI_SUCCESS) { char error[10240]; @@ -169,7 +169,7 @@ size_t getSize(MPI_Comm comm) { //---------------------------------------------------------------------------------------------------------------------- -Parallel::Parallel(const std::string& name) : +Parallel::Parallel(std::string_view name) : Comm(name) /* don't use member initialisation list */ { pthread_once(&once, init); @@ -185,7 +185,7 @@ Parallel::Parallel(const std::string& name) : size_ = getSize(comm_); } -Parallel::Parallel(const std::string& name, MPI_Comm comm, bool) : +Parallel::Parallel(std::string_view name, MPI_Comm comm, bool) : Comm(name) /* don't use member initialisation list */ { pthread_once(&once, init); @@ -201,7 +201,7 @@ Parallel::Parallel(const std::string& name, MPI_Comm comm, bool) : size_ = getSize(comm_); } -Parallel::Parallel(const std::string& name, int comm) : +Parallel::Parallel(std::string_view name, int comm) : Comm(name) { pthread_once(&once, init); diff --git a/src/eckit/mpi/Parallel.h b/src/eckit/mpi/Parallel.h index f0abfae0e..0b8e99aae 100644 --- a/src/eckit/mpi/Parallel.h +++ b/src/eckit/mpi/Parallel.h @@ -29,9 +29,9 @@ class Parallel : public eckit::mpi::Comm { template friend class CommBuilder; - Parallel(const std::string& name); - Parallel(const std::string& name, MPI_Comm comm, bool); - Parallel(const std::string& name, int comm); + Parallel(std::string_view name); + Parallel(std::string_view name, MPI_Comm comm, bool); + Parallel(std::string_view name, int comm); ~Parallel() override; diff --git a/src/eckit/mpi/ParallelGroup.cc b/src/eckit/mpi/ParallelGroup.cc index 0d7feae24..4653ecbd3 100644 --- a/src/eckit/mpi/ParallelGroup.cc +++ b/src/eckit/mpi/ParallelGroup.cc @@ -8,6 +8,8 @@ * does it submit to any jurisdiction. */ +#include + #include "eckit/mpi/ParallelGroup.h" #include "eckit/log/CodeLocation.h" #include "eckit/mpi/Parallel.h" @@ -16,7 +18,7 @@ namespace eckit { namespace mpi { -void MPICall(int code, const char* mpifunc, const eckit::CodeLocation& loc); +void MPICall(int code, std::string_view mpifunc, const eckit::CodeLocation& loc); #define MPI_CALL(a) MPICall(a, #a, Here()) //---------------------------------------------------------------------------------------------------------------------- @@ -35,8 +37,7 @@ ParallelGroup::ParallelGroup(MPI_Group group) : group_(group) {} void ParallelGroup::print(std::ostream& os) const { - os << "ParallelGroup(" - << ")"; + os << "ParallelGroup()"; } int ParallelGroup::group() const { diff --git a/src/eckit/mpi/ParallelRequest.cc b/src/eckit/mpi/ParallelRequest.cc index 6710cd79e..67b1c0036 100644 --- a/src/eckit/mpi/ParallelRequest.cc +++ b/src/eckit/mpi/ParallelRequest.cc @@ -8,6 +8,8 @@ * does it submit to any jurisdiction. */ +#include + #include "eckit/mpi/ParallelRequest.h" #include "eckit/log/CodeLocation.h" #include "eckit/mpi/Parallel.h" @@ -16,7 +18,7 @@ namespace eckit { namespace mpi { -void MPICall(int code, const char* mpifunc, const eckit::CodeLocation& loc); +void MPICall(int code, std::string_view mpifunc, const eckit::CodeLocation& loc); #define MPI_CALL(a) MPICall(a, #a, Here()) //---------------------------------------------------------------------------------------------------------------------- diff --git a/src/eckit/mpi/Serial.cc b/src/eckit/mpi/Serial.cc index 923bf464a..70a49e1fd 100644 --- a/src/eckit/mpi/Serial.cc +++ b/src/eckit/mpi/Serial.cc @@ -134,13 +134,13 @@ class SerialRequestPool : private NonCopyable { //---------------------------------------------------------------------------------------------------------------------- -Serial::Serial(const std::string& name) : +Serial::Serial(std::string_view name) : Comm(name) { rank_ = 0; size_ = 1; } -Serial::Serial(const std::string& name, int) : +Serial::Serial(std::string_view name, int) : Comm(name) { rank_ = 0; size_ = 1; diff --git a/src/eckit/mpi/Serial.h b/src/eckit/mpi/Serial.h index 482fe60f2..b561b1531 100644 --- a/src/eckit/mpi/Serial.h +++ b/src/eckit/mpi/Serial.h @@ -30,8 +30,8 @@ class Serial : public eckit::mpi::Comm { template friend class CommBuilder; - Serial(const std::string& name); - Serial(const std::string& name, int); + Serial(std::string_view name); + Serial(std::string_view name, int); ~Serial() override;