Merge pull request #118 from ecmwf/feature/eckit_mpi_with_string_view

Use std::string_view in eckit_mpi

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<Mutex> lock(mutex_);
 
-        std::map<std::string, Comm*>::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<Mutex> lock(mutex_);
         std::map<std::string, Comm*>::iterator itr = communicators.find(name);
         if (itr != communicators.end()) {
@@ -120,30 +120,30 @@ class Environment {
     void finaliseAllComms() {
         AutoLock<Mutex> lock(mutex_);
 
-        std::map<std::string, Comm*>::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<Mutex> lock(mutex_);
 
-        if (!name && default_) {
+        if (name.empty() && default_) {
             return *default_; /* most common case first */
         }
 
         if (!default_) {
             initDefault();
         }
 
-        if (!name) {
+        if (name.empty()) {
             ASSERT(default_); /* sanity check init was successful */
             return *default_;
         }
 
-        std::map<std::string, Comm*>::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<Mutex> 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<Mutex> 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<Mutex> 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<std::string, Comm*> communicators;
+    std::map<std::string, Comm*, std::less<>> 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<Mutex> 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<Mutex> lock(mutex_);
-        factories.erase(builder);
+        factories.erase(std::string{builder});
     }
 
-    CommFactory& getFactory(const std::string& builder) {
+    CommFactory& getFactory(std::string_view builder) const {
         AutoLock<Mutex> lock(mutex_);
 
-        std::map<std::string, CommFactory*>::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<std::string, CommFactory*> factories;
-    eckit::Mutex mutex_;
+    std::map<std::string, CommFactory*, std::less<>> 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);
 }
 

src/eckit/mpi/Comm.h

@@ -14,6 +14,7 @@
 #include <cstddef>
 #include <iterator>
 #include <string>
+#include <string_view>
 #include <vector>
 
 #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<std::string> 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 T>
 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) :

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);

src/eckit/mpi/Parallel.h

@@ -29,9 +29,9 @@ class Parallel : public eckit::mpi::Comm {
     template <class T>
     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;