From 5334d22a37c9f83aefeaed4a4b05f17c456b0d90 Mon Sep 17 00:00:00 2001 From: Nathan Hjelm Date: Mon, 30 Nov 2015 23:03:47 -0700 Subject: [PATCH] ompi/group: release ompi_proc_t's at group destruction This commit changes the way ompi_proc_t's are retained/released by ompi_group_t's. Before this change ompi_proc_t's were retained once for the group and then once for each retain of a group. This method adds unnecessary overhead (need to traverse the group list each time the group is retained) and causes problems when using an async add_procs. Signed-off-by: Nathan Hjelm --- ompi/communicator/comm.c | 11 ----------- ompi/communicator/comm_init.c | 2 -- ompi/dpm/dpm.c | 6 ++---- ompi/group/group.c | 1 - ompi/group/group_init.c | 2 ++ ompi/mca/osc/rdma/osc_rdma_active_target.c | 5 ----- ompi/mpi/c/comm_remote_group.c | 1 - ompi/mpi/c/intercomm_create.c | 4 +--- ompi/mpi/c/intercomm_merge.c | 1 - ompi/win/win.c | 3 --- 10 files changed, 5 insertions(+), 31 deletions(-) diff --git a/ompi/communicator/comm.c b/ompi/communicator/comm.c index 538f4058a9..ea61f04c7f 100644 --- a/ompi/communicator/comm.c +++ b/ompi/communicator/comm.c @@ -172,7 +172,6 @@ int ompi_comm_set_nb ( ompi_communicator_t **ncomm, } else { newcomm->c_local_group = local_group; OBJ_RETAIN(newcomm->c_local_group); - ompi_group_increment_proc_count(newcomm->c_local_group); } newcomm->c_my_rank = newcomm->c_local_group->grp_my_rank; @@ -189,7 +188,6 @@ int ompi_comm_set_nb ( ompi_communicator_t **ncomm, } else { newcomm->c_remote_group = remote_group; OBJ_RETAIN(newcomm->c_remote_group); - ompi_group_increment_proc_count(newcomm->c_remote_group); } newcomm->c_flags |= OMPI_COMM_INTER; @@ -256,9 +254,6 @@ int ompi_comm_group ( ompi_communicator_t* comm, ompi_group_t **group ) /* increment reference counters for the group */ OBJ_RETAIN(comm->c_local_group); - /* increase also the reference counter for the procs */ - ompi_group_increment_proc_count(comm->c_local_group); - *group = comm->c_local_group; return OMPI_SUCCESS; } @@ -570,8 +565,6 @@ int ompi_comm_split( ompi_communicator_t* comm, int color, int key, goto exit; } - ompi_group_increment_proc_count(local_group); - mode = OMPI_COMM_CID_INTER; } else { rranks = NULL; @@ -603,7 +596,6 @@ int ompi_comm_split( ompi_communicator_t* comm, int color, int key, } if ( inter ) { - ompi_group_decrement_proc_count (local_group); OBJ_RELEASE(local_group); if (NULL != newcomp->c_local_comm) { snprintf(newcomp->c_local_comm->c_name, MPI_MAX_OBJECT_NAME, @@ -2006,9 +1998,6 @@ static int ompi_comm_fill_rest(ompi_communicator_t *comm, comm->c_remote_group = comm->c_local_group; OBJ_RETAIN( comm->c_remote_group ); - /* retain these proc pointers */ - ompi_group_increment_proc_count(comm->c_local_group); - /* set the rank information */ comm->c_local_group->grp_my_rank = my_rank; comm->c_my_rank = my_rank; diff --git a/ompi/communicator/comm_init.c b/ompi/communicator/comm_init.c index a7f302bbd4..b2200bdb71 100644 --- a/ompi/communicator/comm_init.c +++ b/ompi/communicator/comm_init.c @@ -425,7 +425,6 @@ static void ompi_comm_destruct(ompi_communicator_t* comm) } if (NULL != comm->c_local_group) { - ompi_group_decrement_proc_count (comm->c_local_group); OBJ_RELEASE ( comm->c_local_group ); comm->c_local_group = NULL; if ( OMPI_COMM_IS_INTRA(comm) ) { @@ -438,7 +437,6 @@ static void ompi_comm_destruct(ompi_communicator_t* comm) } if (NULL != comm->c_remote_group) { - ompi_group_decrement_proc_count (comm->c_remote_group); OBJ_RELEASE ( comm->c_remote_group ); comm->c_remote_group = NULL; } diff --git a/ompi/dpm/dpm.c b/ompi/dpm/dpm.c index ab6c3d4922..9a236d01b9 100644 --- a/ompi/dpm/dpm.c +++ b/ompi/dpm/dpm.c @@ -441,12 +441,11 @@ int ompi_dpm_connect_accept(ompi_communicator_t *comm, int root, i=0; OPAL_LIST_FOREACH(cd, &rlist, ompi_dpm_proct_caddy_t) { new_group_pointer->grp_proc_pointers[i++] = cd->p; + /* retain the proc */ + OBJ_RETAIN(cd->p); } OPAL_LIST_DESTRUCT(&rlist); - /* increment proc reference counters */ - ompi_group_increment_proc_count(new_group_pointer); - /* set up communicator structure */ rc = ompi_comm_set ( &newcomp, /* new comm */ comm, /* old comm */ @@ -465,7 +464,6 @@ int ompi_dpm_connect_accept(ompi_communicator_t *comm, int root, goto exit; } - ompi_group_decrement_proc_count (new_group_pointer); OBJ_RELEASE(new_group_pointer); new_group_pointer = MPI_GROUP_NULL; diff --git a/ompi/group/group.c b/ompi/group/group.c index e5e2540fd7..d88996590d 100644 --- a/ompi/group/group.c +++ b/ompi/group/group.c @@ -37,7 +37,6 @@ int ompi_group_free ( ompi_group_t **group ) ompi_group_t *l_group; l_group = (ompi_group_t *) *group; - ompi_group_decrement_proc_count (l_group); OBJ_RELEASE(l_group); *group = MPI_GROUP_NULL; diff --git a/ompi/group/group_init.c b/ompi/group/group_init.c index 5352493c4f..4054b32195 100644 --- a/ompi/group/group_init.c +++ b/ompi/group/group_init.c @@ -266,6 +266,8 @@ static void ompi_group_destruct(ompi_group_t *group) the proc counts are not increased during the constructor, either). */ + ompi_group_decrement_proc_count (group); + /* release thegrp_proc_pointers memory */ if (NULL != group->grp_proc_pointers) { free(group->grp_proc_pointers); diff --git a/ompi/mca/osc/rdma/osc_rdma_active_target.c b/ompi/mca/osc/rdma/osc_rdma_active_target.c index ef0b018409..2e9a8a21c4 100644 --- a/ompi/mca/osc/rdma/osc_rdma_active_target.c +++ b/ompi/mca/osc/rdma/osc_rdma_active_target.c @@ -193,7 +193,6 @@ int ompi_osc_rdma_post_atomic (ompi_group_t *group, int assert, ompi_win_t *win) /* save the group */ OBJ_RETAIN(group); - ompi_group_increment_proc_count(group); OPAL_THREAD_LOCK(&module->lock); @@ -371,7 +370,6 @@ int ompi_osc_rdma_start_atomic (ompi_group_t *group, int assert, ompi_win_t *win /* save the group */ OBJ_RETAIN(group); - ompi_group_increment_proc_count(group); if (!(assert & MPI_MODE_NOCHECK)) { /* look through list of pending posts */ @@ -440,7 +438,6 @@ int ompi_osc_rdma_complete_atomic (ompi_win_t *win) sync->epoch_active = false; /* phase 2 cleanup group */ - ompi_group_decrement_proc_count(group); OBJ_RELEASE(group); peers = sync->peer_list.peers; @@ -526,7 +523,6 @@ int ompi_osc_rdma_wait_atomic (ompi_win_t *win) module->pw_group = NULL; OPAL_THREAD_UNLOCK(&module->lock); - ompi_group_decrement_proc_count(group); OBJ_RELEASE(group); OSC_RDMA_VERBOSE(MCA_BASE_VERBOSE_TRACE, "wait complete"); @@ -571,7 +567,6 @@ int ompi_osc_rdma_test_atomic (ompi_win_t *win, int *flag) module->pw_group = NULL; OPAL_THREAD_UNLOCK(&(module->lock)); - ompi_group_decrement_proc_count(group); OBJ_RELEASE(group); return OMPI_SUCCESS; diff --git a/ompi/mpi/c/comm_remote_group.c b/ompi/mpi/c/comm_remote_group.c index 7ec955c8f0..576ed5a6fa 100644 --- a/ompi/mpi/c/comm_remote_group.c +++ b/ompi/mpi/c/comm_remote_group.c @@ -69,6 +69,5 @@ int MPI_Comm_remote_group(MPI_Comm comm, MPI_Group *group) } *group = (MPI_Group) comm->c_remote_group; - ompi_group_increment_proc_count(*group); return MPI_SUCCESS; } diff --git a/ompi/mpi/c/intercomm_create.c b/ompi/mpi/c/intercomm_create.c index 1da9b55c57..8346a75ec1 100644 --- a/ompi/mpi/c/intercomm_create.c +++ b/ompi/mpi/c/intercomm_create.c @@ -171,10 +171,9 @@ int MPI_Intercomm_create(MPI_Comm local_comm, int local_leader, /* put group elements in the list */ for (j = 0; j < rsize; j++) { new_group_pointer->grp_proc_pointers[j] = rprocs[j]; + OBJ_RETAIN(rprocs[j]); } - ompi_group_increment_proc_count(new_group_pointer); - rc = ompi_comm_set ( &newcomp, /* new comm */ local_comm, /* old comm */ local_comm->c_local_group->grp_proc_count, /* local_size */ @@ -196,7 +195,6 @@ int MPI_Intercomm_create(MPI_Comm local_comm, int local_leader, goto err_exit; } - ompi_group_decrement_proc_count (new_group_pointer); OBJ_RELEASE(new_group_pointer); new_group_pointer = MPI_GROUP_NULL; diff --git a/ompi/mpi/c/intercomm_merge.c b/ompi/mpi/c/intercomm_merge.c index 64a6d476b4..b0cfb2dcde 100644 --- a/ompi/mpi/c/intercomm_merge.c +++ b/ompi/mpi/c/intercomm_merge.c @@ -114,7 +114,6 @@ int MPI_Intercomm_merge(MPI_Comm intercomm, int high, goto exit; } - ompi_group_decrement_proc_count(new_group_pointer); OBJ_RELEASE(new_group_pointer); new_group_pointer = MPI_GROUP_NULL; diff --git a/ompi/win/win.c b/ompi/win/win.c index a4629a3a42..6a371e6973 100644 --- a/ompi/win/win.c +++ b/ompi/win/win.c @@ -145,7 +145,6 @@ static int alloc_window(struct ompi_communicator_t *comm, ompi_info_t *info, int /* setup data that is independent of osc component */ group = comm->c_local_group; OBJ_RETAIN(group); - ompi_group_increment_proc_count(group); win->w_group = group; *win_out = win; @@ -366,7 +365,6 @@ ompi_win_get_name(ompi_win_t *win, char *win_name, int *length) int ompi_win_group(ompi_win_t *win, ompi_group_t **group) { OBJ_RETAIN(win->w_group); - ompi_group_increment_proc_count(win->w_group); *group = win->w_group; return OMPI_SUCCESS; @@ -406,7 +404,6 @@ ompi_win_destruct(ompi_win_t *win) } if (NULL != win->w_group) { - ompi_group_decrement_proc_count(win->w_group); OBJ_RELEASE(win->w_group); }