RFR (XL) 8155672: Remove instanceKlassHandles and KlassHandles

[coleen.phillimore at oracle.com]

On 3/13/17 3:42 PM, serguei.spitsyn at oracle.com wrote:
> On 3/13/17 12:34, coleen.phillimore at oracle.com wrote:
>>
>>
>> On 3/13/17 2:31 PM, serguei.spitsyn at oracle.com wrote:
>>> Hi Coleen,
>>>
>>> It looks good.
>>> Nice cleanup!
>>>
>>> http://oklahoma.us.oracle.com/~cphillim/webrev/8155627.src.closed.02/webrev/share/vm/jfr/jfrClassAdapter.cpp.frames.html 
>>>
>>>
>>> The HandleMark's at 1743 & 1784 can be not needed anymore.
>>
>> Thank you for reviewing!   I suspect many HandleMarks lying around 
>> the code aren't needed anymore, because KlassHandles (and even 
>> methodHandles) aren't cleaned up using HandleMark.   I've removed 
>> those two (and ran some retests because this is getting dangerous).
>>
>> Are you going to review the open too?
>
> Yes, but now I'm waiting for webrev version 03.
> This link is not resolved for me:
>   http://cr.openjdk.java.net/~coleenp/8155672.03/webrev

It's there now.  I transposed the digits in the last webrev.  Thanks for 
letting me know.

Coleen

>
> Thanks,
> Serguei
>
>>
>> thank you!
>> Coleen
>>>
>>> Thanks,
>>> Serguei
>>>
>>>
>>> On 3/12/17 21:18, coleen.phillimore at oracle.com wrote:
>>>>
>>>> As requested by David on the closed part of this review, I fixed 
>>>> variable names of the form h_ik and klass_h where the 'h' indicated 
>>>> that the klass was in a handle.
>>>>
>>>> open webrev at http://cr.openjdk.java.net/~coleenp/8155672.01/webrev
>>>>
>>>> Also, fixing InstanceKlass to not pass this_k can be done in a 
>>>> separate cleanup.
>>>>
>>>> Thanks,
>>>> Coleen
>>>>
>>>> On 3/12/17 11:32 AM, coleen.phillimore at oracle.com wrote:
>>>>> Summary: Use unhandled pointers for Klass and InstanceKlass, 
>>>>> remove handles with no implementation.
>>>>>
>>>>> These are fairly extensive changes.  The KlassHandle types have 
>>>>> been dummy types since permgen elimination and were thought to be 
>>>>> useful for future features.  They aren't, so can be removed (see 
>>>>> bug for more details).   This allows stricter typing because you 
>>>>> can use the more specific type, and using const more.  I didn't 
>>>>> add const to these changes, because of the cascading effect.
>>>>>
>>>>> The change isn't hard to review, just tedious.  The main bug that 
>>>>> I had was redeclaring a type inside a scope, and 
>>>>> InstanceKlass::cast(k) can't take a NULL k, whereas 
>>>>> instanceKlassHandle ik(THREAD, k) could.
>>>>>
>>>>> It's so nice being able to single step on gdb without going into 
>>>>> KlassHandle constructors!
>>>>>
>>>>> open webrev at http://cr.openjdk.java.net/~coleenp/8155672.01/webrev
>>>>> bug link https://bugs.openjdk.java.net/browse/JDK-8155672
>>>>>
>>>>> Tested with all hotspot jtreg tests, java/lang/invoke tests, 
>>>>> java/lang/instrument tests, all closed tonga colocated tests, and 
>>>>> JPRT.
>>>>>
>>>>> I can continue to hold this change for a convenient time for 
>>>>> merging purposes with other people's JDK10 changes that they've 
>>>>> been holding, or if there are other jdk9 changes that are likely 
>>>>> to cause a problem for merging. I'll update the copyrights to 2017 
>>>>> on commit.
>>>>>
>>>>> Thanks,
>>>>> Coleen
>>>>
>>>
>>
>