RewriteFrequentPairs flag was disabled in debug model(and discussion for fix)

[San Hong Li]

Hi,
Just add a few more words to explain the benefit on debug experience...

If with that fix(hmmm, I just assumed that the fix in previous discussion
already covered all the cases to make the RewriteFrequentPairs work in
debug mode)

we at least give the user another option to enable the
 RewriteFrequentPairs flag in debug mode, just like we did it for
RewriteBytecodes flag,  so that the debugger can also inspect these super
instructions in interpreter in debug mode.

your options?

Thanks!
San Hong

On Tue, Jan 27, 2015 at 9:56 PM, Daniel D. Daugherty <
daniel.daugherty at oracle.com> wrote:

> Resending to include the Serviceability team since they own JVM/TI...
>
> Dan
>
>
> On 1/26/15 11:53 PM, San Hong Li wrote:
>
>> HI All:
>> When the JVM is running in debug model,   the RewriteFrequentPairs
>> optimization will be disabled by following code  in
>>   JvmtiManageCapabilities::update():
>>
>> *  if (avail.can_generate_breakpoint_events) {*
>> *    RewriteFrequentPairs = false;*
>> *  }*
>>
>> *[ pls. check the above code  @Line 328 in jvmtiManageCapabilities.cpp  in
>> jdk7 codebase]*
>>
>> I understand the reason why the  RewriteFrequentPairs doesn't work in
>> debug
>> model is that the TOS cache will be disturbed after the execution of
>> TemplateTable::aload_0  in such this case,
>>
>> Let us discuss it in more details about what will happen if we enable
>> RewriteFrequentPairs  flag in debugging model...
>>
>>
>> The following code was  excerpted from  TemplateTable::aload_0
>> ( @L789 in templateTable_x86_64.cpp).
>> For better discussion, I just skipped irrelevant parts:
>>
>> *void TemplateTable::aload_0() {*
>> *  transition(vtos, atos);*
>>
>>
>> *  if (RewriteFrequentPairs) {*
>> *      ......  *
>> *      // do actual aload_0*
>> *      aload(0);                     *
>> *      ......*
>> *      // rewrite*
>> *      // bc: fast bytecode*
>> *      __ bind(rewrite);*
>> *      patch_bytecode(Bytecodes::_aload_0, bc, rbx, false);*
>> *      __ bind(done);*
>> *  } else {*
>> *    aload(0);*
>> *  }*
>> *}*
>>
>> In above code:
>> The *aload(0)  *happened before * patch_bytecode. *
>> The TOS cache will be set correctly after *aload(0) *, that's, the  state
>> should be atos as we have expected.
>>
>> But the cache will be corrupted by the following *patch_bytecode
>> *which actually
>>   calls out to InterpreterRuntime::set_original_bytecode_at:
>>
>> Pls. check the implementation around at L256 in  templateTable_x86_64.cpp:
>>
>> *    // Let breakpoint table handling rewrite to quicker bytecode*
>> *    __ call_VM(noreg, CAST_FROM_FN_PTR(address,
>> InterpreterRuntime::set_original_bytecode_at), temp_reg, r13, bc_reg);*
>>
>> The problem  here is:
>> The call out into InterpreterRuntime::set_original_bytecode_at will
>> eventually change the values cached in RAX.
>>
>> I would like to discuss and  propose a fix for this issue:  just do
>> the * aload(0)
>> *after *patch_bytecode!*
>> (actually similar  thing we did in TemplateTable::iload) ,
>> So the updated code with fix likes this:
>>
>> *void TemplateTable::aload_0() {*
>> *  transition(vtos, atos);*
>>
>>
>> *  if (RewriteFrequentPairs) {*
>> *      ......  *
>> *      // do actual aload_0*
>> *      //aload(0);                     *
>> *      ......*
>> *      // rewrite*
>> *      // bc: fast bytecode*
>> *      __ bind(rewrite);*
>> *      patch_bytecode(Bytecodes::_aload_0, bc, rbx, false);*
>> *      __ bind(done);*
>> *  }  *
>> *   // do actual aload_0*
>> *    __ movptr(rax, aaddress(0));    *
>> *}*
>>
>> So that the TOS has been cached correctly without disruption by
>> *patch_bytecode,
>> *safe use for next bytecode.
>>
>> Finally, just a summary for my questions:
>>
>> Whether or not *this is the only case t*hat explained why the current
>> implementation of HotSpot  has to disable RewriteFrequentPairs flag in
>> debug mode?
>>      --- If so,  how do u think about the fix?  I did some tests in my
>> environment, it works fine.
>>      --- if not,  would you pls. point out in which case we also have to
>> disable RewriteFrequentPairs  during debugging?
>>
>> Thanks!
>> San Hong
>>
>
>