RFR(S): 8027593: performance drop with constrained codecache starting with hs25 b111

[Igor Veresov]

I’d vote to put it under PrintCodeCache. And make the messages not warnings, but just “compiler disabled/enabled”. What do you think?

igor

On Nov 7, 2013, at 3:24 AM, Albert Noll <albert.noll at oracle.com> wrote:

> Hi Chris,
> 
> On 11/06/2013 03:18 AM, Chris Plummer wrote:
>> BTW, one thing I forgot to mention is I now see a lot of messages for the codecache filling up. For example:
>> 
>> Java HotSpot(TM) Client VM warning: CodeCache is full. Compiler has been disabled.
>> Java HotSpot(TM) Client VM warning: Try increasing the code cache size using -XX:ReservedCodeCacheSize=
>> CodeCache: size=2700Kb used=2196Kb max_used=2196Kb free=503Kb
>> 
>> With b111, I was only seeing one message. I suspect with b111, once this message appeared compilation was never re-enabled so the message never appeared again. In that case seeing in many times now is actually a good indicator. However, it appears even when not using -XX:+PrintCodeCache, and I can see this output being a distraction for programs whose normal operation may involve constraining the codecache and having it constantly filling up. Perhaps this message should be off by default, or possibly only appear once.
>> 
> You are right. The previous version just never re-enabled compilation. I also agree that the
> output is distracting. There are multiple ways to solve this issue. I would go for a product -XX flag
> which allows to turn this warning on/off. Would that be ok or do you have a different solution in mind?
> 
> Best,
> Albert
> 
>> cheers,
>> 
>> Chris
>> 
>> On 11/5/13 5:59 PM, Chris Plummer wrote:
>>> Hi Albert,
>>> 
>>> I applied your patch and got some new numbers. Performance is now even better than it was with b110. See the chart I added to the bug.
>>> 
>>> Nice work!
>>> 
>>> Chris
>>> 
>>> On 11/5/13 6:44 AM, Albert Noll wrote:
>>>> Hi,
>>>> 
>>>> could I get reviews for this small patch?
>>>> 
>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8027593
>>>> webrev: http://cr.openjdk.java.net/~anoll/8027593/webrev.00/
>>>> 
>>>> Problem: The implementation of the sweeper (8020151) causes a performance regression for small code cache sizes. There are two issues that cause this regression:
>>>>  1) NmethodSweepFraction is only adjusted according to the ReservedCodecacheSize if TieredCompilation is enabled. As a result, NmethodSweepFraction remains 16 (if TieredCompilation is not used). This is way too large for small code cache sizes (e.g., <5m).
>>>> 2) _request_mark_phase (sweeper.cpp) is initialized to false. As a result, mark_active_nmethods() did not set _invocations and _current, which results in not invoking the sweeper (calling sweep_code_cache()) at all. When TieredCompilation is enabled this was not an issue, since NmethodSweeper::notify() (which sets _request_mark_phase) is called much more frequently.
>>>> 
>>>> Solution: 1) Move setting of NmethodSweepFraction so that it is always executed.
>>>> Solution: 2) Remove need_marking_phase(), request_nmethod_marking(), and reset_nmetod_marking().
>>>>                   I think that these checks are not needed since 8020151, since we do stack scanning of
>>>>                   active nmethods irrespective of the value of what need_marking_phase() returns. Since
>>>>                   the patch removes need_marking_phase() printing out the warning (line 327 in
>>>>                   sweeper.cpp) is incorrect, i.e., we continue to invoke the sweeper. I removed the warning
>>>>                   and the associated code.
>>>> 
>>>> 
>>>> Also, I think that we can either remove -XX:MethodFlushing or -XX:UseCodeCacheFlushing. Since 8020151, one of them is redundant and can be removed. I am not quite sure if we should do that now so it is not included in the patch.
>>>> 
>>>> Testing
>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8027593 also shows a performance evaluation.
>>>> 
>>>> Many thanks for looking at the patch.
>>>> Best,
>>>> Albert
>>> 
>> 
>