RFR: 8058563: InstanceKlass::_dependencies list isn't cleared from empty nmethodBucket entries

[Stefan Karlsson]

On 2015-10-16 08:00, Vladimir Ivanov wrote:
> Looks good.

Thanks, Vladimir.

StefanK

>
> Best regards,
> Vladimir Ivanov
>
> On 10/14/15 12:36 PM, Stefan Karlsson wrote:
>> Hi all,
>>
>> Please review this patch to reintroduce deletion of entries in the
>> InstanceKlass::_dependencies list.
>>
>> It would be good if this could be reviewed by both the GC team and the
>> Compiler team.
>>
>> http://cr.openjdk.java.net/~stefank/8058563/webrev.01/
>> https://bugs.openjdk.java.net/browse/JDK-8058563
>>
>>
>> Some background to this bug:
>>
>> Before JDK-8049421, it was guaranteed that only one thread at a time
>> could delete an nmethodBucket in the _dependencies list. JDK-8049421
>> parallelized the unloading of nmethods for G1 and the deletion of the
>> entries were deferred to a later GC phase, to save the cost of having to
>> synchronize the deletion of entries between the GC threads. The
>> deletions are instead done at a later phase when the GC threads claim
>> Klasses for cleaning and it's guaranteed that each Klass will only be
>> cleaned by one GC thread.
>>
>>
>> This patch will solve two problems with the current implementation of
>> the deferred deletion:
>>
>> 1) Today only G1 deletes the deferred entries and all other GCs leak the
>> entries. The patch adds calls to clean out entries from all GCs.
>>
>> 2) Entries used to be deleted immediately when flush_dependencies was
>> called from non-GC code, but today this code path also defers the
>> deletion. This is unnecessary, since the callers hold the CodeCache_lock
>> while flushing the dependencies, and the code is thereby only executed
>> by one thread at a time. The patch adds back the immediate deletion of
>> entries, when called from non-GC code.
>>
>> The code has changed a bit in JDK 9, but it might still be useful to
>> take a look at the patch that introduced the deferred deletion and
>> compare that to the suggested patch:
>> http://hg.openjdk.java.net/jdk8u/hs-dev/hotspot/diff/2c6ef90f030a/src/share/vm/oops/instanceKlass.cpp 
>>
>>
>>
>>
>> Tested with:
>>   JPRT, Kitchensink, parallel_class_unloading, Weblogic12medrec,
>> runThese, new unit test
>>
>> Thanks,
>> StefanK