RFR: 8056240: Investigate increased GC remark time after class unloading changes in CRM Fuse

[Mikael Gerdin]

Stefan,

On 2014-10-13 15:11, Stefan Karlsson wrote:
> Hi again,
>
> Latest changes after comments from Bertrand and Coleen:
> http://cr.openjdk.java.net/~stefank/8056240/webrev.01

chunkedList.hpp
902 +    // Don't clear the next pointers since that would interfer
Typo: interfer -> interfere

thread.hpp
Can't you forward-declare MetadataOnStackBuffer instead of #including it?
You only do simple manipulation on the pointers so it should work.

Ideally, it would be nice to not have the MetadataOnStackBuffer* in 
Thread but in one of its subclasses. Unfortunately it's used by both 
WorkerThread and VMThread so I don't have a good solution for that. I 
just find the pollution of the Thread superclass somewhat unfortunate.

classLoaderData.hpp
Why is clean_metaspaces public?

nmethod.cpp
381 +  // In this lmetadata, we must only follow those metadatas 
directly embedded in
Typo: lmetadata -> metadata

401 +    // Call function Method*, not embedded in these other places.
In the above comment you talk about "visiting metadata", can you change 
these comments to use consistent terminology?

Otherwise the change looks ok.

/Mikael


> http://cr.openjdk.java.net/~stefank/8056240/webrev.01.delta
>
> 1) Updated AccessFlags:set_on_stack to use a looping cmpxchg
> implementation.
> 2) Remove MetadataOnStackMark::_doit and restructure
> ClassLoaderDataGraph::do_unloading instead.
> 3) Move MetadataOnStackBuffer and make it a utility class named
> ChunkedList.
>
> thanks,
> StefanK
>
> On 2014-10-02 12:53, Stefan Karlsson wrote:
>> Hi all,
>>
>> (The following patch changes HotSpot code in areas concerning GC, RT,
>> and Compiler. So, it would be good to get reviews from all three teams.)
>>
>> Please, review this patch to optimize and parallelize the CodeCache
>> part of MetadaOnStackMark.
>>
>> G1 performance measurements showed longer than expected remark times
>> on an application using a lot of nmethods and Metadata. The cause for
>> this performance regression was the call to
>> CodeCache::alive_nmethods_do(nmethod::mark_on_stack); in
>> MetadataOnStackMark. This code path is only taken when class
>> redefinition is used. Class redefinition is typically used in
>> monitoring and diagnostic frameworks.
>>
>> With this patch we now:
>> 1) Filter out duplicate Metadata* entries instead of storing a
>> Metadata* per visited metadata reference.
>> 2) Collect the set of Metadata* entries in parallel. The code
>> piggy-backs on the parallel CodeCache cleaning task.
>>
>> http://cr.openjdk.java.net/~stefank/8056240/webrev.00/
>> https://bugs.openjdk.java.net/browse/JDK-8056240
>>
>> Functional testing:
>> JPRT, Kitchensink, parallel_class_unloading, unit tests
>>
>> Performance testing:
>> CRM Fuse - where the regression was found
>>
>> The patch changes HotSpot code in areas concerning GC, RT, Compiler,
>> and Serviceability. It would be good to get some reviews from the
>> other teams, and not only from the GC team.
>>
>> thanks,
>> StefanK
>