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

[Zoltán Majó]

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