RFR(S) 8206467: Refactor G1ParallelCleaningTask into shared

[Zhengyu Gu]

Thanks for reviewing, Kim!

I will fix styles/indents and file RFE accordingly.

-Zhengyu

On 08/21/2018 04:01 PM, Kim Barrett wrote:
>> On Jul 6, 2018, at 12:56 PM, Zhengyu Gu <zgu at redhat.com> wrote:
>>
>> Hi,
>>
>> Shenandoah has a similar version that was derived from G1ParallelCleaningTask, with additional time tracking of each cleaning task.
>>
>> Due to code movement and renaming, the changeset appears to be large, but it is really not. Other than the renaming, followings are the actual diffs vs. current G1ParallelCleaningTask:
>>
>> - G1StringDedupUnlinkOrOopsDoClosure is passed as a parameter.
>>
>> - Counters (_string_processed, _string_removed, and etc.) in StringAndSymbolCleaningTask are volatile now, cause they are updated using atomic operations.
>>
>> - Added ParallelCleaningTimes and ParallelCleaningTaskTimer classes for tracking times.
>>
>> - ParallelCleaningTask::work() added time tracking code.
>>
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8206467
>> Webrev: http://cr.openjdk.java.net/~zgu/8206467/webrev.00/
>>
>>
>> Test:
>>   hotspot_gc on Linux 64 (fastdebug and release)
>>
>>
>> Thanks,
>>
>> -Zhengyu
> 
> A few minor issues (mostly formatting), but otherwise looks good.  I
> don't need a new webrev.
> 
> ------------------------------------------------------------------------------
> src/hotspot/share/gc/shared/parallelCleaning.hpp
>    51   StringAndSymbolCleaningTask(BoolObjectClosure* is_alive, StringDedupUnlinkOrOopsDoClosure* dedup_closure,
>    52                                 bool process_strings, bool process_symbols, bool process_string_dedup);
> 
> Fix indentation of parameter list.
> 
> ------------------------------------------------------------------------------
> src/hotspot/share/gc/shared/parallelCleaning.hpp
>    57   size_t strings_processed() const { return (size_t)_strings_processed; }
>    58   size_t strings_removed()   const { return (size_t)_strings_removed; }
>    59
>    60   size_t symbols_processed() const { return (size_t)_symbols_processed; }
>    61   size_t symbols_removed()   const { return (size_t)_symbols_removed; }
> 
> Pre-existing: The counter members should be of type size_t, and remove
> the casts here.  Oh, but that would run into problems because the
> possibly_parallel_unlink functions take int*.  Bleh.  Don't try to fix
> any of this as part of this change; please file an RFE.
> 
> ------------------------------------------------------------------------------
> src/hotspot/share/gc/shared/parallelCleaning.cpp
>    34 StringAndSymbolCleaningTask::StringAndSymbolCleaningTask(BoolObjectClosure* is_alive,
> 
> While I appreciate the newly introduced line breaks in the parameter
> list, the chosen indentation of the additional lines is atypical for
> HotSpot and makes the transition between parameters and initializer
> list hard to spot.
> 
> ------------------------------------------------------------------------------
> src/hotspot/share/gc/shared/parallelCleaning.cpp
>    45   _initial_string_table_size = (int) StringTable::the_table()->table_size();
>    46   _initial_symbol_table_size = SymbolTable::the_table()->table_size();
> 
> Pre-existing: These initializations ought to be in the initializer
> list.
> 
> ------------------------------------------------------------------------------
> src/hotspot/share/gc/shared/parallelCleaning.cpp
>   120 void CodeCacheUnloadingTask::add_to_postponed_list(CompiledMethod* nm) {
> ...
>   125     } while (Atomic::cmpxchg(nm, &_postponed_list, old) != old);
> 
> Mostly got re-indented to match normal HotSpot style, but this one
> line didn't for some reason.
> 
> ------------------------------------------------------------------------------
>