request for review (m): 4957990: Perm heap bloat in JVM

[Tom Rodriguez]

On Aug 5, 2009, at 11:10 PM, Y. Srinivas Ramakrishna wrote:

> Tom Rodriguez wrote:
>> After I wrote this I realized that this is a natural out growth of  
>> your fix.  Without profiling information an nmethod should only  
>> refer to types which are statically reachable.  Having the MDO act  
>> as a strong root would keep the nmethod from getting unloaded when  
>> using predicted call sites.  Once the MDO is a weak root then the  
>> nmethod could want to be unloaded.  I guess without your fix CHA  
>> could produce a case where an OSR nmethod would be unloaded without  
>> being unlinked from the OSR nmethod list but I'd have to think  
>> about that more.
>
> So, Tom, is my understanding correct that the criteria for unloading  
> of the nmethod are not too
> weak. Do you suggest that make_unloaded should just go unlink the  
> nmethod from the osr list?
> I'll give that a try tonight and update later on how that looks wrt  
> the current test,
> but let me know if i am misunderstanding and that would not  be the  
> correct way to
> fix this problem,

We can't allow the sweeper to free an nmethod unless we are certain  
that it's impossible for new activations of that nmethod to form.   
Note this comment:

void nmethod::make_not_entrant_or_zombie(int state) {
   assert(state == zombie || state == not_entrant, "must be zombie or  
not_entrant");

   // 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.

So I don't think reclamation of OSR nmethods in the current code is  
safe.  I do believe that OSR nmethods could be reclaimed like other  
normal nmethods and I have an old webrev that illustrates this at http://javaweb.sfbay.sun.com/~never/webrev/osr 
.  I think that the changes in the assembly code are unneeded since  
the requirement we're trying to enforce is that if an OSR nmethod  
becomes unloaded at a safepoint it won't be used for form a new  
activation which is accomplished by the interpeterRuntime.cpp  
changes.  I'm not sure we want to include the reclaiming of OSR  
nmethods into hs16 at this point.  So maybe OSR nmethods need to act  
as strong roots for now?

tom

>
> thanks!
> -- ramki
>>
>> tom
>>
>> On Aug 5, 2009, at 6:15 PM, Tom Rodriguez wrote:
>>
>>> My understanding was that OSR nmethods could/should only be  
>>> reclaimed once the klass of their holder was unloaded.  The reason  
>>> for this is that the normal not_entrant sweeping logic isn't  
>>> implemented safely for OSR methods so the sweeper can't safely  
>>> reclaim them.  If the klass itself is being reclaimed then it  
>>> should be safe to reclaim the OSR nmethods.  It's possible that we  
>>> have a hole in the logic which doesn't account for dealing with  
>>> unloaded OSR methods properly and you changes just make it more  
>>> likely to happen.  We either need to close the not_entrant hole  
>>> for OSR nmethods, which isn't that hard, and then correct  
>>> nmethod::make_unloaded to unlink the OSR nmethod or we have to  
>>> make do_unloading disallow unloading for OSR nmethods is the  
>>> methodOop is strongly reachable.
>>>
>>> tom
>>>
>>> On Aug 5, 2009, at 4:57 PM, Y.S.Ramakrishna at Sun.COM wrote:
>>>
>>>>
>>>> Can there be a situation where an osr nmethod associated
>>>> with a method oop gets unloaded while the method and its
>>>> class are still alive? I see that happening with my workspace
>>>> with the changes for 4957990 (yet to figure out why the osr
>>>> nmethod was unloaded).
>>>>
>>>> Shortly after said unload, the sweeper flushes the nmethod,
>>>> but the nmethod is still left linked off of the klass's  
>>>> _osr_nmethod_head
>>>> list, and a subsequent invocation counter overflow at a bci does
>>>> an osr nmethod lookup and falls foul of the flushed nmethod left
>>>> in the instanceKlass's osr nmethod head.
>>>>
>>>> So I have two questions:
>>>>
>>>> (a) can an osr nmethod be unloaded while its klass is still alive ?
>>>> (b) if the answer to (a) is yes, then we need to take special steps
>>>> (not present in current code) to unlink said nmethod from the
>>>> list of osr nmethods in its klass.
>>>>
>>>> Am I making sense? Otherwise i can provide more direct detail.
>>>> (I am still digging to find out why we decided to unload the
>>>> nmethod and will have more follow-up info in a subsequent email.)
>>>>
>>>> thanks.
>>>> -- ramki
>>>
>>
>