RFR: 8221542: ~15% performance degradation due to less optimized inline decision

[Jie Fu]

Hi Vladimir,

The patch[1] seems unreasonable for the following test case:
----------------------------------------------------------
public class MonteCarlo {
     public static void main(String[] args) {
         double sum = 0.0;
         MonteCarlo mc = new MonteCarlo();

         for(int i = 1; i < 3000; i++) {
             sum += mc.integrate(i);
         }

         System.out.println("sum = " + sum);
     }

     public final double integrate(int n) {
         Random R = null;
         if (n > 0) {
           R = new Random(1);
         } else {
           // This call site is not reached.
           // But AbstractInterpreter::is_not_reached(...) returns false 
for it.
           R = new Random(2);
         }

         int underCurve = 0;
         for (int count = 0; count < 1000000; count++) {

             double x = R.nextDouble();
             double y = R.nextDouble();

             if ( x*x + y*y <= 1.0) {
                 underCurve ++;
             }
         }
         return underCurve;
     }
}
----------------------------------------------------------

In patch[1], AbstractInterpreter::is_not_reached(...) is somewhat just 
like callee_method->was_executed_more_than(0).
So I still prefer your previous patch[2].

What do you think?
Thanks.

Best regards,
Jie

[1] http://cr.openjdk.java.net/~vlivanov/jiefu/8221542/webrev.01/
[2] http://cr.openjdk.java.net/~vlivanov/jiefu/8221542/webrev.00/


On 2019/4/17 下午3:33, Vladimir Ivanov wrote:
> Though I don't consider parallel execution case as problematic,
> I got a better idea while browsing the code :-)
>
>   http://cr.openjdk.java.net/~vlivanov/jiefu/8221542/webrev.01
>
> It's inspired by AbstractInterpreter::is_not_reached() and piggybacks 
> on constant pool entry resolution state to determine whether a call 
> was executed in interpreter before.
>
> (The change in cpCache.cpp fixes a latent bug in 
> ConstantPoolCacheEntry::method_if_resolved().)
>
> Best regards,
> Vladimir Ivanov
>
> On 11/04/2019 19:27, Jie Fu wrote:
>> Hi Vladimir,
>>
>>>> Fixed in 
>>>> http://cr.openjdk.java.net/~jiefu/monte_carlo-perf-drop/webrev.03/
>>>
>>> I like it. What do you think about the following version?
>>>
>>> http://cr.openjdk.java.net/~vlivanov/jiefu/8221542/webrev.00/
>> It is more clearer and easier to understand.
>> I prefer your version.
>>
>> One question: I'm not sure if the following condition still holds 
>> with parallel execution of the caller.
>> ---------------------------------------------
>> if (caller_method->was_executed_more_than(1))  return false; // trust 
>> profile
>> ---------------------------------------------
>>
>> For example, assuming that the caller methods was executed 
>> concurrently by 12 threads, is it possible that 
>> caller_method->interpreter_invocation_count()=3 && profile.count()=0 
>> && no exception thrown earlier?
>> Thanks a lot.
>>
>> Best regards,
>> Jie
>>