"Smith, Barry F." <[email protected]> writes:
>> On May 2, 2019, at 6:15 PM, Jed Brown <[email protected]> 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.
