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

[Zoltán Majó]

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