[9] RFR(S): 8072398: assert fails in L1RGenerator::increment_event_counter_impl

[Zoltán Majó]

Thank you, Vladimir!

Best regards,


Zoltan

On 02/06/2015 06:11 PM, Vladimir Kozlov wrote:
> Good.
>
> Thanks,
> Vladimir
>
> On 2/6/15 2:56 AM, Zoltán Majó wrote:
>> Hi,
>>
>>
>> the previous webrev excluded a one-line change that is needed to make 
>> this work.
>>
>> Here is the new webrev: 
>> http://cr.openjdk.java.net/~zmajo/8072398/webrev.01/
>>
>> The new change is in arguments.cpp at
>>
>> *** 3919,3929 ****
>> --- 3925,3936 ----
>>
>> This change is analogous to the change at
>>
>> *** 1202,1213 ****
>> --- 1207,1219 ----
>>
>> in the previous webrev.
>>
>> Sorry for the extra effort.
>>
>> Thank you and best regards,
>>
>>
>> Zoltan
>>
>>
>>
>> On 02/06/2015 10:57 AM, Zoltán Majó wrote:
>>> Thank you, Vladimir and Igor, for the review! I'll push this change 
>>> today.
>>>
>>> Best regards,
>>>
>>>
>>> Zoltan
>>>
>>> On 02/05/2015 11:52 PM, Zoltán Majó wrote:
>>>> Hi Igor,
>>>>
>>>>
>>>> thank you for the feedback.
>>>>
>>>>
>>>> On 02/05/2015 09:59 PM, Igor Veresov wrote:
>>>>> The code looks good. But I think the comment about 
>>>>> CompileThresholdScaling == 0 being equivalent to -Xint is bit
>>>>> misleading. The VM does not become interpreter-only, does it?
>>>>
>>>> It does.
>>>>
>>>> The reason is that CompileThreshold==0 has been historically 
>>>> equivalent to -Xint (see lines 3488-3493 in
>>>> arguments.cpp). CompileThresholdScaling==0.0 scales down 
>>>> CompileThreshold to 0, so we decided to keep functionality
>>>> consistent and make CompileThresholdScaling equivalent to -Xint as 
>>>> well when we introduced the
>>>> CompileThresholdScaling flag in 8059604.
>>>>
>>>> I hope this makes sense.
>>>>
>>>> Thank you!
>>>>
>>>> Best regards,
>>>>
>>>>
>>>> Zoltan
>>>>
>>>>
>>>>
>>>>>
>>>>> igor
>>>>>
>>>>>> On Feb 5, 2015, at 8:18 AM, Zoltán Majó <zoltan.majo at oracle.com> 
>>>>>> wrote:
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>>
>>>>>> please review the following small patch.
>>>>>>
>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8072398
>>>>>>
>>>>>> Problem: The reported assert is triggered because the method 
>>>>>> LIRGenerator::increment_event_counter_impl is called
>>>>>> with parameter 'frequency' set to -1.
>>>>>>
>>>>>> The value originates from Arguments::scaled_freq_log(): If both 
>>>>>> variables 'freq_log' and 'scale' are small,
>>>>>> 'scaled_freq' can become 0 and the function log2_intptr returns 
>>>>>> -1 for the value 0. A similar problem appears if
>>>>>> freq_log == 0.
>>>>>>
>>>>>>
>>>>>> Solution:
>>>>>>
>>>>>> To treat the above mentioned cases graciously, this patch:
>>>>>>
>>>>>> - Changes Arguments::scaled_freq_log() and 
>>>>>> Arguments::scaled_compile_threshold() to allow scaling the
>>>>>> frequency/threshold by 0.0 and return 0.0 in these cases without 
>>>>>> calling log2_intptr().
>>>>>>
>>>>>> - Adds a new condition to Arguments::set_tiered_flags(): 
>>>>>> Threshold values are *not scaled* if
>>>>>> 'CompileThresholdScaling' is 0.0 (behavior equivalent to -Xint). 
>>>>>> Previously, this functionality was encoded in
>>>>>> scaled_freq_log() and scaled_compile_threshold(). Handling this 
>>>>>> case in set_tiered_flags() results in clearer code.
>>>>>>
>>>>>> - Improves comments.
>>>>>>
>>>>>>
>>>>>> Webrev: http://cr.openjdk.java.net/~zmajo/8072398/webrev.00/
>>>>>>
>>>>>> Testing:
>>>>>> - manual testing with failing test cases
>>>>>> - compiler/arguments/CheckCompileThresholdScaling.java
>>>>>> - JPRT
>>>>>>
>>>>>> Thank you and best regards,
>>>>>>
>>>>>>
>>>>>> Zoltan
>>>>>>
>>>>
>>>
>>