RFR: 8171238: Unify cleanup code used in G1 Remark and Full GC marking

Stefan Johansson stefan.johansson at oracle.com
Thu Feb 23 10:40:23 UTC 2017

Thanks for looking at this Thomas,

On 2017-02-21 12:11, Thomas Schatzl wrote:
> Hi,
> On Tue, 2017-02-21 at 11:17 +0100, Stefan Johansson wrote:
>> Hi,
>> Please review this re-factoring for:
>> https://bugs.openjdk.java.net/browse/JDK-8171238
>> Webrev:
>> http://cr.openjdk.java.net/~sjohanss/8171238/hotspot.00/
>> Summary:
>> Some small parts of the code in the G1 Mark Sweep Full GC is
>> parallel,
>> one of those is the string and symbol table cleaning as well as the
>> string deduplication cleanup. They are currently run as two separate
>> tasks which means the worker threads have to be started and stopped
>> two
>> times. The two tasks can be joined together and with the cleanup in
>> place the Remark phase of the concurrent cycle can also be unified
>> to
>> use the same code paths.
> The following comments are mostly about naming, since these are very
> much opinions, feel free to ignore what you do not like. The change
> itself looks good.
> - not really happy with "G1StringSymbolTableUnlinkTask" not showing
> that it also does dedup now.
I agree, as we talked about off-line G1StringAndSymbolCleaningTask is 
not optimal either but better at least.
> - I would kind of think "complete" as the matching word for "partial",
> not "full".
Agree, changed.
> - I would prefer if the comments for full/partial cleaning would
> explain what the method does, not (only) when/what it is used for?
> - maybe improve the "cleaning" to indicate what is cleaned (or what
> cleaning means), i.e. something like
> "full/partial_clean_dead_weak_references" or so - that is however a bit
> long for me too. Probably you can find a better name.
> Some better comments might remove the need for this.
Added comments, but left partial_cleaning/complete_cleaning as the names.

New webrev:
Full: http://cr.openjdk.java.net/~sjohanss/8171238/hotspot.01/
Inc: http://cr.openjdk.java.net/~sjohanss/8171238/hotspot.00-01/


> Thanks,
>    Thomas

More information about the hotspot-gc-dev mailing list