From: Christian Heinrich Date: Wed, 6 Dec 2017 13:24:07 +0000 (+0100) Subject: [SMPI] Next step for dynamic privatization X-Git-Tag: v3.19~312^2~52 X-Git-Url: http://info.iut-bm.univ-fcomte.fr/pub/gitweb/simgrid.git/commitdiff_plain/07fa4dbf48f51edfee6c6a56c11095b02e93bf5e [SMPI] Next step for dynamic privatization This commit breaks the following tests: The following tests FAILED: 181 - smpi-msg-masterslave-thread (Failed) 182 - smpi-msg-masterslave-ucontext (Failed) 183 - smpi-msg-masterslave-raw (Failed) To fix this, many more changes will be required. This will be done in the next commit. This commit is just there to make sure changes remain easily understandable (by having small, easy-to-read commits). I changed the following: - Remove variable index_to_process_data - Make process_data a map instead of an array. - Remove the "index" variable for SMPI instances (no longer needed) And cleanup the mess that comes with these changes (delete/free, ...) --- diff --git a/src/smpi/include/private.hpp b/src/smpi/include/private.hpp index 917f72daa1..2eb7847f7c 100644 --- a/src/smpi/include/private.hpp +++ b/src/smpi/include/private.hpp @@ -431,6 +431,8 @@ XBT_PRIVATE int smpi_process_papi_event_set(); #endif extern std::unordered_map location2speedup; +// TODO: Move this to the right location (if we keep this...) +void smpi_add_process(int adjusted_proc_id); /** @brief Returns the last call location (filename, linenumber). Process-specific. */ extern "C" { diff --git a/src/smpi/internals/smpi_deployment.cpp b/src/smpi/internals/smpi_deployment.cpp index 85de568017..7a75f15f26 100644 --- a/src/smpi/internals/smpi_deployment.cpp +++ b/src/smpi/internals/smpi_deployment.cpp @@ -21,16 +21,18 @@ public: : name(name) , size(max_no_processes) , present_processes(0) - , index(process_count) , comm_world(comm) , finalization_barrier(finalization_barrier) { + int cur_process_count = smpi_process_count(); + for (int i = 0; i < max_no_processes; i++) { + smpi_add_process(cur_process_count + i); + } } const char* name; int size; int present_processes; - int index; // Badly named. This should be "no_processes_when_registering" ;) MPI_Comm comm_world; msg_bar_t finalization_barrier; }; @@ -42,7 +44,6 @@ using simgrid::smpi::app::Instance; static std::map smpi_instances; extern int process_count; // How many processes have been allocated over all instances? -extern int* index_to_process_data; /** \ingroup smpi_simulation * \brief Registers a running instance of a MPI program. @@ -80,15 +81,12 @@ void SMPI_app_instance_register(const char *name, xbt_main_func_t code, int num_ void smpi_deployment_register_process(const char* instance_id, int rank, int index) { - if (smpi_instances.empty()) { // no instance registered, we probably used smpirun. - index_to_process_data[index]=index; + if (smpi_instances.empty()) // no instance registered, we probably used smpirun. return; - } Instance& instance = smpi_instances.at(instance_id); instance.present_processes++; - index_to_process_data[index] = instance.index + rank; instance.comm_world->group()->set_mapping(index, rank); } diff --git a/src/smpi/internals/smpi_global.cpp b/src/smpi/internals/smpi_global.cpp index 902d15498e..4ba19caf85 100644 --- a/src/smpi/internals/smpi_global.cpp +++ b/src/smpi/internals/smpi_global.cpp @@ -52,10 +52,9 @@ struct papi_process_data { #endif std::unordered_map location2speedup; -static simgrid::smpi::Process** process_data = nullptr; +static std::map process_data; int process_count = 0; int smpi_universe_size = 0; -int* index_to_process_data = nullptr; extern double smpi_total_benched_time; xbt_os_timer_t global_timer; /** @@ -82,6 +81,12 @@ static simgrid::config::Flag smpi_init_sleep( void (*smpi_comm_copy_data_callback) (smx_activity_t, void*, size_t) = &smpi_comm_copy_buffer_callback; +void smpi_add_process(int smx_process) +{ + process_data.insert( + std::pair(smx_process, new simgrid::smpi::Process(smx_process, nullptr))); +} + int smpi_process_count() { return process_count; @@ -98,7 +103,7 @@ simgrid::smpi::Process* smpi_process() simgrid::smpi::Process* smpi_process_remote(int index) { - return process_data[index_to_process_data[index]]; + return process_data.at(index); } MPI_Comm smpi_process_comm_self(){ @@ -241,7 +246,7 @@ static void smpi_check_options(){ } int smpi_enabled() { - return process_data != nullptr; + return not process_data.empty(); } void smpi_global_init() @@ -340,30 +345,13 @@ void smpi_global_init() } #endif - if (index_to_process_data == nullptr) { - index_to_process_data = new int[SIMIX_process_count()]; - } - - bool smpirun = 0; if (process_count == 0) { // The program has been dispatched but no other // SMPI instances have been registered. We're using smpirun. - smpirun = true; SMPI_app_instance_register(smpi_default_instance_name, nullptr, SIMIX_process_count()); // This call has a side effect on process_count... MPI_COMM_WORLD = *smpi_deployment_comm_world(smpi_default_instance_name); } smpi_universe_size = process_count; - process_data = new simgrid::smpi::Process*[process_count]; - for (int i = 0; i < process_count; i++) { - if (smpirun) { - process_data[i] = new simgrid::smpi::Process(i, smpi_deployment_finalization_barrier(smpi_default_instance_name)); - smpi_deployment_register_process(smpi_default_instance_name, i, i); - } else { - // TODO We can pass a nullptr here because Process::set_data() assigns the - // barrier from the instance anyway. This is ugly and should be changed - process_data[i] = new simgrid::smpi::Process(i, nullptr); - } - } } void smpi_global_destroy() @@ -371,20 +359,18 @@ void smpi_global_destroy() smpi_bench_destroy(); smpi_shared_destroy(); smpi_deployment_cleanup_instances(); - int count = smpi_process_count(); - for (int i = 0; i < count; i++) { - if(process_data[i]->comm_self()!=MPI_COMM_NULL){ - simgrid::smpi::Comm::destroy(process_data[i]->comm_self()); + for (auto& pair : process_data) { + auto& process = pair.second; + if (process->comm_self() != MPI_COMM_NULL) { + simgrid::smpi::Comm::destroy(process->comm_self()); } - if(process_data[i]->comm_intra()!=MPI_COMM_NULL){ - simgrid::smpi::Comm::destroy(process_data[i]->comm_intra()); + if (process->comm_intra() != MPI_COMM_NULL) { + simgrid::smpi::Comm::destroy(process->comm_intra()); } - xbt_os_timer_free(process_data[i]->timer()); - xbt_mutex_destroy(process_data[i]->mailboxes_mutex()); - delete process_data[i]; + xbt_os_timer_free(process->timer()); + xbt_mutex_destroy(process->mailboxes_mutex()); } - delete[] process_data; - process_data = nullptr; + process_data.clear(); if (simgrid::smpi::Colls::smpi_coll_cleanup_callback != nullptr) simgrid::smpi::Colls::smpi_coll_cleanup_callback(); @@ -395,7 +381,6 @@ void smpi_global_destroy() xbt_os_timer_free(global_timer); } - delete[] index_to_process_data; if(smpi_privatize_global_variables == SMPI_PRIVATIZE_MMAP) smpi_destroy_global_memory_segments(); smpi_free_static(); @@ -628,10 +613,9 @@ int smpi_main(const char* executable, int argc, char *argv[]) } } int ret = 0; - int count = smpi_process_count(); - for (int i = 0; i < count; i++) { - if(process_data[i]->return_value()!=0){ - ret=process_data[i]->return_value();//return first non 0 value + for (int i = 0, count = smpi_process_count(); i < count; i++) { + if (process_data.at(i)->return_value() != 0) { + ret = process_data.at(i)->return_value(); // return first non 0 value break; } } diff --git a/src/smpi/internals/smpi_process.cpp b/src/smpi/internals/smpi_process.cpp index 4b55a615b7..2ed9958cf6 100644 --- a/src/smpi/internals/smpi_process.cpp +++ b/src/smpi/internals/smpi_process.cpp @@ -14,8 +14,6 @@ XBT_LOG_NEW_DEFAULT_SUBCATEGORY(smpi_process, smpi, "Logging specific to SMPI (kernel)"); -extern int* index_to_process_data; - #define MAILBOX_NAME_MAXLEN (5 + sizeof(int) * 2 + 1) static char *get_mailbox_name(char *str, int index) @@ -114,11 +112,9 @@ int Process::finalized() /** @brief Check if a process is initialized */ int Process::initialized() { - if (index_to_process_data == nullptr){ - return false; - } else{ - return ((index_ != MPI_UNDEFINED) && (state_ == SMPI_INITIALIZED)); - } + // TODO cheinrich: Check if we still need this. This should be a global condition, not for a + // single process ... ? + return ((index_ != MPI_UNDEFINED) && (state_ == SMPI_INITIALIZED)); } /** @brief Mark a process as initialized (=MPI_Init called) */ @@ -279,10 +275,6 @@ void Process::init(int *argc, char ***argv){ int index = proc->pid - 1; // The maestro process has always ID 0 but we don't need that process here - if(index_to_process_data == nullptr){ - index_to_process_data=static_cast(xbt_malloc(SIMIX_process_count()*sizeof(int))); - } - char* instance_id = (*argv)[1]; try { int rank = std::stoi(std::string((*argv)[2]));