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

[Vladimir Ivanov]

Good catch, Jie!

The resolution state is usually shared between call sites referring to 
the same constant pool entry, so is_not_reached() is definite only when 
invoke is not reached.

I'm OK with dropping was_executed_more_than(1) check. I hoped it could 
help catching the case when exception is thrown before the call, but the 
check itself causes more problems than I thought.

Here's updated version:
   http://cr.openjdk.java.net/~vlivanov/jiefu/8221542/webrev.02

After some explorations I decided to keep original behavior for immature 
profiles (profile.count == -1).

Best regards,
Vladimir Ivanov

On 18/04/2019 01: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
>>>
>