10 RFR: 8169517: WhiteBox should provide concurrent GC phase control

[Kim Barrett]

> On Mar 6, 2017, at 8:14 AM, Dmitry Fazunenko <dmitry.fazunenko at oracle.com> wrote:
> 
> Hi Kim,
> 
> First of all thank you for doing that change. Adding an access to GC phases is a great step toward GC testability improvement.

Thanks for looking at it.

> Would it make sense to provide method returned the current GC Phase.
>  
> public String getConcurrentGCPhase();

I don't think so, as the phase may change immediately after the call.

> 2) Would it make sense to extend sun.hotspot.gc.GC enum (test/lib/sun/hotspot/gc/GC.java)
> with 'supportsConcurrentGCPhaseControl()’ ?

I don't think so. With the exception of the "basics" test, I expect
any test using this feature to be GC-specific, as the set of phases
and their meanings are not cross-GC. We might, over time, adopt
conventional names for phases that exist in all concurrent GCs, but
I'm not certain truly common phases even exist.

> 3) I totally support Aleksey's position regarding ability to obtain all the available phases for a collector:

OK

> - it will allow to develop tests without looking at the code

I really doubt that; see earlier comment about common phases or lack thereof.

> 4) hotspot/test/gc/TestConcurrentPhaseControlBasics.java
> 
> - This class could be better located in hotspot/test/gc/whitebox next to other test for whitebox itself.

I overlooked the existence of that directory.  I'll move the test
there before pushing.  I'm leaving it in hotspot/test/gc until then,
to make incremental updates easier to review.

> - I think it will be correctly to have two tests instead of this one:
>    * the first test invokes VM with a GC which doesn't support phase control (like serial) and check WB methods
>    * the second test invokes VM with a GC which support this feature

Various GC-specific tests can be added to verify expected behavior, if
you think that's useful.

> 5) test/lib/jdk/test/lib/gc/TestConcurrentPhaseControlSupport.java and hotspot/test/gc/g1/TestConcurrentPhaseControlG1.java
> 
> I don't really like how the code is distributed between classes. Inheritance brings some limitations and reduce code readability.

I think I modelled this on some other code, but in retrospect I agree
the structure I used might not be ideal.  I'll work on this and be back
soon with an updated webrev.