RFR 8215047: Task terminators do not complete termination in consistent state

[yumin qi]

Zhengyu,

  It looks good to me.

Thanks
Yumin

On Sat, Jan 26, 2019 at 12:59 AM <zgu at redhat.com> wrote:

> Hi,
>
> > > > I will think a bit more about the change while waiting for the
> > > > test
> > > > results.
> > > >
> >
> > Testing showed good results, no issues.
>
> Thanks!
>
> >
> > I would like to ask if I understood the changes correctly: to me it
> > seems that some of these changes are the actual fix, others seem to
> > be
> > enhancements.
> >
> > So the changes in owstTaskTerminator.cpp:68-74 seem to be the actual
> > fix for the issue - it needs to recheck _offered_termination within
> > the
> > lock at that point because of the mentioned race.
> I would say that this is actually an enhancement. Without it, merely
> delays the termination of this particular thread.
>
> Filed RFE: https://bugs.openjdk.java.net/browse/JDK-8217794
>
> >
> > There are additional improvements to both terminators
> > (owstTaskTerminator.cpp:171-174, taskqueue.cpp:220-232) that may
> > result
> > in potential needless additional rounds of offer_termination().
> > I.e. before returning a result that indicates to the caller to
> > continue
> > doing work for another round, instead if in the meantime everyone has
> > offered termination, we are obviously done too.
>
> >
> > If I am correct, I would strongly prefer if these changes (the
> > enhancements) would be changed separately.
> >
> > Also, the code in taskqueue.cpp:220-232 is worth an extra method with
> > a
> > description indicating why we can return true in offer_termination in
> > that case (because obviously, if all threads are sure to have offered
> > termination, between the evaluation of the condition and trying to
> > decrement the _offer_termination variable, work has been completed.
>
> Okay. But I want to point out that it is not necessary a race, because
> TerminatorTerminator may decide to request an abort based on external
> conditions, which may have nothing to do with the progress of the
> termination.
>
> Updated Webrev: http://cr.openjdk.java.net/~zgu/JDK-8215047/webrev.02/i
> ndex.html
> <http://cr.openjdk.java.net/~zgu/JDK-8215047/webrev.02/index.html>
>
> Passed hotspot_gc test, will rerun all other tests.
>
> > As an additional enhancement, it might be worth doing padding of the
> > ParallelTaskTerminator::_offered_termination variable.
> >
> I filed a separate RFE: https://bugs.openjdk.java.net/browse/JDK-821778
> 5
>
>
> -Zhengyu
>
>
> > Thanks,
> >   Thomas
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20190129/1870b399/attachment.htm>