RFR(S) 8206467: Refactor G1ParallelCleaningTask into shared

[Kim Barrett]

> 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.

------------------------------------------------------------------------------