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

[Vladimir Kozlov]

On 11/25/13 11:27 AM, Albert Noll wrote:
> Hi Vladimir,
>
> Thanks for looking at this. The current patch casts the unsigned value (ReservedCodeCacheSize) to a signed value. That should be fine, since the result will always be small enough to fit into an int.

Yes, it is true.

>
> This seems clearer to me than depending on ordering and implicit type casts.

Okay.

Thanks,
Vladimir

>
> Best,
> Albert
>
> Von meinem iPhone gesendet
>
>> Am 25.11.2013 um 18:58 schrieb Vladimir Kozlov <vladimir.kozlov at oracle.com>:
>>
>> 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