RFR(M): 8039147: Cleanup SuspendibleThreadSet

[Per Liden]

Hi,

Thanks Mikael, good catch! I found another comment in G1CollectedHeap 
which I also fixed. The updated webrev also includes the int->uint fix 
from Thomas comments:

http://cr.openjdk.java.net/~pliden/8039147/webrev.1/

Diff against previous webrev:

http://cr.openjdk.java.net/~pliden/8039147/webrev.1-diff_0vs1/

Thanks!
/Per

On 04/09/2014 02:23 PM, Mikael Gerdin wrote:
> Per,
>
> On Tuesday 08 April 2014 13.32.23 Per Liden wrote:
>> Hi,
>>
>> Here's a attempt to cleanup SuspendibleThreadSet that I'd like to get
>> some reviews on.
>>
>> Webrev: http://cr.openjdk.java.net/~pliden/8039147/webrev.0/
>
> The comment at the top of suspendibleThreadSet.hpp mentions
> safepoint_synchronize() and safepoint_desyncrhonize() but you removed the
> prefix from these functions.
>
> Otherwise the change looks good.
>
> /Mikael
>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8039147
>>
>> Summary:
>>
>> * The function of SuspendibleThreadSet is completely unchanged. Still
>> does the same thing, same semantics, same users, etc.
>>
>> * SuspendibleThreadSet is broken out from concurrentGCThread.hpp/cpp
>> into separate files. The relation ship with ConcurrentGCThread is a bit
>> artificial as SuspendibleThreadSet is used by other threads as well e.g.
>> SharedHeap's FlexibleWorkGang.
>>
>> * There's currently two ways of using SuspendibleThreadSet, depending on
>> which context you're in; 1) through direct use of the static _sts member
>> in ConcurrentGCThread and 2) via the static stsXXXX() functions exposed
>> by ConcurrentGCThread. With this patch there's only one interface, which
>> is exposed as static functions on the SuspendibleThreadSet class itself.
>>
>> * I adjusted the names of some of the fields in SuspendibleThreadSet,
>> which didn't quite make sense to me. For example, _async is now
>> _nthreads since it's a counter of number of threads currently in the
>> thread set.
>>
>> * The should_yield() and yield() functions on ConcurrentGCThread (and
>> it's subclasses) where removed, since they were never used. There was
>> one exception here though, which was concurrentMarkThread::yield(), but
>> it just does a direct call to SuspendibleThreadSet's yield() so that's
>> just an useless indirection.
>>
>> * SuspendibleThreadSetJoiner is introduced to replace code like this:
>>
>> if (....) {
>>      _sts.join();
>>      <do something>
>>      _sts.leave();
>> }
>>
>> with code like this:
>>
>> if (....) {
>>      SuspendibleThreadSetJoiner sts;
>>      <do something>
>> }
>>
>> cheers,
>> /Per
>