RFR[s] 8179305 - Avoid repeated calls to JavaThread::last_frame in InterpreterRuntime

[Ioi Lam]

Hi Dean,

Thanks for the review. I left a few thread->last_frame() that were 
inside ifdef ASSERT, but you're right, I should change those as well to 
be consistent:

diff -r dbee2fa1d3df src/share/vm/interpreter/interpreterRuntime.cpp
--- a/src/share/vm/interpreter/interpreterRuntime.cpp Mon May 01 
11:16:01 2017 -0700
+++ b/src/share/vm/interpreter/interpreterRuntime.cpp Mon May 01 
12:30:01 2017 -0700
@@ -642,7 +642,7 @@
  IRT_ENTRY_NO_ASYNC(void, InterpreterRuntime::monitorenter(JavaThread* 
thread, BasicObjectLock* elem))
  #ifdef ASSERT
- thread->last_frame().interpreter_frame_verify_monitor(elem);
+ 
LastFrameAccessor(thread).get_frame().interpreter_frame_verify_monitor(elem);
  #endif

@@ -659,7 +659,7 @@
  #ifdef ASSERT
- thread->last_frame().interpreter_frame_verify_monitor(elem);
+ 
LastFrameAccessor(thread).get_frame().interpreter_frame_verify_monitor(elem);
  #endif
  IRT_END

@@ -667,7 +667,7 @@
  IRT_ENTRY_NO_ASYNC(void, InterpreterRuntime::monitorexit(JavaThread* 
thread, BasicObjectLock* elem))
  #ifdef ASSERT
- thread->last_frame().interpreter_frame_verify_monitor(elem);
+ 
LastFrameAccessor(thread).get_frame().interpreter_frame_verify_monitor(elem);
  #endif

@@ -680,7 +680,7 @@
  #ifdef ASSERT
- thread->last_frame().interpreter_frame_verify_monitor(elem);
+ 
LastFrameAccessor(thread).get_frame().interpreter_frame_verify_monitor(elem);
  #endif
  IRT_END

What do you think?

Thanks
- Ioi



On 5/1/17 12:14 PM, dean.long at oracle.com wrote:
> Looks OK to me, but it seems arbitrary now which calls can use 
> LastFrameAccessor methods and which need to go through the get_frame() 
> back-door.
>
> dl
>
>
> On 4/28/17 4:06 AM, Ioi Lam wrote:
>> https://bugs.openjdk.java.net/browse/JDK-8179305
>> http://cr.openjdk.java.net/~iklam/jdk10/8179305-avoid-last-frame.v01/
>>
>> Summary:
>>
>> JavaThread::last_frame() is an expensive call. We use the helper 
>> class LastFrameAccessor holds the value of last_frame:
>>
>> OLD:
>>   methodHandle m (thread, method(thread));// calls 
>> JavaThread::last_frame() internally
>>   Bytecode_loadconstant ldc(m, bci(thread));// calls 
>> JavaThread::last_frame() internally
>>
>> NEW:
>>   LastFrameAccessor last_frame(thread);
>>   methodHandle m (thread, last_frame.method());
>>   Bytecode_loadconstant ldc(m, last_frame.bci());
>>
>> Testing:
>>
>> Preliminary benchmarking shows significant VM start-up improvement:
>>
>> java -version:
>> no CDS = 80.35 ms -> 79.14ms (-1.5%)
>> w/ CDS = 53.89ms -> 52.62ms (-2.36%)
>>
>> clojure sample app:
>> no CDS = 1442.46ms -> 1436.11ms (-0.44%)
>> w/ CDS = 695.78ms -> 679.52ms (-2.34%)
>>
>> Thanks
>> - Ioi
>