RFR (XXS): 8023191: OSR nmethods should be flushed to free space in CodeCache

[Vladimir Kozlov]

On 10/9/13 10:21 AM, Vladimir Kozlov wrote:
> Albert,
>
> Changes are fine but you need more testing.
>
> Run full CTW too and all jtreg tests from Hotspot and jdk (run jdk tests
> with -Xcomp and without it).
>
> Also the bug report has link to test (jittest). Can you build and run it
> too to see if it solves the problem with small codecache? It fills
> codecache with osr nmethods.
>
> It is very delicate matter. There was a reason we did not remove OSR
> nmethod until corresponding klass is unloaded (yes, deoptimization can
> make it non-entrant). Unfortunately I don't remember it. May be John
> know. Non of inline caches are pointing to OSR nmethods since it is
> called only from Interpreter. There is transition phase at the start of
> OSR nmethod when it calls runtime to convert interpreter frame to
> compiled frame. And method has separate field for osr nmethods.

I think I dig out the answer.

First, the last statement in previous mail is wrong. The method object 
does not have reference to osr nmethods. Each java method can have 
several alive OSR nmethods for different bcis. Instead method's holder 
class has list of all OSR nmethods associated with all methods of this 
class. Each time we do osr lookup we go through this list.
Since method does not have reference to OSR the original flushing 
(speculative disconnection) could not be implemented for OSR nmethods.

Second. Originally osr nmethod were not cleaned up from codecache. Here 
is the comment and the code in nmethod::make_not_entrant_or_zombie() 
from long time ago:

   // Code for an on-stack-replacement nmethod is removed when a class 
gets unloaded.
   // They never become zombie/non-entrant, so the nmethod sweeper will 
never remove
   // them. Instead the entry_bci is set to InvalidOSREntryBci, so the 
osr nmethod
   // will never be used anymore. That the nmethods only gets removed 
when class unloading
   // happens, make life much simpler, since the nmethods are not just 
going to disappear
   // out of the blue.
   if (is_osr_only_method()) {
     if (osr_entry_bci() != InvalidOSREntryBci) {
       // only log this once
       log_state_change(state);
     }
     invalidate_osr_method();
     return;
   }

This code was changed by Tom for 5057818 4 years ago to allow cleanup 
unused OSR nmethods from codecache:

http://hg.openjdk.java.net/hsx/hotspot-comp/hotspot/rev/032260830071

Third, I think those changes are not complete. 
nmethod::invalidate_osr_method() should check for _entry_bci == 
InvalidOSREntryBci to avoid lookup through osr list again after nmethod 
change from non_entrant to zombie (invalidate_osr_method() is called 
each time make_not_entrant_or_zombie() is called). May be other places. 
Note, when we process osr nmethod::_code should not be touched since it 
is pointing to normal compiled nmethod which could still be alive.

Thanks,
Vladimir

>
> Thanks,
> Vladimir
>
> On 10/8/13 10:18 PM, Albert Noll wrote:
>> Hi,
>>
>> could I have a review for this small patch?
>>
>> bug: https://bugs.openjdk.java.net/browse/JDK-8023191
>> webrev: http://cr.openjdk.java.net/~anoll/8023191/webrev.00/
>> <http://cr.openjdk.java.net/%7Eanoll/8023191/webrev.00/>
>>
>> Problem: OSR methods are not removed from code cache to free space.
>> Solution: Remove OSR methods from code cache. OSR methods c an be made
>>                 not-entrant and are then removed from the code cache
>> likenon-OSR
>>                 methods. Other parts in the code (e.g.,
>> deoptimization.cpp:1547) already make
>>                 OSR methods not-entrant.
>>
>> Testing: Passed JPRT; I'll also run specjvm and nashorn to see if there
>> is an impact on performance.
>>
>> Many thanks in advance,
>> Albert