10 RFR: 8169517: WhiteBox should provide concurrent GC phase control
kim.barrett at oracle.com
Tue Mar 7 06:42:02 UTC 2017
> 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:
> - 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.
More information about the hotspot-gc-dev