RFR: 8138920: Refactor the sampling thread from ConcurrentG1RefineThread

[Thomas Schatzl]

Hi,

On Tue, 2015-10-20 at 15:56 -0400, Derek White wrote:
> 4th Webrev for review please: 
> 
> This version includes the cleanups suggested by Per.
> 
> RFE: JDK-8138920 Refactor the sampling thread from
> ConcurrentG1RefineThread 
> 
> Webrev: http://cr.openjdk.java.net/~drwhite/8138920/webrev.04/
> 
> jprt: waiting for results (which might take a long time given current
> jprt status).
> 
> Thanks for looking!

Looks good overall. Some minor comments:

- I am okay with deferring the move of the sample thread to another CR.
Is there a CR for that already?

- the comment for ConcurrentG1Refine::sampling_thread() might need some
elaboration.

- also add some description of the purpose of the new
G1YoungListSampleThread class. I am okay with the name, it could be more
specific like G1YoungRemSetSamplingThread. I will let you decide whether
to keep it.

- please rename
ConcurrentMarkThread::handle_adaptive_young_list_length() to something
more meaningful. The method delays the current thread so that the next
expected pauses observe the MMU. (I think it is really good to extract
this into a method!)
So something like observe_mmu() or delay_thread_to_keep_mmu() would be
imo more appropriate. These are just suggestions I came up within a few
seconds though...

- please also add some description of what these new methods do if not
really obvious. E.g. that handle_adaptive_young_list_length() method
does not qualify for obvious :) I saw that the change fixes existing
comments, but they are few and far between.

Thanks for handling this issue,
  Thomas