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

[Daniel D. Daugherty]

On 8/27/14 5:40 AM, serguei.spitsyn at oracle.com wrote:
> On 8/20/14 3:45 PM, Daniel D. Daugherty wrote:
>> On 8/20/14 2:01 PM, Coleen Phillimore wrote:
>>> On 8/20/14, 3:49 PM, serguei.spitsyn at oracle.com wrote:
>>>>>
>>>>> If an EMCP method is not running, should we save it on a previous 
>>>>> version list anyway so that we can make it obsolete if it's 
>>>>> redefined and made obsolete?
>>>>
>>>> I hope, Dan will catch me if I'm wrong...
>>>>
>>>> I think, we should not.
>>>> An EMCP method can not be made obsolete if it is not running.
>>>>
>>>
>>>
>>> It should be this way otherwise we'd have to hang onto things forever.
>>
>> An EMCP method should only be made obsolete if a RedefineClasses() or
>> RetransformClasses() operation made it so. We should not be leveraging
>> off the obsolete-ness attribute to solve a life-cycle problem.
>>
>> In the pre-PGR world, we could trust GC to make a completely unused
>> EMCP method collectible and eventually our weak reference would go
>> away. Just because an EMCP method is not on a stack does not mean
>> that it is not used so we need a different way to determine whether
>> it is OK to no longer track an EMCP method.
>
> Most likely, you are right.
> But I'm not convinced yet. Sorry.
> A convincing point would be a test that shows this behavior.
> I understand that it is not an easy task to write such a test though.
> However, such a test would serve nicely if we want a different way
> to determine whether it is OK to no longer track an EMCP method.

Running the Serviceability stack of tests with the following
-XX:TraceRedefineClasses=NN flags:

//    0x00001000 |       4096 - detect calls to obsolete methods
//    0x00002000 |       8192 - fail a guarantee() in addition to detection

will show failures of the guarantee(). I used to do this
periodically and then chase down the failures to make sure
only the "legitimate" races were happening. The reason that
we had to add these flags was to find all the places where
folks were caching methods in the VM. I think the last place
I found and fixed was in reflection.

Dan


>
>
> Thanks,
> Serguei
>
>>
>>
>>>
>>>> BTW, I'm reviewing the webrev too, but probably it'd be better to 
>>>> switch to updated webrev after it is ready.
>>>
>>> Yes, this is a good idea.  I figured out why I made emcp methods 
>>> obsolete, and I'm fixing that as well as Dan's comments. Thanks!
>>
>> Cool! I'm looking forward to the next review.
>>
>> Dan
>>
>>
>>>
>>> Coleen
>>>
>>>>
>>>> Thanks,
>>>> Serguei
>>>
>>
>