RFR(M): 8039147: Cleanup SuspendibleThreadSet

[Per Liden]

Thanks Bengt, Thomas and Mikael!

/Per

On 04/11/2014 09:59 AM, Mikael Gerdin wrote:
> Per,
>
> On Wednesday 09 April 2014 15.36.49 Per Liden wrote:
>> 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/
>
> Looks good.
> /Mikael
>
>>
>> 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
>