RFR (XL): 8218668: Clean up evacuation of optional collection set

[Thomas Schatzl]

Hi Sangheon,

  thanks for your review.

On Thu, 2019-04-04 at 14:41 -0700, sangheon.kim at oracle.com wrote:
> Hi Thomas,
> 
> On 4/4/19 4:18 AM, Thomas Schatzl wrote:
> > Hi Kim,
> > 
> > On Tue, 2019-04-02 at 21:02 -0400, Kim Barrett wrote:
> > > > On Mar 13, 2019, at 9:12 AM, Thomas Schatzl <
> > > > thomas.schatzl at oracle.com> wrote:
> > > > 
[...]
> > New webrevs:
> > http://cr.openjdk.java.net/~tschatzl/8218668/webrev.1_to_2/
> > http://cr.openjdk.java.net/~tschatzl/8218668/webrev.2/
> 
> Nice clean up!
> 
> The updated comment at G1CollectionSet is much better to me.
> 
> I have minor nits:
> --------------------------------------------------
> share/gc/g1/g1CollectedHeap.cpp
>   157 Tickspan G1CollectedHeap::run_task(AbstractGangTask* task,
> uint num_workers) {
> - Couldn't always use workers()->run_task(task) instead of
> num_workers treatment? The difference between
> WorkGang::run_task(AbstractGangTask* task) and
> G1ColectedHeap::run_task() looks like WorkGang doesn't
> allow active workers to be 0. I don't think we have such case.

I removed the optional parameter.

> - Just an idea and not expect to fix here. It would nice if this api 
> exists on WorkGang something like run_task() and run_task_ms().

Yeah, that might be interesting to consider as well as other automatic
time-taking improvements for WorkGang.

> --------------------------------------------------
> share/gc/g1/g1CollectionSet.hpp
>    52 // This set is built incrementally at mutator time as regions
> are retired, and,
> - two commas?
> 
>   174   size_t _inc_part_start;
> - Not initialized at ctor.
> 
> --------------------------------------------------
> share/gc/g1/g1RemSet.cpp
>   342 void
> G1ScanRSForRegionClosure::scan_opt_rem_set_roots(HeapRegion* r){
> - Space between '){'
> 
> --------------------------------------------------
> share/gc/g1/g1OopStarChunkedList.cpp
> - copyright year update
> 
> --------------------------------------------------
> share/gc/g1/g1OopStarChunkedList.hpp
> - copyright year update

All fixed. Thanks,

New webrevs:
http://cr.openjdk.java.net/~tschatzl/8218668/webrev.2_to_3/
http://cr.openjdk.java.net/~tschatzl/8218668/webrev.3/ 


Thanks,
  Thomas