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

serguei.spitsyn at oracle.com serguei.spitsyn at oracle.com
Wed Sep 3 20:22:39 UTC 2014


Coleen,


Thank you for the answers!
They are helpful.

The new webrev looks good to me.
The only minor comment is about the fragment with long lines:

3633         RC_TRACE(0x00000400, ("add: EMCP method %s is on_stack " INTPTR_FORMAT, old_method->name_and_sig_as_C_string(), old_method));
3634       } else if (!old_method->is_obsolete()) {
3635         RC_TRACE(0x00000400, ("add: EMCP method %s is NOT on_stack " INTPTR_FORMAT, old_method->name_and_sig_as_C_string(), old_method));
3636       }

Could you, please, split these lines?


Thanks!
Serguei



On 9/3/14 12:55 PM, Coleen Phillimore wrote:
>
> 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
>>>>
>>>
>>
>



More information about the hotspot-dev mailing list