[petsc-dev] PetscCommGetThreadComm() is currently collective

[Shri]

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.html>