RFR 8055008: Clean up code that saves the previous versions of redefined classes

[Coleen Phillimore]

Hi Serguei,

I'm going to cut some things...

<<cut>>
> Thank you for the explanation!
>
> There is also a potential scalability issue for class redefinitions as 
> we do a search through
> all these previous_versions and their old methods in the 
> mark_newly_obsolete_methods ().
> In the case of sub-sequential the same class redefinitions this search 
> will become worse and worse.
> However, I'm not suggesting to fix this now. :)

I agree, it seems to take way too long to clear old methods once they 
are in the CodeCache.
>
>> It's different than just saying it's emcp.   It's emcp and it's 
>> running also so needs a breakpoint.
>>
>> The states are really:
>>
>>     is_obsolete() or !is_obsolete() same as is_emcp()
>>
>>     is_running_emcp() == !is_obsolete() && method->on_stack()
>>
>> We need to distinguish the running emcp methods from the non-running 
>> emcp methods.
>
> I suspect, sometimes this invariant is going to be broken:
>   is_running_emcp() == !is_obsolete() && method->on_stack()
>
> When the method has been finished and the on_stack is cleared,
> the method is_running_emcp bit can remain still uncleared, right?
> Would it be more simple just to use "!is_obsolete() && 
> method->on_stack()" ?
> It must be just in a couple of places.

We only set on_stack when we do class redefinition and class unloading 
with MetadataOnStackMark.  After this safepoint, the bit is cleared.   
We don't clear it when the method finishes.

Is running_emcp is in only 4 places, but the place where we really need 
it (setting breakpoints) the "on_stack" bit isn't set because we don't 
do MetadataOnStackMark at that safepoint.   It's sort of an expensive 
operation.

So I need is_running_emcp() to capture the last known running state.

>
>>
>> I guess we could just set breakpoints in all emcp methods whether 
>> they are running or not, and not have this flag.   This seemed to 
>> preserve the old behavior better.
>
> I was thinking about the same but do not really have a preference.
> It is hard to estimate how big memory leak will cause these unneeded 
> breakpoints.
>

It's not so much leakage, because the methods are there anyway but it 
seems inefficient to do breakpoints on methods that have exited.

Setting these breakpoints looks expensive as well!

> <<cut>>
> This is nice, thanks!
> I'm looking at the new webrev version now.

Ok, let me know if there's anything else.
Coleen

>
>
> Thanks,
> Serguei
>
>
>>
>> Thanks,
>> Coleen
>>
>>> Thanks,
>>> Serguei
>>>
>>>
>>> On 9/2/14 5:29 AM, Coleen Phillimore wrote:
>>>>
>>>> Serguei, I didn't answer one of your questions.
>>>>
>>>> On 8/28/14, 5:43 PM, serguei.spitsyn at oracle.com wrote:
>>>>>> This bit is set during purging previous versions when all methods 
>>>>>> have been marked on_stack() if found in various places.  The bit 
>>>>>> is only used for setting breakpoints.
>>>>>
>>>>> I had to ask slightly different.
>>>>> "How precise must be the control of this bit?"
>>>>> Part of this question is the question below about what happens 
>>>>> when the method invocation is finished.
>>>>> I realized now that it can impact only setting breakpoints.
>>>>> Suppose, we did not clear the bit in time and then another 
>>>>> breakpoint is set.
>>>>> The only bad thing is that this new breakpoint will be useless.
>>>>
>>>> Yes.  We set the on_stack bit which causes setting the 
>>>> is_running_emcp bit during safepoints for class redefinition and 
>>>> class unloading.  After the safepoint, the on_stack bit is 
>>>> cleared.   After the safepoint, we may also set breakpoints using 
>>>> the is_running_emcp bit.  If the method has exited we would set a 
>>>> breakpoint in a method that is never reached.  But this shouldn't 
>>>> be noticeable to the programmer.
>>>>
>>>> The method's is_running_emcp bit and maybe metadata would be 
>>>> cleaned up the next time we do class unloading at a safepoint.
>>>>
>>>>>
>>>>> But let me look at new webrev first to see if any update is needed 
>>>>> here.
>>>>>
>>>>
>>>> Yes, please review this again and let me know if this does what I 
>>>> claim it does.
>>>>
>>>> Thank you!
>>>> Coleen
>>>
>>
>