Merge pull request #2011 from bradking/comm-mpi-ownership

helper: Do not always duplicate MPI_Comm on Comm construction

bindings/C/adios2/c/adios2_c_adios_mpi.cpp

@@ -30,7 +30,7 @@ adios2_adios *adios2_init_config_glue_mpi(const char *config_file,
         const bool debugBool =
             (debug_mode == adios2_debug_mode_on) ? true : false;
         adios = reinterpret_cast<adios2_adios *>(new adios2::core::ADIOS(
-            config_file, adios2::helper::CommFromMPI(comm), debugBool,
+            config_file, adios2::helper::CommDupMPI(comm), debugBool,
             host_language));
     }
     catch (...)

bindings/C/adios2/c/adios2_c_io_mpi.cpp

@@ -29,7 +29,7 @@ adios2_engine *adios2_open_new_comm(adios2_io *io, const char *name,
         engine = reinterpret_cast<adios2_engine *>(
             &reinterpret_cast<adios2::core::IO *>(io)->Open(
                 name, adios2_ToOpenMode(mode),
-                adios2::helper::CommFromMPI(comm)));
+                adios2::helper::CommDupMPI(comm)));
     }
     catch (...)
     {

bindings/CXX11/adios2/cxx11/ADIOSMPI.cpp

@@ -13,7 +13,7 @@
 namespace adios2
 {
 ADIOS::ADIOS(const std::string &configFile, MPI_Comm comm, const bool debugMode)
-: m_ADIOS(std::make_shared<core::ADIOS>(configFile, helper::CommFromMPI(comm),
+: m_ADIOS(std::make_shared<core::ADIOS>(configFile, helper::CommDupMPI(comm),
                                         debugMode, "C++"))
 {
 }

bindings/CXX11/adios2/cxx11/IOMPI.cpp

@@ -18,7 +18,7 @@ Engine IO::Open(const std::string &name, const Mode mode, MPI_Comm comm)
 {
     helper::CheckForNullptr(m_IO,
                             "for engine " + name + ", in call to IO::Open");
-    return Engine(&m_IO->Open(name, mode, helper::CommFromMPI(comm)));
+    return Engine(&m_IO->Open(name, mode, helper::CommDupMPI(comm)));
 }
 
 } // end namespace adios2

bindings/CXX11/adios2/cxx11/fstream/ADIOS2fstreamMPI.cpp

@@ -16,7 +16,7 @@ namespace adios2
 fstream::fstream(const std::string &name, const openmode mode, MPI_Comm comm,
                  const std::string engineType)
 : m_Stream(std::make_shared<core::Stream>(
-      name, ToMode(mode), helper::CommFromMPI(comm), engineType, "C++"))
+      name, ToMode(mode), helper::CommDupMPI(comm), engineType, "C++"))
 {
 }
 
@@ -24,7 +24,7 @@ fstream::fstream(const std::string &name, const openmode mode, MPI_Comm comm,
                  const std::string &configFile,
                  const std::string ioInConfigFile)
 : m_Stream(std::make_shared<core::Stream>(name, ToMode(mode),
-                                          helper::CommFromMPI(comm), configFile,
+                                          helper::CommDupMPI(comm), configFile,
                                           ioInConfigFile, "C++"))
 {
 }
@@ -34,7 +34,7 @@ void fstream::open(const std::string &name, const openmode mode, MPI_Comm comm,
 {
     CheckOpen(name);
     m_Stream = std::make_shared<core::Stream>(
-        name, ToMode(mode), helper::CommFromMPI(comm), engineType, "C++");
+        name, ToMode(mode), helper::CommDupMPI(comm), engineType, "C++");
 }
 
 void fstream::open(const std::string &name, const openmode mode, MPI_Comm comm,
@@ -43,7 +43,7 @@ void fstream::open(const std::string &name, const openmode mode, MPI_Comm comm,
 {
     CheckOpen(name);
     m_Stream = std::make_shared<core::Stream>(
-        name, ToMode(mode), helper::CommFromMPI(comm), configFile,
+        name, ToMode(mode), helper::CommDupMPI(comm), configFile,
         ioInConfigFile, "C++");
 }
 

bindings/Python/py11ADIOS.cpp

@@ -23,7 +23,7 @@ namespace py11
 ADIOS::ADIOS(const std::string &configFile, MPI4PY_Comm mpiComm,
              const bool debugMode)
 : m_ADIOS(std::make_shared<adios2::core::ADIOS>(
-      configFile, helper::CommFromMPI(mpiComm), debugMode, "Python"))
+      configFile, helper::CommDupMPI(mpiComm), debugMode, "Python"))
 {
 }
 

bindings/Python/py11File.cpp

@@ -34,7 +34,7 @@ File::File(const std::string &name, const std::string mode, MPI_Comm comm,
            const std::string engineType)
 : m_Name(name), m_Mode(mode),
   m_Stream(std::make_shared<core::Stream>(
-      name, ToMode(mode), helper::CommFromMPI(comm), engineType, "Python"))
+      name, ToMode(mode), helper::CommDupMPI(comm), engineType, "Python"))
 {
 }
 
@@ -42,7 +42,7 @@ File::File(const std::string &name, const std::string mode, MPI_Comm comm,
            const std::string &configFile, const std::string ioInConfigFile)
 : m_Name(name), m_Mode(mode),
   m_Stream(std::make_shared<core::Stream>(name, ToMode(mode),
-                                          helper::CommFromMPI(comm), configFile,
+                                          helper::CommDupMPI(comm), configFile,
                                           ioInConfigFile, "Python"))
 {
 }

bindings/Python/py11IO.cpp

@@ -254,7 +254,7 @@ Engine IO::Open(const std::string &name, const int mode, MPI4PY_Comm comm)
                             "for engine " + name + ", in call to IO::Open");
 
     return Engine(&m_IO->Open(name, static_cast<adios2::Mode>(mode),
-                              helper::CommFromMPI(comm)));
+                              helper::CommDupMPI(comm)));
 }
 #endif
 

source/adios2/helper/adiosCommMPI.cpp

@@ -567,19 +567,31 @@ Comm::Status CommReqImplMPI::Wait(const std::string &hint)
     return status;
 }
 
-Comm CommFromMPI(MPI_Comm mpiComm)
+Comm CommWithMPI(MPI_Comm mpiComm)
 {
     static InitMPI const initMPI;
     if (mpiComm == MPI_COMM_NULL)
     {
         return CommDummy();
     }
-    MPI_Comm newComm;
-    MPI_Comm_dup(mpiComm, &newComm);
-    auto comm = std::unique_ptr<CommImpl>(new CommImplMPI(newComm));
+    auto comm = std::unique_ptr<CommImpl>(new CommImplMPI(mpiComm));
     return CommImpl::MakeComm(std::move(comm));
 }
 
+Comm CommDupMPI(MPI_Comm mpiComm)
+{
+    MPI_Comm newComm;
+    if (mpiComm != MPI_COMM_NULL)
+    {
+        MPI_Comm_dup(mpiComm, &newComm);
+    }
+    else
+    {
+        newComm = MPI_COMM_NULL;
+    }
+    return CommWithMPI(newComm);
+}
+
 MPI_Comm CommAsMPI(Comm const &comm)
 {
     if (CommImplMPI *mpi = dynamic_cast<CommImplMPI *>(CommImpl::Get(comm)))

source/adios2/helper/adiosCommMPI.h

@@ -22,10 +22,15 @@ namespace adios2
 namespace helper
 {
 
+/**
+ * @brief Create a communicator by taking ownership of a MPI communicator.
+ */
+Comm CommWithMPI(MPI_Comm mpiComm);
+
 /**
  * @brief Create a communicator by duplicating a MPI communicator.
  */
-Comm CommFromMPI(MPI_Comm mpiComm);
+Comm CommDupMPI(MPI_Comm mpiComm);
 
 /**
  * @brief Get the underlying raw MPI communicator.

source/adios2/toolkit/sst/util/sst_conn_tool.cxx

@@ -7,7 +7,7 @@
 #ifdef ADIOS2_HAVE_MPI
 #include "adios2/helper/adiosCommMPI.h"
 static adios2::helper::Comm CommWorld =
-    adios2::helper::CommFromMPI(MPI_COMM_WORLD);
+    adios2::helper::CommWithMPI(MPI_COMM_WORLD);
 #else
 #include "adios2/helper/adiosCommDummy.h"
 static adios2::helper::Comm CommWorld = adios2::helper::CommDummy();

source/utils/adios_reorganize/Reorganize.cpp

@@ -52,7 +52,7 @@ Reorganize::Reorganize(int argc, char *argv[])
 {
 #ifdef ADIOS2_HAVE_MPI
     {
-        auto commWorld = helper::CommFromMPI(MPI_COMM_WORLD);
+        auto commWorld = helper::CommWithMPI(MPI_COMM_WORLD);
         m_Comm = commWorld.Split(m_CommSplitColor, 0);
     }
 #else