From 8b3b8b94b252c69516b8ef4ba97596b05033d9ea Mon Sep 17 00:00:00 2001 From: degomme Date: Wed, 15 Mar 2017 14:30:52 +0100 Subject: [PATCH] Factorize keyval handling for Comm and Datatype (Win to follow) --- include/smpi/forward.hpp | 6 +-- src/smpi/private.h | 1 + src/smpi/smpi_comm.cpp | 71 +++++++++----------------------- src/smpi/smpi_comm.hpp | 7 ++-- src/smpi/smpi_datatype.cpp | 56 ++++++++----------------- src/smpi/smpi_datatype.hpp | 16 ++----- src/smpi/smpi_global.cpp | 1 - src/smpi/smpi_pmpi.cpp | 16 +++++-- tools/cmake/DefinePackages.cmake | 1 + 9 files changed, 61 insertions(+), 114 deletions(-) diff --git a/include/smpi/forward.hpp b/include/smpi/forward.hpp index eb150705dd..acbc310140 100644 --- 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 e2376b8a55..d1a37995fb 100644 --- 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 8ea2304215..7d67ca1cf5 100644 --- 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 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(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(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(xbt_dict_get_or_null_ext(keyvals_, reinterpret_cast(&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(xbt_dict_get_or_null_ext(keyvals_, reinterpret_cast(&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(xbt_dict_get_or_null_ext(keyvals_, reinterpret_cast(&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(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(keyval), sizeof(int),static_cast(value), nullptr); - keyval_id_++; - return MPI_SUCCESS; -} - -int Comm::keyval_free(int* keyval){ - smpi_comm_key_elem elem = - static_cast(xbt_dict_get_or_null_ext(keyvals_, reinterpret_cast(keyval), sizeof(int))); - if(elem==nullptr) - return MPI_ERR_ARG; - xbt_dict_remove_ext(keyvals_, reinterpret_cast(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 4c2ead02c7..2c4401394a 100644 --- 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 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 c33bd1d170..086b0ddb65 100644 --- 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 Datatype::keyvals_; +std::unordered_map 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(size_)) return MPI_ERR_BUFFER; diff --git a/src/smpi/smpi_datatype.hpp b/src/smpi/smpi_datatype.hpp index 1cec8fb33d..29cd8aee6c 100644 --- a/src/smpi/smpi_datatype.hpp +++ b/src/smpi/smpi_datatype.hpp @@ -10,7 +10,7 @@ #include #include "private.h" -#include + #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 keyvals_; - static int keyval_id_; - public: - static MPI_Datatype null_id_; + static std::unordered_map 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 aa83bc2137..25c4deddd4 100644 --- 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 6bd1b8af3f..73a103eabf 100644 --- 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(_copy_fn, _delete_fn, keyval, extra_state); } int PMPI_Keyval_free(int* keyval) { - return Comm::keyval_free(keyval); + return Keyval::keyval_free(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(_copy_fn, _delete_fn, keyval, extra_state); } int PMPI_Type_free_keyval(int* keyval) { - return Datatype::keyval_free(keyval); + return Keyval::keyval_free(keyval); } int PMPI_Info_create( MPI_Info *info){ diff --git a/tools/cmake/DefinePackages.cmake b/tools/cmake/DefinePackages.cmake index ece7b316a7..9bd28d7f82 100644 --- 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 -- 2.20.1