[petsc-dev] PetscViewerASCIIPushSynchronized

[Jed Brown]

"Smith, Barry F." <bsmith at mcs.anl.gov> writes:

>> On May 2, 2019, at 6:15 PM, Jed Brown <jed at jedbrown.org> wrote:
>> 
>> I think this is a consequence of PetscViewerFlush_ASCII flushing any
>> synchronized messages (code is basically copied from
>> PetscSynchronizedFlush).
>> 
>>  if (vascii->allowsynchronized) {
>>    PetscMPIInt   tag,i,j,n = 0,dummy = 0;
>>    char          *message;
>>    MPI_Status    status;
>> 
>>    ierr = PetscCommDuplicate(comm,&comm,&tag);CHKERRQ(ierr);
>> 
>>    /* First processor waits for messages from all other processors */
>>    if (!rank) {
>> 
>> 
>> But if all ranks don't agree about whether synchronized messages may
>> exist, then this slow code path would always be taken.  In the current
>> code, PetscViewerASCIIPopSynchronized doesn't do anything other than
>> tracking that the pushes were matched.  It should probably flush
>
>    What about the push? Does it need to do a flush also?  Note that the queued up messages are not tagged by the synchronized count so with 
> multiple pushes there can be jumbled mess. 

I would think printing in the order of the pops would be okay.  It means
each push would use a fresh buffer so only the content printed inside
that level of nesting would print.  SegBuffer would be convenient for
this buffer management.

>> (or
>> provide equivalent semantic on ordering).
>
> Maybe the whole chunk of code
>
>  if (vascii->allowsynchronized) {
>    PetscMPIInt   tag,i,j,n = 0,dummy = 0;
>     ....
>
> could just be moved to the Pop. Or even better if possible have it call PetscSynchronizedFlush()  and
> avoid the duplicate code. I'm not sure why I duplicated the code  but there must have been a reason.

Yup, but it should be possible to deduplicate, even if via an internal helper.

> The current code is a bit defective. Maybe the push/pop model should be abandoned and instead require that Allow be closed before another one is open. So PetscViewerASCIIOpenSynchronized(), synchronized writes, PetscViewerASCIICloseSynchronized() and the close automatically flushes. I'm not sure if the current routines are used in a nested manner (nor how to search for that ;)).

Insert a check for allowsynchronized > 1 and run the test suite.

> After a few hours I'm becoming more convinced that the reason for the Allow/Push/Pop model is just to avoid the global synchronization.

I suspect that's right.