RFR: 8058563: InstanceKlass::_dependencies list isn't cleared from empty nmethodBucket entries
mikael.gerdin at oracle.com
Wed Oct 14 12:29:34 UTC 2015
On 2015-10-14 11:36, 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.
in instanceKlass.cpp, nmethodBucket::remove_dependent_nmethod
would it make sense to assert that delete_immediately is true iff the
current thread is the owner of the CodeCache_lock?
in instanceKlass.hpp the bool parameter to remove_dependent* is named
in the .cpp file it's "bool delete_immediately"
I'm not particularly fond of the "convenience method":
1946 bool nmethodBucket::remove_dependent_nmethod(nmethodBucket* deps,
Its only caller, MethodHandles::remove_dependent_nmethod, looks like it
actually should do
since it immediately calls clean_dependent_nmethods and updates the list
head if remove_dependent returned true.
I also rediscovered a known issue, that remove_dependent_nmethod can
actually be called during parallel unloading and thereby cause crashes
but fixing that is another step.
Otherwise I think the change is ok, looking forward to further cleanups
> 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:
> Tested with:
> JPRT, Kitchensink, parallel_class_unloading, Weblogic12medrec,
> runThese, new unit test
More information about the hotspot-dev