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

Thu Nov 7 13:37:11 PST 2013

Hi,

On 11/07/2013 08:39 PM, Vladimir Kozlov wrote:
> On 11/7/13 11:04 AM, Igor Veresov wrote:
>> I’d vote to put it under PrintCodeCache. And make the messages not 
>> warnings, but just “compiler disabled/enabled”. What do you think?
>
> Unfortunately there could be customer's tools which look for this 
> message. So changing it, at least now for jdk8, is not good. With 
> small codecache we will expect this message showing up. But with big 
> codecache it should not happen. I think we should keep it as warning 
> but throttle it when small codecache is used as Chris suggested.
>
> May be put it under combined check:
>
> if (PrintCodeCache || ReservedCodeCacheSize > X)
>
> Do we have a state now when we definitely will not compile any more? 
> Or we always making progress? I think it will be difficult to find 
> when it should be printed only once.
>
With the current version (when sweeper is enabled) we should not reach a 
state (unless the entire code cache is filled with OSR-methods or native 
methods) where we disable compilation and never enable it.
As soon as we free memory from the code cache, we re-enable compilation. 
The message will be printed very frequently, if the code cache is 
significantly smaller than the application demands.

We could solve the 'problem' also by adding code that prints the warning 
only if compilation is
disabled for a certain time. The current patch (webrev.01) defines a 
virtual time for the sweeper (we increment time counter by one every 
time we call mark_active_nmethods), which we could use.

Best,
Albert
> Thanks,
> Vladimir
>
>>
>> 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
>>>>>
>>>>
>>>
>>