[mpich2-commits] r7915 - in mpich2/trunk/src: include mpi/comm

[goodell at mcs.anl.gov]

Author: goodell
Date: 2011-02-04 16:55:46 -0600 (Fri, 04 Feb 2011)
New Revision: 7915

Modified:
   mpich2/trunk/src/include/mpiimpl.h
   mpich2/trunk/src/mpi/comm/commutil.c
Log:
performance: inline MPIR_Comm_release based on feedback from IBM

This change improves message rate by >5% for some benchmarks.

Reviewed by buntinas at .

Modified: mpich2/trunk/src/include/mpiimpl.h
===================================================================
--- mpich2/trunk/src/include/mpiimpl.h	2011-02-04 22:55:44 UTC (rev 7914)
+++ mpich2/trunk/src/include/mpiimpl.h	2011-02-04 22:55:46 UTC (rev 7915)
@@ -1177,19 +1177,52 @@
 } MPID_Comm;
 extern MPIU_Object_alloc_t MPID_Comm_mem;
 
-/* MPIR_Comm_release is a helper routine that releases references to a comm.
-   The second arg is false unless this is called as part of 
-   MPI_Comm_disconnect .
-*/
-int MPIR_Comm_release(MPID_Comm *, int );
-int MPIR_Comm_release_always(MPID_Comm *comm_ptr, int isDisconnect);
+/* this function should not be called by normal code! */
+int MPIR_Comm_delete_internal(MPID_Comm * comm_ptr, int isDisconnect);
 
 #define MPIR_Comm_add_ref(_comm) \
     do { MPIU_Object_add_ref((_comm)); } while (0)
-
 #define MPIR_Comm_release_ref( _comm, _inuse ) \
     do { MPIU_Object_release_ref( _comm, _inuse ); } while (0)
 
+
+/* Release a reference to a communicator.  If there are no pending
+   references, delete the communicator and recover all storage and
+   context ids.
+
+   This routine has been inlined because keeping it as a separate routine
+   results in a >5% performance hit for the SQMR benchmark.
+*/
+#undef FUNCNAME
+#define FUNCNAME MPIR_Comm_release
+#undef FCNAME
+#define FCNAME MPIU_QUOTE(FUNCNAME)
+static inline int MPIR_Comm_release(MPID_Comm * comm_ptr, int isDisconnect)
+{
+    int mpi_errno = MPI_SUCCESS;
+    int in_use;
+
+    MPIR_Comm_release_ref(comm_ptr, &in_use);
+    if (unlikely(!in_use)) {
+        /* the following routine should only be called by this function and its
+         * "_always" variant. */
+        mpi_errno = MPIR_Comm_delete_internal(comm_ptr, isDisconnect);
+        /* not ERR_POPing here to permit simpler inlining.  Our caller will
+         * still report the error from the comm_delete level. */
+    }
+
+ fn_exit:
+    return mpi_errno;
+}
+#undef FUNCNAME
+#undef FCNAME
+
+/* MPIR_Comm_release_always is the same as MPIR_Comm_release except it uses
+   MPIR_Comm_release_ref_always instead.
+*/
+int MPIR_Comm_release_always(MPID_Comm *comm_ptr, int isDisconnect);
+
+
 /* Preallocated comm objects.  There are 3: comm_world, comm_self, and 
    a private (non-user accessible) dup of comm world that is provided 
    if needed in MPI_Finalize.  Having a separate version of comm_world

Modified: mpich2/trunk/src/mpi/comm/commutil.c
===================================================================
--- mpich2/trunk/src/mpi/comm/commutil.c	2011-02-04 22:55:44 UTC (rev 7914)
+++ mpich2/trunk/src/mpi/comm/commutil.c	2011-02-04 22:55:46 UTC (rev 7915)
@@ -1225,18 +1225,21 @@
  * helper function frees the actual MPID_Comm structure and any associated
  * storage.  It also releases any refernces to other objects, such as the VCRT.
  * This function should only be called when the communicator's reference count
- * has dropped to 0. */
+ * has dropped to 0.
+ *
+ * !!! This routine should *never* be called outside of MPIR_Comm_release{,_always} !!!
+ */
 #undef FUNCNAME
-#define FUNCNAME comm_delete
+#define FUNCNAME MPIR_Comm_delete_internal
 #undef FCNAME
 #define FCNAME MPIU_QUOTE(FUNCNAME)
-static int comm_delete(MPID_Comm * comm_ptr, int isDisconnect)
+int MPIR_Comm_delete_internal(MPID_Comm * comm_ptr, int isDisconnect)
 {
     int in_use;
     int mpi_errno = MPI_SUCCESS;
-    MPID_MPI_STATE_DECL(MPID_STATE_COMM_DELETE);
+    MPID_MPI_STATE_DECL(MPID_STATE_COMM_DELETE_INTERNAL);
 
-    MPID_MPI_FUNC_ENTER(MPID_STATE_COMM_DELETE);
+    MPID_MPI_FUNC_ENTER(MPID_STATE_COMM_DELETE_INTERNAL);
 
     MPIU_Assert(MPIU_Object_get_ref(comm_ptr) == 0); /* sanity check */
 
@@ -1347,41 +1350,13 @@
     }
 
  fn_exit:
-    MPID_MPI_FUNC_EXIT(MPID_STATE_COMM_DELETE);
+    MPID_MPI_FUNC_EXIT(MPID_STATE_COMM_DELETE_INTERNAL);
     return mpi_errno;
  fn_fail:
     goto fn_exit;
 }
 
 /* Release a reference to a communicator.  If there are no pending
-   references, delete the communicator and recover all storage and 
-   context ids */
-#undef FUNCNAME 
-#define FUNCNAME MPIR_Comm_release
-#undef FCNAME
-#define FCNAME "MPIR_Comm_release"
-int MPIR_Comm_release(MPID_Comm * comm_ptr, int isDisconnect)
-{
-    int mpi_errno = MPI_SUCCESS;
-    int in_use;
-    MPID_MPI_STATE_DECL(MPID_STATE_MPIR_COMM_RELEASE);
-
-    MPID_MPI_FUNC_ENTER(MPID_STATE_MPIR_COMM_RELEASE);
-
-    MPIR_Comm_release_ref(comm_ptr, &in_use);
-    if (!in_use) {
-        mpi_errno = comm_delete(comm_ptr, isDisconnect);
-        if (mpi_errno) MPIU_ERR_POP(mpi_errno);
-    }
-
- fn_exit:
-    MPID_MPI_FUNC_EXIT(MPID_STATE_MPIR_COMM_RELEASE);
-    return mpi_errno;
- fn_fail:
-    goto fn_exit;
-}
-
-/* Release a reference to a communicator.  If there are no pending
    references, delete the communicator and recover all storage and
    context ids.  This version of the function always manipulates the reference
    counts, even for predefined objects. */
@@ -1401,7 +1376,7 @@
      * predefined communicators, such as MPI_COMM_WORLD or MPI_COMM_SELF. */
     MPIU_Object_release_ref_always(comm_ptr, &in_use);
     if (!in_use) {
-        mpi_errno = comm_delete(comm_ptr, isDisconnect);
+        mpi_errno = MPIR_Comm_delete_internal(comm_ptr, isDisconnect);
         if (mpi_errno) MPIU_ERR_POP(mpi_errno);
     }