RFR(XXS): 8029091: Bug in calculation of code cache sweeping interval

[Vladimir Kozlov]

Sorry, it is Monday morning.

CodeCache::reverse_free_ratio() returns double. Then why calculation is not done in doubles in original code?
Can you just swap time_since_last_sweep and CodeCache::reverse_free_ratio() if it is order dependent?

Thanks,
Vladimir

On 11/25/13 9:47 AM, Vladimir Kozlov wrote:
> On other hand ReservedCodeCacheSize can't be big since there is 2Gb limit. So it is small int value.
> That in mind the fix is good.
>
> Thanks,
> Vladimir
>
> On 11/25/13 9:33 AM, Vladimir Kozlov wrote:
>> Albert,
>>
>> What if you use double type for calculations?:
>>
>> +     const double max_wait_time = (double)ReservedCodeCacheSize / (16 * M);
>>
>> Thanks,
>> Vladimir
>>
>> On 11/25/13 5:27 AM, Albert Noll wrote:
>>> Hi all,
>>>
>>> could I have reviews for this small patch?
>>>
>>> Bug: _
>>> _https://bugs.openjdk.java.net/browse/JDK-8029091
>>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~anoll/8029091/webrev.00/
>>>
>>> Problem:
>>> The calculation of 'wait_until_next_sweep' in 'NMethodSweeper::possibly_sweep() {}' has a signed
>>> to unsigned conversion bug. The calculation is currently done as follows:
>>>
>>> double wait_until_next_sweep = (ReservedCodeCacheSize / (16 * M)) - time_since_last_sweep -
>>> CodeCache::reverse_free_ratio();
>>>
>>> Since the type of 'ReservedCodeCacheSize' (uintx) has a higher rank than 'time_since_last_sleep' (int) and
>>> 'time_since_last_sweep' can be larger than ' (ReservedCodeCacheSize / (16 * M))' there is an underflow. Consequently,
>>> 'wait_until_next_sweep' is assigned a wrong value, which disables the intended periodic code cache sweeps.
>>>
>>> Solution:
>>> Use only signed types for the calculation of 'wait_until_next_sweep'. Furthermore, an assert checks that
>>> 'wait_until_next_sweep' is never larger than ' (ReservedCodeCacheSize / (16 * M))', which is the
>>> maximum time between two sweeps.
>>>
>>>
>>> Many thanks in advance,
>>> Albert