RFR(S) 8206467: Refactor G1ParallelCleaningTask into shared

[Zhengyu Gu]

Hi Thomas,

On 08/22/2018 08:51 AM, Thomas Schatzl wrote:
> Hi Zhengyu,
> 
> On Tue, 2018-08-21 at 16:16 -0400, Zhengyu Gu wrote:
>> Thanks for reviewing, Kim!
>>
>> I will fix styles/indents and file RFE accordingly.
>>
>> -Zhengyu
> 
>    the change introduces a deadlock:
> 
> The new ResolvedMethodCleaningTask, when calling unlink tries to grab
> the ResolvedMethodTable_lock, that may also be grabbed by the various
> compiler methods when e.g. resolving methods. Now when that happens,
> and a GC is in progress you already know what this leads to :)
> 
> We have a (closed) test that deadlocks 100% because of that.

I don't know I made functional change here.

I don't see JDK-8206423 pushed. But it should remove 
ResolvedMethodCleaningTask from parallel cleaning anyway, and move to 
ServiceThread, do I miss something here?

> 
> I try to follow up with an RFR asap that removes that task, if that
> takes too long I will revert the whole change.
> 
> And, please, in the future, when introducing code sharing changes,
> please split refactoring changes (moving code around) and functionality
> changes.

Okay.

Thanks,

-Zhengyu

> 
> That would have made it a lot easier to spot this issue during review.
> 
> Also we really want to sit down about how to measure time in our code:
> this change introduces *another* time base (us) using *another* base
> type (jint) without adding support for existing collectors (i.e. G1;
> *without* even filing an RFE to add this). This would also have been
> spotted easily if the reviews for old/new functionality were separated.
> 
> Thanks,
>    Thomas
>