second part of the 'running out of cids problem', this time focusing on what

happens when hierarch is used. . Two major items:
 - modify the comm_activate step to take an additional argument, indicating
 whether the new communicatio has to go through the collective selection
 step. This is not required sometimes (e.g. when a process calls
 MPI_COMM_SPLIT with color=MPI_UNDEFINED), and contributed significantly to
 the exhaustion of cids.
 - when freeing a communicator, check whether we can reuse the block of cids
 assigned to that comm. This only works if the current front of the cid
 assignment (cid_block_start) is right ater the block of cids assigned to this
 comm.

Fixes trac:1904
Fixes trac:1926

This commit was SVN r21296.

The following Trac tickets were found above:
  Ticket 1904 --> https://svn.open-mpi.org/trac/ompi/ticket/1904
  Ticket 1926 --> https://svn.open-mpi.org/trac/ompi/ticket/1926

ompi/communicator/comm.c

@@ -9,7 +9,7 @@
  *                         University of Stuttgart.  All rights reserved.
  * Copyright (c) 2004-2005 The Regents of the University of California.
  *                         All rights reserved.
- * Copyright (c) 2007-2008 University of Houston. All rights reserved.
+ * Copyright (c) 2007-2009 University of Houston. All rights reserved.
  * Copyright (c) 2007-2008 Cisco Systems, Inc. All rights reserved.
  * Copyright (c) 2009      Sun Microsystems, Inc.  All rights reserved.
  * $COPYRIGHT$
@@ -350,7 +350,7 @@ int ompi_comm_create ( ompi_communicator_t *comm, ompi_group_t *group,
              newcomp->c_contextid, comm->c_contextid );
 
     /* Activate the communicator and init coll-component */
-    rc = ompi_comm_activate( &newcomp );  /* new communicator */ 
+    rc = ompi_comm_activate( &newcomp, 1 );  /* new communicator */ 
     if ( OMPI_SUCCESS != rc ) {
         goto exit;
     }
@@ -573,7 +573,17 @@ int ompi_comm_split ( ompi_communicator_t* comm, int color, int key,
              newcomp->c_contextid, comm->c_contextid );
 
     /* Activate the communicator and init coll-component */
-    rc = ompi_comm_activate( &newcomp );  /* new communicator */ 
+    if ( MPI_UNDEFINED != color ) {
+	rc = ompi_comm_activate( &newcomp, 1 );  /* new communicator */ 
+    }
+    else {
+	/* 
+        ** this should prevent creating the subcommunicators
+        ** of the hierarch module for the cases where we
+        ** know that they will be freed anyway (e.g. color = MPI_UNDEFINED
+	*/
+	rc = ompi_comm_activate( &newcomp, 0 );  /* new communicator */ 
+    }
     if ( OMPI_SUCCESS != rc ) {
         goto exit;
     }
@@ -664,7 +674,7 @@ int ompi_comm_dup ( ompi_communicator_t * comm, ompi_communicator_t **newcomm )
              newcomp->c_contextid, comm->c_contextid );
 
     /* activate communicator and init coll-module */
-    rc = ompi_comm_activate( &newcomp );  /* new communicator */ 
+    rc = ompi_comm_activate( &newcomp, 1 );  /* new communicator */ 
     if ( OMPI_SUCCESS != rc ) {
         return rc;
     }
@@ -1501,13 +1511,17 @@ int ompi_topo_create (ompi_communicator_t *old_comm,
         return ret;
     }
 
-    ret = ompi_comm_activate( &new_comm );  /* new communicator */
+    if ( MPI_UNDEFINED != new_rank ) {
-    if (OMPI_SUCCESS != ret) {
+	ret = ompi_comm_activate( &new_comm, 1 );  /* new communicator */
-        /* something wrong happened during setting the communicator */
+    }
-        *comm_topo = new_comm;
+    else {
-        return ret;
+	ret = ompi_comm_activate( &new_comm, 0 );  /* new communicator */
+    }
+    if (OMPI_SUCCESS != ret) {
+	/* something wrong happened during setting the communicator */
+	*comm_topo = new_comm;
+	return ret;
     }
-
     
     /* if the returned rank is -1, then this process is not in the 
      * new topology, so free everything we have allocated and return */

ompi/communicator/comm_cid.c

@@ -50,6 +50,7 @@ BEGIN_C_DECLS
 static int cid_block_start = 28;
 
 static int ompi_comm_cid_checkforreuse ( int c_id_start_index, int block );
+static int ompi_comm_get_blocksize ( ompi_communicator_t* comm  );
 
 
 typedef int ompi_comm_cid_allredfct (int *inbuf, int* outbuf, 
@@ -131,6 +132,7 @@ int ompi_comm_nextcid ( ompi_communicator_t* newcomm,
      * Determine which implementation of allreduce we have to use
      * for the current scenario 
      */
+
     switch (mode) 
     {
         case OMPI_COMM_CID_INTRA: 
@@ -245,7 +247,7 @@ int ompi_comm_nextcid ( ompi_communicator_t* newcomm,
       * In case the communication mode is INTRA_OOB or INTAR_BRIDGE, we use the 
       * highest-free algorithm
       */
-    if ( OMPI_COMM_CID_INTRA_OOB == mode || OMPI_COMM_CID_INTRA_BRIDGE == mode) {
+    if ( OMPI_COMM_CID_INTRA_OOB == mode || OMPI_COMM_CID_INTRA_BRIDGE == mode) {       
         (allredfnct)(&cid_block_start, &global_block_start, 1, 
                      MPI_MAX, comm, bridgecomm,
                      local_leader, remote_leader, send_first );
@@ -254,66 +256,59 @@ int ompi_comm_nextcid ( ompi_communicator_t* newcomm,
         cid_block_start = cid_block_start + 1;
     }
     else {
-        flag=false;
+    
-        block = 0;
+        block = ompi_comm_get_blocksize ( comm  ); 
-        if( 0 == comm->c_contextid ) {
-            block = OMPI_COMM_BLOCK_WORLD;
-        }
-        else {
-            block = OMPI_COMM_BLOCK_OTHERS;
-        }
 
-        while(!flag) {
+	/**
-            /**
+	 * If the communicator has IDs available then allocate one for the child
-             * If the communicator has IDs available then allocate one for the child
+	 */
-             */
+	if ( MPI_UNDEFINED != comm->c_id_available && 
-            if ( MPI_UNDEFINED != comm->c_id_available && 
+	     MPI_UNDEFINED != comm->c_id_start_index &&  
-                 MPI_UNDEFINED != comm->c_id_start_index &&  
+	     block > comm->c_id_available - comm->c_id_start_index) {
-                 block > comm->c_id_available - comm->c_id_start_index) {
+	    nextcid = comm->c_id_available;
-                nextcid = comm->c_id_available;
+	    flag=opal_pointer_array_test_and_set_item (&ompi_mpi_communicators,
-                flag=opal_pointer_array_test_and_set_item (&ompi_mpi_communicators,
+						       nextcid, comm);
-                                                           nextcid, comm);
+	}
-            }
+	/**
-            /**
+	 * Otherwise the communicator needs to negotiate a new block of IDs
-             * Otherwise the communicator needs to negotiate a new block of IDs
+	 */
-             */
+	else {
-            else {
+	    int start[3], gstart[3];
-		int start[3], gstart[3];
+	    /* the next function either returns exactly the same start_id as 
-		/* the next function either returns exactly the same start_id as 
+	       the communicator had, or the cid_block_start*/
-                   the communicator had, or the cid_block_start*/
+	    start[0] = ompi_comm_cid_checkforreuse ( comm->c_id_start_index, block );
-		start[0] = ompi_comm_cid_checkforreuse ( comm->c_id_start_index, block );
+	    
-
+	    /* this is now a little tricky. By multiplying the start[0] values with -1
-		/* this is now a little tricky. By multiplying the start[0] values with -1
+	       and executing the MAX operation on those as well, we will be able to
-		   and executing the MAX operation on those as well, we will be able to
+	       determine the minimum value across the provided input */ 
-		   determine the minimum value across the provided input */ 
+	    start[1] = (-1) * start[0]; 
-		start[1] = (-1) * start[0]; 
+	    start[2] = cid_block_start;
-		start[2] = cid_block_start;
+	    
-
+	    (allredfnct)(start, gstart, 3, MPI_MAX, comm, bridgecomm,
-		(allredfnct)(start, gstart, 3, MPI_MAX, comm, bridgecomm,
+			 local_leader, remote_leader, send_first );
-                             local_leader, remote_leader, send_first );
+	    
-
+	    /* revert the minimum value back to a positive number */
-		/* revert the minimum value back to a positive number */
+	    gstart[1] = (-1) * gstart[1];
-		gstart[1] = (-1) * gstart[1];
+	    
-		
+	    if  ( gstart[0] == start[0] && 
-		if  ( gstart[0] == start[0] && 
+		  gstart[1] == start[0] && 
-		      gstart[1] == start[0] && 
+		  gstart[0] != cid_block_start ) {
-		      gstart[0] != cid_block_start ) {
+		comm->c_id_available   = gstart[0];
-		    comm->c_id_available   = gstart[0];
+		comm->c_id_start_index = gstart[0];
-		    comm->c_id_start_index = gstart[0];
+		nextcid                = comm->c_id_available;
-
+		/* note: cid_block_start not modified in this section */
-		    /* note: cid_block_start not modified in this section */
+	    }
-		}
+	    else {
-		else {
+		/* no, one process did not agree on the reuse of the block
-		    /* no, one process did not agree on the reuse of the block
+		   so we have to go with the higher number */
-		       so we have to go with the higher number */
+		comm->c_id_available   = gstart[2];
-		    comm->c_id_available   = gstart[2];
+		comm->c_id_start_index = gstart[2];
-		    comm->c_id_start_index = gstart[2];
+		cid_block_start        = gstart[2] + block;
-		    cid_block_start = gstart[2] + block;
+		nextcid                = comm->c_id_available;
-		}
+	    }
-            }
+	}
-        }
+	
-        
+	comm->c_id_available++;
-        comm->c_id_available++;
     }
     /* set the according values to the newcomm */
     newcomm->c_contextid = nextcid;
@@ -432,7 +427,7 @@ static uint32_t ompi_comm_lowest_cid (void)
  * comm.c is, that this file contains the allreduce implementations
  * which are required, and thus we avoid having duplicate code...
  */
-int ompi_comm_activate ( ompi_communicator_t** newcomm )
+int ompi_comm_activate ( ompi_communicator_t** newcomm, int do_coll_select )
 {
     int ret = 0;
 
@@ -471,8 +466,10 @@ int ompi_comm_activate ( ompi_communicator_t** newcomm )
 
     /* Let the collectives components fight over who will do
        collective on this new comm.  */
-    if (OMPI_SUCCESS != (ret = mca_coll_base_comm_select(*newcomm))) {
+    if ( do_coll_select ) {
-        goto bail_on_error;
+	if (OMPI_SUCCESS != (ret = mca_coll_base_comm_select(*newcomm))) {
+	    goto bail_on_error;
+	}
     }
     return OMPI_SUCCESS;
 
@@ -496,7 +493,6 @@ static int ompi_comm_cid_checkforreuse ( int c_id_start_index, int block )
     int i, count=0;
     ompi_communicator_t * tempcomm;
 
-
     if ( MPI_UNDEFINED != c_id_start_index ) {
 	for ( i= c_id_start_index; i < c_id_start_index + block; i++ ) {
 	    tempcomm = (ompi_communicator_t *) opal_pointer_array_get_item ( &ompi_mpi_communicators, i );
@@ -512,6 +508,51 @@ static int ompi_comm_cid_checkforreuse ( int c_id_start_index, int block )
 
     return ret;
 }
+
+/* this function is called from the communicator destructor. It is a
+   generic interface which verifies whether the cid block assigned to 
+   that communicator can be reused. 
+   In the current implementation it checks whether all communicators 
+   have been freed. If that's the case *and* the current leading fron
+   of cid's (i.e. cid_block-start) is right after the block following
+   the block assigned to that communicator, we reset the leading block.
+*/
+void  ompi_comm_checkfor_blockreset ( ompi_communicator_t *comm )
+{
+    int block=ompi_comm_get_blocksize ( comm );
+    int next=0;
+
+    if (MPI_THREAD_MULTIPLE == ompi_mpi_thread_provided) {
+	return;
+    }
+
+    if ( MPI_UNDEFINED == comm->c_id_start_index ) {
+	return;
+    }
+
+    next = ompi_comm_cid_checkforreuse ( comm->c_id_start_index, block );
+    if ( next            == comm->c_id_start_index && 
+	 cid_block_start == (comm->c_id_start_index + block ) ) {
+ 	cid_block_start = comm->c_id_start_index;
+    }
+
+    return;
+}
+
+static int ompi_comm_get_blocksize ( ompi_communicator_t* comm  ) 
+{
+    int block=0;
+
+    if( 0 == comm->c_contextid ) {
+	block = OMPI_COMM_BLOCK_WORLD;
+    }
+    else {
+	block = OMPI_COMM_BLOCK_OTHERS;
+    }
+
+    return block;
+}
+
 /**************************************************************************/
 /**************************************************************************/
 /**************************************************************************/

ompi/communicator/comm_init.c

@@ -10,7 +10,7 @@
  *                         University of Stuttgart.  All rights reserved.
  * Copyright (c) 2004-2005 The Regents of the University of California.
  *                         All rights reserved.
- * Copyright (c) 2006-2007 University of Houston. All rights reserved.
+ * Copyright (c) 2006-2009 University of Houston. All rights reserved.
  * Copyright (c) 2007      Cisco Systems, Inc. All rights reserved.
  * Copyright (c) 2009      Sun Microsystems, Inc. All rights reserved.
  * $COPYRIGHT$
@@ -420,6 +420,10 @@ static void ompi_comm_destruct(ompi_communicator_t* comm)
                                              comm->c_f_to_c_index )) {
         opal_pointer_array_set_item ( &ompi_mpi_communicators,
                                       comm->c_f_to_c_index, NULL);
+
+	if ( MPI_UNDEFINED != comm->c_id_start_index ) {
+	    ompi_comm_checkfor_blockreset ( comm );
+	}
     }
 
 

ompi/communicator/communicator.h

@@ -11,7 +11,7 @@
  * Copyright (c) 2004-2005 The Regents of the University of California.
  *                         All rights reserved.
  * Copyright (c) 2006      Cisco Systems, Inc.  All rights reserved.
- * Copyright (c) 2006-2008 University of Houston.  All rights reserved.
+ * Copyright (c) 2006-2009 University of Houston.  All rights reserved.
  * Copyright (c) 2009      Sun Microsystems, Inc. All rights reserved.
  * $COPYRIGHT$
  * 
@@ -482,7 +482,7 @@ int ompi_comm_determine_first ( ompi_communicator_t *intercomm,
                                 int high );
 
 
-OMPI_DECLSPEC int ompi_comm_activate ( ompi_communicator_t** newcomm );
+OMPI_DECLSPEC int ompi_comm_activate ( ompi_communicator_t** newcomm, int do_coll_select );
 
 /**
  * a simple function to dump the structure
@@ -504,6 +504,12 @@ void ompi_comm_reg_finalize(void);
 extern int ompi_comm_num_dyncomm;
 
 
+/* check in the communicator destructor whether a block if cid's 
+   can be reused.
+*/
+OMPI_DECLSPEC void  ompi_comm_checkfor_blockreset ( ompi_communicator_t *comm );
+
+
 END_C_DECLS
 
 #endif /* OMPI_COMMUNICATOR_H */

ompi/mca/dpm/orte/dpm_orte.c

@@ -10,7 +10,7 @@
  * Copyright (c) 2004-2005 The Regents of the University of California.
  *                         All rights reserved.
  * Copyright (c) 2007      Cisco Systems, Inc.  All rights reserved.
- * Copyright (c) 2006-2008 University of Houston.  All rights reserved.
+ * Copyright (c) 2006-2009 University of Houston.  All rights reserved.
  * Copyright (c) 2009      Sun Microsystems, Inc.  All rights reserved.
  * $COPYRIGHT$
  *
@@ -441,7 +441,7 @@ static int connect_accept ( ompi_communicator_t *comm, int root,
     }
 
     /* activate comm and init coll-component */
-    rc = ompi_comm_activate ( &newcomp );             /* new communicator */
+    rc = ompi_comm_activate ( &newcomp, 1 );             /* new communicator */
     if ( OMPI_SUCCESS != rc ) {
         goto exit;
     }

ompi/mpi/c/intercomm_create.c

@@ -10,7 +10,7 @@
  * Copyright (c) 2004-2005 The Regents of the University of California.
  *                         All rights reserved.
  * Copyright (c) 2006-2007 Cisco Systems, Inc.  All rights reserved.
- * Copyright (c) 2006-2008 University of Houston.  All rights reserved.
+ * Copyright (c) 2006-2009 University of Houston.  All rights reserved.
  * $COPYRIGHT$
  * 
  * Additional copyrights may follow
@@ -212,7 +212,7 @@ int MPI_Intercomm_create(MPI_Comm local_comm, int local_leader,
     }
 
     /* activate comm and init coll-module */
-    rc = ompi_comm_activate ( &newcomp );
+    rc = ompi_comm_activate ( &newcomp, 1 );
     if ( MPI_SUCCESS != rc ) {
         goto err_exit;
     }

ompi/mpi/c/intercomm_merge.c

@@ -10,7 +10,7 @@
  * Copyright (c) 2004-2005 The Regents of the University of California.
  *                         All rights reserved.
  * Copyright (c) 2006-2009 Cisco Systems, Inc.  All rights reserved.
- * Copyright (c) 2006-2008 University of Houston.  All rights reserved.
+ * Copyright (c) 2006-2009 University of Houston.  All rights reserved.
  * $COPYRIGHT$
  * 
  * Additional copyrights may follow
@@ -132,7 +132,7 @@ int MPI_Intercomm_merge(MPI_Comm intercomm, int high,
     }
 
     /* activate communicator and init coll-module */
-    rc = ompi_comm_activate( &newcomp );           /* new comm */ 
+    rc = ompi_comm_activate( &newcomp, 1 );           /* new comm */ 
     if ( OMPI_SUCCESS != rc ) {
         goto exit;
     }