[9] RFR (S): 8074548: Never-taken branches cause repeated deopts in MHs.GWT case

[Vladimir Ivanov]

Thanks, Vladimir.

Best regards,
Vladimir Ivanov

On 3/17/15 9:10 PM, Vladimir Kozlov wrote:
> Looks good.
>
> Thanks,
> Vladimir
>
> On 3/17/15 8:59 AM, Vladimir Ivanov wrote:
>> Updated webrev in place:
>>    http://cr.openjdk.java.net/~vlivanov/8074548/webrev.00/
>>
>> On 3/17/15 4:13 AM, Vladimir Kozlov wrote:
>>> Reading an other comment in this method helped to understand what is
>>> going on:
>>>
>>> // Profile is int[2] where [0] and [1] correspond to false and true
>>> value occurrences respectively.
>>>
>>> May be add comment before next line that result is 0 or 1 and profile is
>>> number of each value occurrences:
>>>
>>> Node* result = argument(0);
>> Done.
>>
>>> I think you need to changes PROB_NEVER to PROB_ALWAYS since you swapped
>>> condition. But please, verify generated code.
>> You are right. Generated code looks good.
>>
>> Best regards,
>> Vladimir Ivanov
>>
>>>
>>> Thanks,
>>> Vladimir
>>>
>>>
>>> On 3/16/15 4:42 PM, Vladimir Ivanov wrote:
>>>>>>> There is no explanation in changes why false_count never seen (==
>>>>>>> 0) is
>>>>>>> more important than true_count == 0. The comment says "one of the
>>>>>>> values".
>>>>>> The code is the following:
>>>>>>      if (false_cnt == 0 || true_cnt == 0) {
>>>>>>        // According to profile, one value has been never seen.
>>>>>
>>>>> That is what confuse me "one value never seen" but true_cnt > 0. What
>>>>> does it mean?
>>>> By "value" I mean boolean value here: "true" or "false".
>>>>
>>>> If (false_cnt == 0) or (true_cnt == 0), but (false_cnt + true_cnt) > 0
>>>> (earlier check), then either the value is always "true" or "false"
>>>> correspondingly. In other words, "false" or "true" (but not both!) has
>>>> been never observed during profiling. That's what the comment
>>>> communicates.
>>>>
>>>> Best regards,
>>>> Vladimir Ivanov
>>>>
>>>>>
>>>>> Vladimir K
>>>>>
>>>>>>        int expected_val = (false_cnt == 0) ? 1 : 0;
>>>>>>
>>>>>> (false_cnt != 0) => (true_cnt == 0).
>>>>>> Do you want me to add additional comment about that?
>>>>>>
>>>>>> I chose (false_cnt == 0) because it refers to "always true" case.
>>>>>>
>>>>>> Best regards,
>>>>>> Vladimir Ivanov
>>>>>>
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Vladimir
>>>>>>>
>>>>>>> On 3/16/15 11:26 AM, Vladimir Ivanov wrote:
>>>>>>>> http://cr.openjdk.java.net/~vlivanov/8074548/webrev.00/
>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8074548
>>>>>>>>
>>>>>>>> MethodHandleImpl::profileBoolean doesn't update never-taken branch
>>>>>>>> count
>>>>>>>> when hitting a deopt on it. As a result, for rarely taken branches
>>>>>>>> consequent compilations consider them as never-taken and prune them
>>>>>>>> again causing repeated deopts. It severely affects peak
>>>>>>>> performance.
>>>>>>>>
>>>>>>>> The fix is to update MHI::profileBoolean intrinsic to insert a
>>>>>>>> guard
>>>>>>>> and
>>>>>>>> uncommon trap w/ reexecute bit set for never-seen value. Once
>>>>>>>> previously
>>>>>>>> never seen value is encountered, the execution resumes after
>>>>>>>> deopt in
>>>>>>>> MHI::profileBoolean and corresponding count becomes non-zero.
>>>>>>>>
>>>>>>>> The guard doesn't add any additional overhead, since it
>>>>>>>> dominates all
>>>>>>>> value usages and all branches on the same value are eliminated.
>>>>>>>>
>>>>>>>> Testing: java/lang/invoke, nashorn, octane
>>>>>>>>
>>>>>>>> Best regards,
>>>>>>>> Vladimir Ivanov
>>>>>>>> _______________________________________________
>>>>>>>> mlvm-dev mailing list
>>>>>>>> mlvm-dev at openjdk.java.net
>>>>>>>> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
>>>>>>> _______________________________________________
>>>>>>> mlvm-dev mailing list
>>>>>>> mlvm-dev at openjdk.java.net
>>>>>>> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev