RFR (XL) 8155672: Remove instanceKlassHandles and KlassHandles

serguei.spitsyn at oracle.com serguei.spitsyn at oracle.com
Mon Mar 13 18:31:12 UTC 2017


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.

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
>



More information about the hotspot-dev mailing list