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

[Thomas Schatzl]

Hi,

On Wed, 2019-01-23 at 09:32 -0500, zgu at redhat.com wrote:
> Hi Thomas,
> 
> Thanks for getting the review started.
> 
> > initial comments:
> > 
> > - taskqueue.hpp: I recommend adding an empty virtual destructor in
> > any
> > case.
> > 
> > - owstTaskTerminator.cpp:69: s/returnning/returning/
> > 
> 
> Updated based on your comments:
> 
> http://cr.openjdk.java.net/~zgu/JDK-8215047/webrev.01/index.html
> 
> Thanks,
> 
> -Zhengyu
> 
> > I will think a bit more about the change while waiting for the test
> > results.
> > 

Testing showed good results, no issues.

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.

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.

As an additional enhancement, it might be worth doing padding of the
ParallelTaskTerminator::_offered_termination variable.

Thanks,
  Thomas