RFR(M): 8039147: Cleanup SuspendibleThreadSet

[Bengt Rutisson]

On 2014-04-09 15:36, 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/
>
> Diff against previous webrev:
>
> http://cr.openjdk.java.net/~pliden/8039147/webrev.1-diff_0vs1/

Still looks good to me.

Bengt

>
> 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
>>
>