RFR: Refactoring ShControlThread::run_service() method

[Zhengyu Gu]

On 10/25/2018 11:25 AM, Aleksey Shipilev wrote:
> On 10/25/2018 04:55 PM, Zhengyu Gu wrote:
>> Breaking up lengthy ShControlThread::run_service() method, moving setup, prepare and execute GC into
>> separate methods.
>>
>> The patch should not have functional differences.
>>
>> Webrev: http://cr.openjdk.java.net/~zgu/shenandoah/cleanup_ctrl_thr/webrev.00/index.html
> 
> Massaged patch, apply on top:
>    http://cr.openjdk.java.net/~shade/shenandoah/sh-control-thread-1.patch
> 
> I am still not sure this is needed or sensible to do. The current code has particular symmetries
> that are broken if you hide parts of the bulk method in submethods. For example,
> set_should_clear_all_soft_refs handling should be on the same level, but it is not now. It is not
> evident it would be unset on all paths. Another example: there is (pre|post)_gc_work, but some of
> that work is done _outside_ the method because it needs alloc_failure_pending/explicit_gc_requested
> flags.
Okay, as I said, it should not have functional differences. If I missed 
something or need further polish, I am all for it.

> 
> Anyhow, what I want to see in proper review is the *entire* thing sliced by changeset: first
> refactoring, then the fix. Note you can RFR multiple changesets at once in when doing this. It would
> then become evident that: a) refactoring was needed; b) refactoring simplified things for new
> feature; c) what are the new things in new feature; d) refactoring was complete, and no other
> refactoring touchups were needed to introduce the feature.

This is unpolished and half tested patch on top of my original patch, 
just give you some ideas.

http://cr.openjdk.java.net/~zgu/shenandoah/metadata_gc/webrev.01/

If you want to see the problem, here it one of test case:

make CONF=linux-x86_64-server-fastdebug run-test 
TEST=vmTestbase/metaspace/stressHierarchy/stressHierarchy012/TestDescription.java 
JTREG="VM_OPTIONS=-XX:UnlockExperimentalVMOptions -XX:+UseShenandoahGC 
-Xlog:gc"

-Zhengyu


> 
> -Aleksey
>