[petsc-dev] PetscCommGetThreadComm() is currently collective

[Jed Brown]

On Sat, Oct 20, 2012 at 4:54 PM, Shri <abhyshr at mcs.anl.gov> wrote:

> On Oct 20, 2012, at 3:58 PM, Jed Brown wrote:
>
> > Worse, validity of the cache changes whether it has a barrier (in debug
> mode) or not.
>
> I don't understand the point you are making here.


There is a barrier in PetscCommDuplicate, but we cache the thread comm
because it involves several lookups and had a nontrivial performance
effect. Thus

Rank 0
PetscCommGetThreadComm(WORLD)
PetscCommGetThreadComm(SELF)  // tcomm cache miss, okay because comm is
sequential
PetscCommGetThreadComm(WORLD) // tcomm cache miss, triggers an
MPI_Barrier(WORLD)

Rank 1
PetscCommGetThreadComm(WORLD)
PetscCommGetThreadComm(WORLD) // tcomm cache HIT, no matching barrier


I'm going to commit the patch below unless there is an objection.

# HG changeset patch
# User Jed Brown <jed at 59A2.org>
# Date 1350767578 18000
# Node ID 376bb5351f20762cfd2309cbede0c349a2b34a6e
# Parent  538a44f2a56cf3a419067269fb5f2e2ce7c2a70d
Attach ThreadComm to outer comm instead of inner comm

diff --git a/src/sys/threadcomm/interface/threadcomm.c
b/src/sys/threadcomm/interface/threadcomm.c
--- a/src/sys/threadcomm/interface/threadcomm.c
+++ b/src/sys/threadcomm/interface/threadcomm.c
@@ -77,18 +77,15 @@
   PetscErrorCode ierr;
   PetscMPIInt    flg;
   void*          ptr;
-  MPI_Comm       icomm;

   PetscFunctionBegin;
   if (comm == comm_cached) {
     *tcommp = tcomm_cached;
     PetscFunctionReturn(0);
   }
-  ierr = PetscCommDuplicate(comm,&icomm,PETSC_NULL);CHKERRQ(ierr);
-  ierr =
MPI_Attr_get(icomm,Petsc_ThreadComm_keyval,&ptr,&flg);CHKERRQ(ierr);
+  ierr =
MPI_Attr_get(comm,Petsc_ThreadComm_keyval,&ptr,&flg);CHKERRQ(ierr);
   if (!flg) SETERRQ(PETSC_COMM_SELF,PETSC_ERR_ARG_CORRUPT,"MPI_Comm does
not have a thread communicator");
   *tcommp = (PetscThreadComm)ptr;
-  ierr = PetscCommDestroy(&icomm);CHKERRQ(ierr);
   comm_cached = comm;
   tcomm_cached = *tcommp;
   PetscFunctionReturn(0);
@@ -1154,20 +1151,14 @@
 PetscErrorCode PetscThreadCommDetach(MPI_Comm comm)
 {
   PetscErrorCode ierr;
-  MPI_Comm       icomm;
   PetscMPIInt    flg;
   void           *ptr;

   PetscFunctionBegin;
-  ierr = PetscCommDuplicate(comm,&icomm,PETSC_NULL);CHKERRQ(ierr);
-  ierr =
MPI_Attr_get(icomm,Petsc_ThreadComm_keyval,&ptr,&flg);CHKERRQ(ierr);
+  ierr =
MPI_Attr_get(comm,Petsc_ThreadComm_keyval,&ptr,&flg);CHKERRQ(ierr);
   if (flg) {
-    ierr = MPI_Attr_delete(icomm,Petsc_ThreadComm_keyval);CHKERRQ(ierr);
+    ierr = MPI_Attr_delete(comm,Petsc_ThreadComm_keyval);CHKERRQ(ierr);
   }
-  ierr = PetscCommDestroy(&icomm);CHKERRQ(ierr);
-  /* Release reference from PetscThreadCommAttach */
-  ierr = PetscCommDestroy(&comm);CHKERRQ(ierr);
-
   PetscFunctionReturn(0);
 }

@@ -1180,16 +1171,14 @@
 PetscErrorCode PetscThreadCommAttach(MPI_Comm comm,PetscThreadComm tcomm)
 {
   PetscErrorCode ierr;
-  MPI_Comm       icomm;
   PetscMPIInt    flg;
   void           *ptr;

   PetscFunctionBegin;
-  ierr = PetscCommDuplicate(comm,&icomm,PETSC_NULL);CHKERRQ(ierr); /* This
extra reference is released in PetscThreadCommDetach */
-  ierr =
MPI_Attr_get(icomm,Petsc_ThreadComm_keyval,&ptr,&flg);CHKERRQ(ierr);
+  ierr =
MPI_Attr_get(comm,Petsc_ThreadComm_keyval,&ptr,&flg);CHKERRQ(ierr);
   if (!flg) {
     tcomm->refct++;
-    ierr = MPI_Attr_put(icomm,Petsc_ThreadComm_keyval,tcomm);CHKERRQ(ierr);
+    ierr = MPI_Attr_put(comm,Petsc_ThreadComm_keyval,tcomm);CHKERRQ(ierr);
   }
   PetscFunctionReturn(0);
 }
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.mcs.anl.gov/pipermail/petsc-dev/attachments/20121020/cfa53e5e/attachment.html>