RFR: 8028993: Full collections with ParallelScavenge slower in JDK 8 compared to 7u40

[Stefan Karlsson]

Hi Johansson,

On 2013-12-09 10:33, Stefan Johansson wrote:
> Hi all,
>
> Updated one thing in the webrev after an offline comment. The new 
> webrev is at:
> http://cr.openjdk.java.net/~sjohanss/8028993/webrev.01/

I think this looks good.

A comment about this comment.

http://cr.openjdk.java.net/~sjohanss/8028993/webrev.01/src/share/vm/oops/instanceMirrorKlass.cpp.udiff.html

+    // For anonymous classes we need to handle the class loader data,
+    // otherwise it won't be claimed and can be unloaded.
+    if (klass->oop_is_instance() && InstanceKlass::cast(klass)->is_anonymous()) {
+      PSParallelCompact::follow_class_loader(cm, klass->class_loader_data());
+    } else {
      PSParallelCompact::follow_klass(cm, klass);
+    }


IMHO, the comment doesn't help with the understanding of this code. All 
klasses, not only anonymous klasses, "need to handle the class loader 
data, otherwise it won't be claimed and can be unloaded."

Could you explain that:
1) We can't call follow_klass, since that would just try to follow the 
mirror we're currently processing?
2) That for non-anonymous klasses we call follow_class_loader when the 
java.lang.ClassLoader of the klass is visited. But for anonymous 
klasses, which don't have their own java.lang.ClassLoader instance, we 
instead call follow_class_loader when the mirror is visited?

thanks,
StefanK

>
> The only change was to move the definition of follow_klass() in 
> psParallelCompact.hpp to be below the definition of mark_and_push(). 
> This might help inlining for some compilers, and it also maps to how 
> the declarations are ordered.
>
> Please review this fix,
> Stefan
>
>
> On 2013-12-04 17:48, Stefan Johansson wrote:
>> Hi,
>>
>> Can I have some reviews for this change to fix:
>> https://bugs.openjdk.java.net/browse/JDK-8028993
>>
>> Webrev:
>> http://cr.openjdk.java.net/~sjohanss/8028993/webrev.00/
>>
>> Summary:
>> While fixing JDK-8027675 we realized that we should have a similar 
>> regression with ParallelGC. The regression is not as big as with 
>> serial but the logic changes are the same and I've been able to 
>> measure a significant improvement with this fix, taking us on par 
>> with 7u40.
>>
>> The theory behind the fix is the same as when fixing serial:
>> ---
>> To improve the mark phase:
>> * Enabled the calls to follow_klass to be inlined.
>> * Reduce the extensive amount of calls to follow_class_loader in 
>> follow_klass and instead just mark the klass holder (the class loader 
>> or the java mirror) for the given klass.
>>
>> To improve the adjust phase:
>> * All calls to adjust_klass have been removed since this is already 
>> handled by processing the ClassLoaderDataGraph.
>> ---
>>
>> Testing:
>> * Building and basic testing through JPRT
>> * Some manual testing running Nashorn tests
>> * Performance testing with SPECjbb2005
>>
>> Thanks,
>> Stefan
>