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

[Stefan Johansson]

Hi Karlsson =)

On 2013-12-09 13:15, Stefan Karlsson wrote:
> 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:     // Anonymous classes doesn't have their 
> own class loader and the
>     // the call to follow_klass will mark and push its java mirror
>     // instead of the class loader. When handling the java mirror for
>     // an anonymous class we need to make sure its class loader data is
>     // claimed, this is done by calling follow_class_loader explicitly,
>     // for non-anonymous classes the call to follow_class_loader is made
>     // when the class loader itself is handled.
> 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?
>
We've spoken about this offline and this is the comment I plan to add, I 
will also change the comment in the SerialGC version of the code:
     // Anonymous classes doesn't have their own class loader and the
     // the call to follow_klass will mark and push its java mirror
     // instead of the class loader. When handling the java mirror for
     // an anonymous class we need to make sure its class loader data is
     // claimed, this is done by calling follow_class_loader explicitly,
     // for non-anonymous classes the call to follow_class_loader is made
     // when the class loader itself is handled.

Cheers,
StefanJ
> 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
>>
>