On Oct 20, 2012, at 5:02 PM, Jed Brown wrote: > 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.
Seems fine to me. > > # 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/419edbf4/attachment-0001.html>