RFR: 8140257: Add support for "gc service threads" to ConcurrentGCThread

[Derek White]

RFR 2nd version.

New version is focused on making ConcurrentMarkSweepThread a proper 
subclass of ConcurrentGCThread, especially related to sharing the same 
initialization and termination protocols. See incremental webrev 
<http://cr.openjdk.java.net/%7Edrwhite/8140257/webrev.01.v.02/> for details.

  * Move CMS-specific code to run_service()/stop_service(), inherit
    run()/stop() methods.
  * Removed ConcurrentMarkSweepThread::_should_terminate, use inherited
    _should_terminate instead.
  * Use the inherited _has_terminated flag instead of _cmst to denote
    termination. Users call cmst() instead of reading _cmst.
  * Change ConcurrentMarkSweepThread::start() and stop() to match G1's
    handling - assume stop() only called after start(), so
    ConcurrentGCThread objects have been created.
      o CMS and G1 start() methods called in same place:
        Universe::Initialize_heap(), and the stop() methods are called
        in same place: before_exit(), so they have the same lifetimes
        for their ConcurrentGCThreads.


*Bug*: https://bugs.openjdk.java.net/browse/JDK-8140257
*Webrev*: http://cr.openjdk.java.net/~drwhite/8140257/webrev.02/
*Incremental 1 vs 2*: 
http://cr.openjdk.java.net/~drwhite/8140257/webrev.01.v.02/

*Tests*:
- jprt
- Aurora Perf (including Startup3-Server-CMS, Footprint3-Server-CMS)
- Aurora Test "hs-nightly-gc-cms".

Thanks for looking!
  - Derek

On 2/26/16 11:51 PM, Derek White wrote:
> I'm working on a new webrev, so please hold off on reviewing.
>
> Some offline comments from Kim suggest trying another approach. Any 
> other approach :-) He rightly pointed out the poor match between the 
> new code and ConcurrentMarkSweepThread is pretty ugly.
>
> Two other options I'm looking at are either having 
> ConcurrentMarkSweepThread not subclass from ConcurrentGCThread, or 
> (more likely) refactor ConcurrentMarkSweepThread to use the common 
> termination protocol instead of doing it's own thing. The approach of 
> adding an intermediate class that handles the common code being 
> factored out was rejected in review comments for "8138920".
>
>  - Derek
>
> On 2/26/16 11:56 AM, Derek White wrote:
>> *Background*:
>> ConcurrentGCThread provides incomplete support for an initialization 
>> and termination protocol for GC threads, so missing parts are 
>> duplicated in almost all subclasses.
>>
>> *Fix*:
>> Move duplicated run(), and stop() methods up from subclasses 
>> ConcurrentG1RefineThread, ConcurrentMarkThread, G1StringDedupThread, 
>> and G1YoungRemSetSamplingThread to ConcurrentGCThread, as well as 
>> declare virtual methods run_service() and stop_service.
>>
>> Note that ConcurrentMarkSweepThread is the odd-ball subclass. It 
>> implements it's own termination protocol, it provides it's own run() 
>> and stop() methods, and does not use run_service() and stop_service().
>>
>> *Bug*: https://bugs.openjdk.java.net/browse/JDK-8140257
>> *Webrev*: http://cr.openjdk.java.net/~drwhite/8140257/webrev.01/
>>
>> *Tests*: jprt, Aurora "nightly" run (I think this is OK)
>> http://aurora.ru.oracle.com/faces/Batch.xhtml?batchName=1325690.VMSQE.adhoc.JPRT 
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20160303/c9f6e7df/attachment.htm>