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

[Jie Fu]

I'm sorry I missed the running script for my test case.

--------------------------------------
#!/bin/bash

JDK=/home/fool/jdk-dev/build/linux-x86_64-server-release/images/jdk

${JDK}/bin/javac MonteCarlo.java

${JDK}/bin/java \
   -XX:+PrintCompilation \
   -XX:-TieredCompilation \
   -XX:CICompilerCount=1 \
   -XX:+UnlockDiagnosticVMOptions \
   -XX:+PrintInlining \
   -XX:-UseOnStackReplacement \
   MonteCarlo
--------------------------------------

On 2019/4/18 下午4:18, Jie Fu wrote:
> 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
>>>
>