Factorize keyval handling for Comm and Datatype (Win to follow)

diff --git a/include/smpi/forward.hpp b/include/smpi/forward.hpp
index eb15070..acbc310 100644 (file)
--- a/include/smpi/forward.hpp
+++ b/include/smpi/forward.hpp
@@ -13,11 +13,14 @@
 namespace simgrid {
 namespace smpi {
 
+class Coll;
+class Colls;
 class Comm;
 class Datatype;
 class F2C;
 class Group;
 class Info;
+class Keyval;
 class Op;
 class Request;
 class Topo;
@@ -26,9 +29,6 @@ class Topo_Graph;
 class Topo_Dist_Graph;
 class Win;
 
-class Colls;
-class Coll;
-
 }
 }
 

diff --git a/src/smpi/private.h b/src/smpi/private.h
index e2376b8..d1a3799 100644 (file)
--- a/src/smpi/private.h
+++ b/src/smpi/private.h
@@ -15,6 +15,7 @@
 #include "xbt/synchro.h"
 #include "xbt/xbt_os_time.h"
 #include "src/smpi/smpi_f2c.hpp"
+#include "src/smpi/smpi_keyvals.hpp"
 #include "src/smpi/smpi_group.hpp"
 #include "src/smpi/smpi_topo.hpp"
 #include "src/smpi/smpi_coll.hpp"

diff --git a/src/smpi/smpi_comm.cpp b/src/smpi/smpi_comm.cpp
index 8ea2304..7d67ca1 100644 (file)
--- a/src/smpi/smpi_comm.cpp
+++ b/src/smpi/smpi_comm.cpp
@@ -47,8 +47,8 @@ static int smpi_compare_rankmap(const void *a, const void *b)
 namespace simgrid{
 namespace smpi{
 
-xbt_dict_t Comm::keyvals_ = nullptr;
-int Comm::keyval_id_ = 0;//avoid collisions
+std::unordered_map<int, smpi_key_elem> Comm::keyvals_;
+int Comm::keyval_id_=0;
 
 Comm::Comm(MPI_Group group, MPI_Topology topo) : group_(group), topo_(topo)
 {
@@ -89,10 +89,9 @@ int Comm::dup(MPI_Comm* newcomm){
     void* value_in;
     void* value_out;
     xbt_dict_foreach (attributes_, cursor, key, value_in) {
-      smpi_comm_key_elem elem =
-          static_cast<smpi_comm_key_elem>(xbt_dict_get_or_null_ext(keyvals_, key, sizeof(int)));
-      if (elem != nullptr && elem->copy_fn != MPI_NULL_COPY_FN) {
-        ret = elem->copy_fn(this, atoi(key), nullptr, value_in, &value_out, &flag);
+      smpi_key_elem elem = keyvals_.at(*key);
+      if (elem != nullptr && elem->copy_fn.comm_copy_fn != MPI_NULL_COPY_FN) {
+        ret = elem->copy_fn.comm_copy_fn(this, *key, nullptr, value_in, &value_out, &flag);
         if (ret != MPI_SUCCESS) {
           Comm::destroy(*newcomm);
           *newcomm = MPI_COMM_NULL;
@@ -291,9 +290,13 @@ void Comm::cleanup_attributes(){
     void* value;
     int flag;
     xbt_dict_foreach (attributes_, cursor, key, value) {
-      smpi_comm_key_elem elem = static_cast<smpi_comm_key_elem>(xbt_dict_get_or_null(keyvals_, key));
-      if (elem != nullptr && elem->delete_fn != nullptr)
-        elem->delete_fn(this, atoi(key), value, &flag);
+      try{
+        smpi_key_elem elem = keyvals_.at(*key);
+        if (elem != nullptr && elem->delete_fn.comm_delete_fn != nullptr)
+          elem->delete_fn.comm_delete_fn(this, *key, value, &flag);
+      }catch(const std::out_of_range& oor) {
+        //already deleted, not a problem;
+      }
     }
     xbt_dict_free(&attributes_);
   }
@@ -497,15 +500,14 @@ void Comm::init_smp(){
 }
 
 int Comm::attr_delete(int keyval){
-  smpi_comm_key_elem elem =
-     static_cast<smpi_comm_key_elem>(xbt_dict_get_or_null_ext(keyvals_, reinterpret_cast<const char*>(&keyval), sizeof(int)));
+  smpi_key_elem elem = keyvals_.at(keyval);
   if(elem==nullptr)
     return MPI_ERR_ARG;
-  if(elem->delete_fn!=MPI_NULL_DELETE_FN){
+  if(elem->delete_fn.comm_delete_fn!=MPI_NULL_DELETE_FN){
     void* value = nullptr;
     int flag;
     if(this->attr_get(keyval, &value, &flag)==MPI_SUCCESS){
-      int ret = elem->delete_fn(this, keyval, value, &flag);
+      int ret = elem->delete_fn.comm_delete_fn(this, keyval, value, &flag);
       if(ret!=MPI_SUCCESS) 
         return ret;
     }
@@ -518,8 +520,7 @@ int Comm::attr_delete(int keyval){
 }
 
 int Comm::attr_get(int keyval, void* attr_value, int* flag){
-  smpi_comm_key_elem elem =
-    static_cast<smpi_comm_key_elem>(xbt_dict_get_or_null_ext(keyvals_, reinterpret_cast<const char*>(&keyval), sizeof(int)));
+  smpi_key_elem elem = keyvals_.at(keyval);
   if(elem==nullptr)
     return MPI_ERR_ARG;
   if(attributes_==nullptr){
@@ -538,17 +539,14 @@ int Comm::attr_get(int keyval, void* attr_value, int* flag){
 }
 
 int Comm::attr_put(int keyval, void* attr_value){
-  if(keyvals_==nullptr)
-    keyvals_ = xbt_dict_new_homogeneous(nullptr);
-  smpi_comm_key_elem elem =
-    static_cast<smpi_comm_key_elem>(xbt_dict_get_or_null_ext(keyvals_,  reinterpret_cast<const char*>(&keyval), sizeof(int)));
+  smpi_key_elem elem = keyvals_.at(keyval);
   if(elem==nullptr)
     return MPI_ERR_ARG;
   int flag;
   void* value = nullptr;
   this->attr_get(keyval, &value, &flag);
-  if(flag!=0 && elem->delete_fn!=MPI_NULL_DELETE_FN){
-    int ret = elem->delete_fn(this, keyval, value, &flag);
+  if(flag!=0 && elem->delete_fn.comm_delete_fn!=MPI_NULL_DELETE_FN){
+    int ret = elem->delete_fn.comm_delete_fn(this, keyval, value, &flag);
     if(ret!=MPI_SUCCESS) 
       return ret;
   }
@@ -588,37 +586,6 @@ int Comm::add_f() {
   return F2C::f2c_id_-1;
 }
 
-int Comm::keyval_create(MPI_Comm_copy_attr_function* copy_fn, MPI_Comm_delete_attr_function* delete_fn, int* keyval,
-                            void* extra_state){
-  if(keyvals_==nullptr)
-    keyvals_ = xbt_dict_new_homogeneous(nullptr);
-
-  smpi_comm_key_elem value = static_cast<smpi_comm_key_elem>(xbt_new0(s_smpi_mpi_comm_key_elem_t,1));
-
-  value->copy_fn=copy_fn;
-  value->delete_fn=delete_fn;
-
-  *keyval = keyval_id_;
-  xbt_dict_set_ext(keyvals_, reinterpret_cast<const char*>(keyval), sizeof(int),static_cast<void*>(value), nullptr);
-  keyval_id_++;
-  return MPI_SUCCESS;
-}
-
-int Comm::keyval_free(int* keyval){
-  smpi_comm_key_elem elem =
-     static_cast<smpi_comm_key_elem>(xbt_dict_get_or_null_ext(keyvals_,  reinterpret_cast<const char*>(keyval), sizeof(int)));
-  if(elem==nullptr)
-    return MPI_ERR_ARG;
-  xbt_dict_remove_ext(keyvals_,  reinterpret_cast<const char*>(keyval), sizeof(int));
-  xbt_free(elem);
-  return MPI_SUCCESS;
-}
-
-void Comm::keyval_cleanup(){
-  if(Comm::keyvals_!=nullptr) 
-    xbt_dict_free(&Comm::keyvals_);
-}
-
 
 }
 }

diff --git a/src/smpi/smpi_comm.hpp b/src/smpi/smpi_comm.hpp
index 4c2ead0..2c44013 100644 (file)
--- a/src/smpi/smpi_comm.hpp
+++ b/src/smpi/smpi_comm.hpp
@@ -19,7 +19,7 @@ typedef struct s_smpi_mpi_comm_key_elem *smpi_comm_key_elem;
 namespace simgrid{
 namespace smpi{
 
-class Comm : public F2C{
+class Comm : public F2C, public Keyval{
 
   private:
     MPI_Group group_;
@@ -35,14 +35,13 @@ class Comm : public F2C{
     int is_blocked_;// are ranks allocated on the same smp node contiguous ?
     xbt_dict_t attributes_;
 
-    static xbt_dict_t keyvals_;
+  public:
+    static std::unordered_map<int, smpi_key_elem> keyvals_;
     static int keyval_id_;
 
-  public:
     Comm() = default;
     Comm(MPI_Group group, MPI_Topology topo);
 
-
     int dup(MPI_Comm* newcomm);
     MPI_Group group();
     MPI_Topology topo();

diff --git a/src/smpi/smpi_datatype.cpp b/src/smpi/smpi_datatype.cpp
index c33bd1d..086b0dd 100644 (file)
--- a/src/smpi/smpi_datatype.cpp
+++ b/src/smpi/smpi_datatype.cpp
@@ -103,7 +103,7 @@ CREATE_MPI_DATATYPE(MPI_PTR, void*);
 namespace simgrid{
 namespace smpi{
 
-std::unordered_map<int, smpi_type_key_elem> Datatype::keyvals_;
+std::unordered_map<int, smpi_key_elem> Datatype::keyvals_;
 int Datatype::keyval_id_=0;
 
 Datatype::Datatype(int size,MPI_Aint lb, MPI_Aint ub, int flags) : name_(nullptr), size_(size), lb_(lb), ub_(ub), flags_(flags), attributes_(nullptr), refcount_(1){
@@ -135,9 +135,9 @@ Datatype::Datatype(Datatype *datatype, int* ret) : name_(nullptr), lb_(datatype-
     void* value_in;
     void* value_out;
     xbt_dict_foreach (datatype->attributes_, cursor, key, value_in) {
-      smpi_type_key_elem elem = keyvals_.at(atoi(key));
-      if (elem != nullptr && elem->copy_fn != MPI_NULL_COPY_FN) {
-        *ret = elem->copy_fn(datatype, atoi(key), nullptr, value_in, &value_out, &flag);
+      smpi_key_elem elem = keyvals_.at(atoi(key));
+      if (elem != nullptr && elem->copy_fn.type_copy_fn != MPI_NULL_COPY_FN) {
+        *ret = elem->copy_fn.type_copy_fn(datatype, atoi(key), nullptr, value_in, &value_out, &flag);
         if (*ret != MPI_SUCCESS) {
           xbt_dict_cursor_free(&cursor);
           break;
@@ -167,9 +167,13 @@ Datatype::~Datatype(){
     void * value;
     int flag;
     xbt_dict_foreach(attributes_, cursor, key, value){
-      smpi_type_key_elem elem = keyvals_.at(atoi(key));
-      if(elem!=nullptr && elem->delete_fn!=nullptr)
-        elem->delete_fn(this,*key, value, &flag);
+      try{
+        smpi_key_elem elem = keyvals_.at(atoi(key));
+        if(elem!=nullptr && elem->delete_fn.type_delete_fn!=nullptr)
+          elem->delete_fn.type_delete_fn(this,*key, value, &flag);
+      }catch(const std::out_of_range& oor) {
+        //already deleted, not a problem;
+      }
     }
     xbt_dict_free(&attributes_);
   }
@@ -259,14 +263,14 @@ void Datatype::set_name(char* name){
 }
 
 int Datatype::attr_delete(int keyval){
-  smpi_type_key_elem elem = keyvals_.at(keyval);
+  smpi_key_elem elem = keyvals_.at(keyval);
   if(elem==nullptr)
     return MPI_ERR_ARG;
-  if(elem->delete_fn!=MPI_NULL_DELETE_FN){
+  if(elem->delete_fn.type_delete_fn!=MPI_NULL_DELETE_FN){
     void * value = nullptr;
     int flag;
     if(this->attr_get(keyval, &value, &flag)==MPI_SUCCESS){
-      int ret = elem->delete_fn(this, keyval, value, &flag);
+      int ret = elem->delete_fn.type_delete_fn(this, keyval, value, &flag);
       if(ret!=MPI_SUCCESS) 
         return ret;
     }
@@ -280,7 +284,7 @@ int Datatype::attr_delete(int keyval){
 
 
 int Datatype::attr_get(int keyval, void* attr_value, int* flag){
-  smpi_type_key_elem elem = keyvals_.at(keyval);
+  smpi_key_elem elem = keyvals_.at(keyval);
   if(elem==nullptr)
     return MPI_ERR_ARG;
   if(attributes_==nullptr){
@@ -298,14 +302,14 @@ int Datatype::attr_get(int keyval, void* attr_value, int* flag){
 }
 
 int Datatype::attr_put(int keyval, void* attr_value){
-  smpi_type_key_elem elem = keyvals_.at(keyval);
+  smpi_key_elem elem = keyvals_.at(keyval);
   if(elem==nullptr)
     return MPI_ERR_ARG;
   int flag;
   void* value = nullptr;
   this->attr_get(keyval, &value, &flag);
-  if(flag!=0 && elem->delete_fn!=MPI_NULL_DELETE_FN){
-    int ret = elem->delete_fn(this, keyval, value, &flag);
+  if(flag!=0 && elem->delete_fn.type_delete_fn!=MPI_NULL_DELETE_FN){
+    int ret = elem->delete_fn.type_delete_fn(this, keyval, value, &flag);
     if(ret!=MPI_SUCCESS) 
       return ret;
   }
@@ -316,30 +320,6 @@ int Datatype::attr_put(int keyval, void* attr_value){
   return MPI_SUCCESS;
 }
 
-int Datatype::keyval_create(MPI_Type_copy_attr_function* copy_fn, MPI_Type_delete_attr_function* delete_fn, int* keyval, void* extra_state){
-
-  smpi_type_key_elem value = (smpi_type_key_elem) xbt_new0(s_smpi_mpi_type_key_elem_t,1);
-
-  value->copy_fn=copy_fn;
-  value->delete_fn=delete_fn;
-
-  *keyval = keyval_id_;
-  keyvals_.insert({*keyval, value});
-  keyval_id_++;
-  return MPI_SUCCESS;
-}
-
-int Datatype::keyval_free(int* keyval){
-  smpi_type_key_elem elem = keyvals_.at(*keyval);
-  if(elem==0){
-    return MPI_ERR_ARG;
-  }
-  keyvals_.erase(*keyval);
-  xbt_free(elem);
-  return MPI_SUCCESS;
-}
-
-
 int Datatype::pack(void* inbuf, int incount, void* outbuf, int outcount, int* position,MPI_Comm comm){
   if (outcount - *position < incount*static_cast<int>(size_))
     return MPI_ERR_BUFFER;

diff --git a/src/smpi/smpi_datatype.hpp b/src/smpi/smpi_datatype.hpp
index 1cec8fb..29cd8ae 100644 (file)
--- a/src/smpi/smpi_datatype.hpp
+++ b/src/smpi/smpi_datatype.hpp
@@ -10,7 +10,7 @@
 #include <xbt/base.h>
 
 #include "private.h"
-#include <unordered_map>
+
 
 #define DT_FLAG_DESTROYED     0x0001  /**< user destroyed but some other layers still have a reference */
 #define DT_FLAG_COMMITED      0x0002  /**< ready to be used for a send/recv operation */
@@ -32,12 +32,6 @@
 
 extern const MPI_Datatype MPI_PTR;
 
-typedef struct s_smpi_mpi_type_key_elem {
-  MPI_Type_copy_attr_function* copy_fn;
-  MPI_Type_delete_attr_function* delete_fn;
-} s_smpi_mpi_type_key_elem_t; 
-typedef struct s_smpi_mpi_type_key_elem *smpi_type_key_elem;
-
 //The following are datatypes for the MPI functions MPI_MAXLOC and MPI_MINLOC.
 typedef struct {
   float value;
@@ -84,7 +78,7 @@ typedef struct {
 namespace simgrid{
 namespace smpi{
 
-class Datatype : public F2C{
+class Datatype : public F2C, public Keyval{
   protected:
     char* name_;
     size_t size_;
@@ -94,11 +88,9 @@ class Datatype : public F2C{
     xbt_dict_t attributes_;
     int refcount_;
 
-    static std::unordered_map<int, smpi_type_key_elem> keyvals_;
-    static int keyval_id_;
-
   public:
-    static MPI_Datatype null_id_;
+    static std::unordered_map<int, smpi_key_elem> keyvals_;
+    static int keyval_id_;
 
     Datatype(int size,MPI_Aint lb, MPI_Aint ub, int flags);
     Datatype(char* name, int size,MPI_Aint lb, MPI_Aint ub, int flags);

diff --git a/src/smpi/smpi_global.cpp b/src/smpi/smpi_global.cpp
index aa83bc2..25c4ded 100644 (file)
--- a/src/smpi/smpi_global.cpp
+++ b/src/smpi/smpi_global.cpp
@@ -647,7 +647,6 @@ void smpi_global_destroy()
   }
 
   xbt_free(index_to_process_data);
-  Comm::keyval_cleanup();
   if(smpi_privatize_global_variables)
     smpi_destroy_global_memory_segments();
   smpi_free_static();

diff --git a/src/smpi/smpi_pmpi.cpp b/src/smpi/smpi_pmpi.cpp
index 6bd1b8a..73a103e 100644 (file)
--- a/src/smpi/smpi_pmpi.cpp
+++ b/src/smpi/smpi_pmpi.cpp
@@ -2772,11 +2772,15 @@ MPI_Fint PMPI_Info_c2f(MPI_Info info){
 }
 
 int PMPI_Keyval_create(MPI_Copy_function* copy_fn, MPI_Delete_function* delete_fn, int* keyval, void* extra_state) {
-  return Comm::keyval_create(copy_fn, delete_fn, keyval, extra_state);
+  smpi_copy_fn _copy_fn;
+  smpi_delete_fn _delete_fn;
+  _copy_fn.comm_copy_fn = copy_fn;
+  _delete_fn.comm_delete_fn = delete_fn;
+  return Keyval::keyval_create<Comm>(_copy_fn, _delete_fn, keyval, extra_state);
 }
 
 int PMPI_Keyval_free(int* keyval) {
-  return Comm::keyval_free(keyval);
+  return Keyval::keyval_free<Comm>(keyval);
 }
 
 int PMPI_Attr_delete(MPI_Comm comm, int keyval) {
@@ -2890,11 +2894,15 @@ int PMPI_Type_delete_attr (MPI_Datatype type, int type_keyval)
 int PMPI_Type_create_keyval(MPI_Type_copy_attr_function* copy_fn, MPI_Type_delete_attr_function* delete_fn, int* keyval,
                             void* extra_state)
 {
-  return Datatype::keyval_create(copy_fn, delete_fn, keyval, extra_state);
+  smpi_copy_fn _copy_fn;
+  smpi_delete_fn _delete_fn;
+  _copy_fn.type_copy_fn = copy_fn;
+  _delete_fn.type_delete_fn = delete_fn;
+  return Keyval::keyval_create<Datatype>(_copy_fn, _delete_fn, keyval, extra_state);
 }
 
 int PMPI_Type_free_keyval(int* keyval) {
-  return Datatype::keyval_free(keyval);
+  return Keyval::keyval_free<Datatype>(keyval);
 }
 
 int PMPI_Info_create( MPI_Info *info){

diff --git a/tools/cmake/DefinePackages.cmake b/tools/cmake/DefinePackages.cmake
index ece7b31..9bd28d7 100644 (file)
--- a/tools/cmake/DefinePackages.cmake
+++ b/tools/cmake/DefinePackages.cmake
@@ -225,6 +225,7 @@ set(SMPI_SRC
   src/smpi/smpi_datatype.hpp
   src/smpi/smpi_info.cpp
   src/smpi/smpi_info.hpp
+  src/smpi/smpi_keyvals.hpp
   src/smpi/smpi_datatype_derived.cpp
   src/smpi/smpi_datatype_derived.hpp
   src/smpi/smpi_op.cpp