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

[San Hong Li]

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