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

[Albert Noll]

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
>>
>