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

[serguei.spitsyn at oracle.com]

On 8/27/14 6:54 AM, Daniel D. Daugherty wrote:
>
> 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.

Ok, thanks.
We threat this as buggy behavior, right?

Thanks,
Serguei

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